Page MenuHomeFreeBSD

i386 pcpu: fix clobbers, suppress warnings, and clean up
ClosedPublic

Authored by rlibby on Jul 1 2024, 4:48 PM.
Tags
None
Referenced Files
F103319154: D45826.diff
Sat, Nov 23, 12:11 PM
Unknown Object (File)
Tue, Nov 19, 6:10 AM
Unknown Object (File)
Tue, Nov 19, 4:53 AM
Unknown Object (File)
Sep 19 2024, 2:44 PM
Unknown Object (File)
Sep 8 2024, 5:07 PM
Unknown Object (File)
Sep 8 2024, 6:32 AM
Unknown Object (File)
Sep 8 2024, 2:11 AM
Unknown Object (File)
Aug 29 2024, 7:57 PM
Subscribers

Details

Summary
  • Add missing cc clobber to __PCPU_ADD (which is currently unused).
  • Allow the compiler the opportunity to marginally improve code generation from __PCPU_PTR by letting it figure out how to do the add (also removing the addition fixes a missing cc clobber).
  • Quiet gcc -Warray-bounds by using constant operands instead of bogus memory references.
  • Remove the struct s s temporaries, just cast through the type.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58441
Build 55329: arc lint + arc unit

Event Timeline

rlibby requested review of this revision.Jul 1 2024, 4:48 PM

Test patch used to examine codegen:

(I also #if 0'd the __SEG_GS section of amd64's pcpu.h in order to check the changes.)

Disassembly before the patch for amd64 clang, amd64 gcc, i386 clang, and i386 gcc:

Disassembly after both D45826 and D45827 for amd64 clang, amd64 gcc, i386 clang, and i386 gcc:

diff -u1000 of the above before/after disassemblies:

This revision is now accepted and ready to land.Jul 1 2024, 9:15 PM

By the way, I thought about also making these strict-aliasing safe with a union. It's straightforward and also gets rid of the casts. However, since we explicitly build with -fno-strict-aliasing, there's not much motivation. But in case you'd prefer that, I can tweak these patches.

By the way, I thought about also making these strict-aliasing safe with a union. It's straightforward and also gets rid of the casts. However, since we explicitly build with -fno-strict-aliasing, there's not much motivation. But in case you'd prefer that, I can tweak these patches.

Why would a special treat for aliasing needed? There is the "memory" clobber.

In D45826#1045245, @kib wrote:

By the way, I thought about also making these strict-aliasing safe with a union. It's straightforward and also gets rid of the casts. However, since we explicitly build with -fno-strict-aliasing, there's not much motivation. But in case you'd prefer that, I can tweak these patches.

Why would a special treat for aliasing needed? There is the "memory" clobber.

Yes, I think you're right that the memory clobbers avoid any actual strict aliasing safety issue.

Gcc would still issue a diagnostic (despite the clobbers) if we didn't use -fno-strict-aliasing, but we do, so, it would be an exercise in suppressing a diagnostic that's not being issued. And I agree that sounds pointless.