This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] Use .irp directives. NFC
ClosedPublic

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

Details

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

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
MaskRay requested review of this revision.Dec 5 2022, 2:03 PM
MaskRay updated this revision to Diff 480332.Dec 5 2022, 10:11 PM

update more

compnerd accepted this revision.Dec 6 2022, 9:50 AM
compnerd added a subscriber: compnerd.

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
MaskRay added a comment.EditedDec 6 2022, 10:49 AM

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
This revision was automatically updated to reflect the committed changes.

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.

ldionne added a subscriber: ldionne.Dec 8 2022, 8:10 AM

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 :-)

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.

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 edited the summary of this revision. (Show Details)Dec 14 2022, 12:24 AM

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.