This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] NFC: Use macros to accommodate differences in representation of PowerPC assemblers
ClosedPublic

Authored by xingxue on Apr 23 2021, 9:33 AM.

Details

Summary

This NFC patch replaces the representation of registers and the left shift operator in the PowerPC assembly code to allow it to be consumed by the GNU flavored assembler and the AIX assembler.

  • Registers - change the representation of PowperPC registers from %rn, %fn, %vsn, and %vrn to the register number alone, e.g., n. The GNU flavored assembler and the AIX assembler are able to determine the register kind based on the context of the instruction in which the register is used.
  • Left shift operator - use macro PPC_LEFT_SHIFT to represent the left shift operator. The left shift operator in the AIX assembly language is < instead of <<

Diff Detail

Event Timeline

xingxue created this revision.Apr 23 2021, 9:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2021, 9:33 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
xingxue requested review of this revision.Apr 23 2021, 9:33 AM
sfertile accepted this revision.Apr 26 2021, 8:29 AM

Thanks for splitting this out Xing. I suggest showing the syntax differences between gas and AIX assembly we intend to work around with these macros in the commit message. LGTM.

While I think that this is fine in general, I'm not sure why the shift operator needs to be replaced. Could you shed some light on that?

MaskRay added inline comments.
libunwind/src/assembly.h
220

unneeded // clang-format off ?

While I think that this is fine in general, I'm not sure why the shift operator needs to be replaced. Could you shed some light on that?

Sorry, I should have mentioned in the description, the left shift operator in the AIX assembly language is < instead of <<.

libunwind/src/assembly.h
220

The pre-merge check failed because clang-format wants to make the following change. However, the assembler will fail if a space is inserted in between % and the register character after it. // clang-format off is used to turn off clang-format for this segment.

clang-format: please reformat the code

-#define GPR(num) %r##num
-#define FPR(num) %f##num
-#define VSR(num) %vs##num
-#define VR(num) %v##num
+#define GPR(num) % r##num
+#define FPR(num) % f##num
+#define VSR(num) % vs##num
+#define VR(num) % v##num
xingxue edited the summary of this revision. (Show Details)May 3 2021, 7:48 AM
xingxue added reviewers: MaskRay, compnerd.

Hi @MaskRay @compnerd, Do you have any further comments?

Do we need GPR? ld 0, 4(3) works as ld r0, 4(r3) in GNU flavored assembly. Other macros may be similar.

xingxue updated this revision to Diff 342984.May 5 2021, 4:17 AM
xingxue edited the summary of this revision. (Show Details)

Addressed comments:

  • Changed the representation of PowperPC registers from %rn, %fn, %vsn, and %vrn to the register number alone. The GNU flavored assembler and the AIX assembler can determine the register kind based on the context of the instruction in which the register is used.

Do we need GPR? ld 0, 4(3) works as ld r0, 4(r3) in GNU flavored assembly. Other macros may be similar.

Good to know, changed according. Thanks!

xingxue updated this revision to Diff 343025.May 5 2021, 6:30 AM

Minor change from clang-format.

compnerd accepted this revision.May 6 2021, 10:05 AM
This revision is now accepted and ready to land.May 6 2021, 10:05 AM
MaskRay accepted this revision.May 6 2021, 10:28 AM

LGTM.