Page MenuHomeFreeBSD

stand: Remove dead store to bi_kernelname
ClosedPublic

Authored by imp on Sep 12 2022, 9:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 22, 10:25 PM
Unknown Object (File)
Fri, Mar 22, 10:25 PM
Unknown Object (File)
Fri, Mar 22, 10:25 PM
Unknown Object (File)
Sat, Mar 9, 3:36 PM
Unknown Object (File)
Sat, Mar 9, 2:31 PM
Unknown Object (File)
Jan 6 2024, 5:26 PM
Unknown Object (File)
Jan 6 2024, 5:22 PM
Unknown Object (File)
Jan 6 2024, 5:22 PM
Subscribers

Details

Summary

We set this value twice: once to 0 and once to the VA that has the name
of the kernel. The first store is redundant.

Sponsored by: Netflix

Test Plan

For some crazy reason, this saves 7 bytes of code.
Clang is not yet smart enough to realize this is a write-only field in the loader
But since it's a crazy result, I need eyes on this otherwise boring change.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested review of this revision.Sep 12 2022, 9:01 PM
imp added a reviewer: jhb.
jrtc27 added inline comments.
stand/i386/libi386/bootinfo32.c
176

The bd_getbigeom call below is opaque and VTOP leaks the address of bi. The compiler doesn't know the function is only called once and that bd_getbigeom doesn't depend on the contents of the leaked bi from a prior call. The basic pattern is https://godbolt.org/z/nGrfoGP97.

stand/userboot/userboot/bootinfo32.c
155

The various CALLBACKs will be similarly opaque, and the copyin of bi leaks its address so it's as before.

stand/i386/libi386/bootinfo32.c
176

But bi is static... though I suppose that bi_load32 could be called recursively, at least as far as the compiler is concerned.

I'm sold.

stand/i386/libi386/bootinfo32.c
176

static... except its address leaks outside the translation unit, which throws out a bunch of optimisation opportunities as it might as well not be once that happens

imp edited the test plan for this revision. (Show Details)

none of these need to be set to 0

This revision was not accepted when it landed; it landed in state Needs Review.Sep 16 2022, 3:54 PM
This revision was automatically updated to reflect the committed changes.