[libunwind] Use .irp directives. NFC
ClosedPublic
Actions

Authored by MaskRay on Dec 5 2022, 2:03 PM.

Details

Reviewers

nemanjai
xingxue
compnerd

Group Reviewers

Restricted Project

Commits

rG356bbbbda9d0: [libunwind] Use .irp directives. NFC
rGc9aff5d2f9d9: Revert "[libunwind] Use .irp directives. NFC"
rG8482e95f75d0: [libunwind] Use .irp directives. NFC

Summary

The repeated instructions make the file long and difficult to read.
Simplify them with .irp directives.

Skip PowerPC since AIX assembler doesn't support .irp

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

MaskRay created this revision.Dec 5 2022, 2:03 PM

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 5 2022, 2:03 PM

Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript

Herald added subscribers: libcxx-commits, StephenFan, s.egerton, simoncook. · View Herald Transcript

MaskRay requested review of this revision.Dec 5 2022, 2:03 PM

Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2022, 2:03 PM

Herald added subscribers: llvm-commits, pcwang-thead. · View Herald Transcript

MaskRay mentioned this in D137599: [libunwind][PowerPC] Fix saving/restoring VSX registers on LE systems.Dec 5 2022, 2:06 PM

MaskRay mentioned this in D137010: [libunwind][LoongArch] Add 64-bit LoongArch support.Dec 5 2022, 2:08 PM

Harbormaster completed remote builds in B201214: Diff 480238.Dec 5 2022, 8:42 PM

update more

Harbormaster completed remote builds in B201287: Diff 480332.Dec 6 2022, 2:56 AM

This is definitely a nice cleanup. There is a way to use .irp to have it count, but this is fine for now.

This revision is now accepted and ready to land.Dec 6 2022, 9:50 AM

In D139368#3975014, @compnerd wrote:

This is definitely a nice cleanup. There is a way to use .irp to have it count, but this is fine for now.

Thanks. Unfortunately, AFAIK, MACRO-11 introduced .irp i,0,1,2,3 and .irpc i,0123 but no generalized iteration from X to Y... GNU as and LLVM MC inherited the directives from MACRO-11 but do not add a generalized iteration...

This revision was landed with ongoing or failed builds.Dec 6 2022, 11:05 AM

Closed by commit rG8482e95f75d0: [libunwind] Use .irp directives. NFC (authored by MaskRay). · Explain Why

This revision was automatically updated to reflect the committed changes.

MaskRay added a commit: rG8482e95f75d0: [libunwind] Use .irp directives. NFC.

This broke the AIX CI: https://buildkite.com/llvm-project/libcxx-ci/builds/15656#0184eac6-521b-4d1e-8502-c226673e0ece. Unfortunately it seems the .irp directive isn't supported by the AIX assembler.

daltenty added a commit: rGc9aff5d2f9d9: Revert "[libunwind] Use .irp directives. NFC".Dec 7 2022, 12:41 PM

daltenty added a reverting change: rGc9aff5d2f9d9: Revert "[libunwind] Use .irp directives. NFC".

Did the libc++/libc++abi/libunwind CI run on this patch? I thought it should, but if not, we can get that setup. Otherwise, I think this is a gentle reminder for folks not to ignore CI failures :-)

In D139368#3981594, @ldionne wrote:

Did the libc++/libc++abi/libunwind CI run on this patch? I thought it should, but if not, we can get that setup. Otherwise, I think this is a gentle reminder for folks not to ignore CI failures :-)

I don't know a libunwind CI. I personally checked the output is the same with multiple architectures, x86_64, aarch64, arm, riscv64, ppc32 linux, ppc64 linux, etc, but I couldn't have known that this did with AIX.
Having a CI set up will provide some value but likely not very useful for many libunwind changes, but nevertheless, it is good if it can check what AIX and z/OS do since most contributors don't have access to the two.

In D139368#3978330, @daltenty wrote:

This broke the AIX CI: https://buildkite.com/llvm-project/libcxx-ci/builds/15656#0184eac6-521b-4d1e-8502-c226673e0ece. Unfortunately it seems the .irp directive isn't supported by the AIX assembler.

What chunks of code do AIX use? All __powerpc__ and __powerpc64__ with all variants including __VSX__?

MaskRay added a commit: rG356bbbbda9d0: [libunwind] Use .irp directives. NFC.Dec 14 2022, 12:24 AM

MaskRay edited the summary of this revision. (Show Details)Dec 14 2022, 12:24 AM

Herald added a subscriber: steven.zhang. · View Herald TranscriptDec 14 2022, 12:24 AM

In D139368#3994092, @MaskRay wrote:

In D139368#3978330, @daltenty wrote:

This broke the AIX CI: https://buildkite.com/llvm-project/libcxx-ci/builds/15656#0184eac6-521b-4d1e-8502-c226673e0ece. Unfortunately it seems the .irp directive isn't supported by the AIX assembler.

What chunks of code do AIX use? All __powerpc__ and __powerpc64__ with all variants including __VSX__?

Both chunks under __powerpc__ (32-bit mode) and __powerpc64__ (64-bit mode) are used for AIX. For __powerpc__, code guarded by __ALTIVEC__ is used. For __powerpc64__, code guarded by __VSX__ is used.