This is an archive of the discontinued LLVM Phabricator instance.

Multiple preprocessor fixes for libunwind on PowerPC*.
ClosedPublic

Authored by Bdragon28 on Nov 20 2020, 8:08 PM.

Details

Summary
  • Remove misnamed PPC64_HAS_VMX in preference of directly checking defined(__VSX__).

libunwind was using "VMX" to mean "VSX". "VMX" is just another name for Altivec, while "VSX" is the vector-scalar extensions first used in POWER7. Exposing a "PPC64_HAS_VMX" define was misleading and incorrect.

  • Add defined(__ALTIVEC__) guards around vector register operations to fix non-altivec CPUS such as the e5500.

When compiling for certain Book-E processors such as the e5500, we want to skip vector save/restore, as the Altivec registers are illegal on non-Altivec implementations.

  • Add !defined(__NO_FPRS__) guards around traditional floating-point save/restore.

When compiling for powerpcspe, we cannot access floating point registers, as there aren't any. (The SPE on e500v2 is a 64-bit extension of the GPRs, and it doesn't have the normal floating-point registers at all.)
This fixes building for powerpcspe, although no actual handling for SPE save/restore is written yet.

Diff Detail

Event Timeline

Bdragon28 created this revision.Nov 20 2020, 8:08 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 20 2020, 8:08 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Bdragon28 requested review of this revision.Nov 20 2020, 8:08 PM
Bdragon28 edited the summary of this revision. (Show Details)
Bdragon28 added inline comments.Nov 21 2020, 7:53 AM
libunwind/src/Registers.hpp
1522

#elif rather. Not sure why I had a typo in my local tree.

MaskRay added a subscriber: luporl.Nov 21 2020, 9:26 PM

Looks good to me. (+@luporl who added PPC64_HAS_VMX)

Looks good to me. I guess I didn't know about __VSX__ back when I added PPC64_HAS_VMX.

compnerd accepted this revision.Nov 23 2020, 9:04 AM
compnerd added a subscriber: compnerd.

Thanks @Bdragon28, this definitely is a nice cleanup (and fix). Please remember to fix the #elseif to a #elif.

This revision is now accepted and ready to land.Nov 23 2020, 9:04 AM
Bdragon28 updated this revision to Diff 307231.Nov 23 2020, 6:40 PM

Fixing typo.

OK, patch updated with the typo fix.

The !__ALTIVEC__ case has been tested on an AmigaONE X5000. I will test the __NO_FPRS__ case on my RB800 later tonight. (Test to verify that it resolves the SIGILL, that is. The actual SPE bits will come later.)

Note: I do not have commit access.

MaskRay accepted this revision.Nov 23 2020, 7:02 PM

OK, patch updated with the typo fix.

The !__ALTIVEC__ case has been tested on an AmigaONE X5000. I will test the __NO_FPRS__ case on my RB800 later tonight. (Test to verify that it resolves the SIGILL, that is. The actual SPE bits will come later.)

Note: I do not have commit access.

I'll do it:)

This revision was automatically updated to reflect the committed changes.