Page MenuHomeFreeBSD

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

Authored by rlibby on Mon, Jul 1, 4:48 PM.
Tags
None
Referenced Files
F87606759: D45826.diff
Fri, Jul 5, 6:35 PM
Unknown Object (File)
Tue, Jul 2, 11:33 PM
Unknown Object (File)
Tue, Jul 2, 7:04 PM
Unknown Object (File)
Tue, Jul 2, 6:03 PM
F87316204: pcpu_demo.before-after.diff
Mon, Jul 1, 5:01 PM
F87316157: pcpu_demo.after-all.disas
Mon, Jul 1, 5:01 PM
F87316134: pcpu_demo.before-all.disas
Mon, Jul 1, 5:01 PM
F87316088: 0001-XXX-pcpu-demo-functions.patch
Mon, Jul 1, 5:01 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 Not Applicable
Unit
Tests Not Applicable

Event Timeline

rlibby requested review of this revision.Mon, Jul 1, 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.Mon, Jul 1, 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.