This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][InlineAsm]Add Clang support for flag output constraints
ClosedPublic

Authored by mingmingl on Apr 24 2023, 10:55 PM.

Details

Summary
  • Mention this change in Clang release notes.

Before:

After:

  • For aarch64 targets (with __aarch64__ defined), Clang validates and parses flag output constraints to generate LLVM IR.

Diff Detail

Event Timeline

mingmingl created this revision.Apr 24 2023, 10:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2023, 10:55 PM
mingmingl requested review of this revision.Apr 24 2023, 10:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2023, 10:55 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
mingmingl edited the summary of this revision. (Show Details)

remove 'No newline at end of file'

While testing this patch with ./bin/clang -cc1 -S -triple=aarch64 inline-asm-aarch64-flag-output.c (which invokes global-isel for instruction selection according to print-after-all output), turns out GlobalISel doesn't support flag output yet (for x86 or aarch64).

/bin/clang -cc1 -O1 -S -triple=aarch64 file.c and /bin/clang -cc1 -O2 -S -triple=aarch64 file.c works.

I filed https://github.com/llvm/llvm-project/issues/62343 to track but but think the issue is a non-blocker of this patch and D149032 for now.

davidxl added inline comments.
clang/lib/Basic/Targets/AArch64.cpp
1216

Name is not modified in this method, so perhaps dropping '&'?

clang/lib/Basic/Targets/AArch64.cpp
1216

Yeah, looks like this was copied from D57394. Probably both places should be fixed.

A char *& is a code smell.

1310

please don't use auto here.

mingmingl marked 3 inline comments as done.
mingmingl edited the summary of this revision. (Show Details)

update on the crashing issue (solved)

resolve comments.

While testing this patch with ./bin/clang -cc1 -S -triple=aarch64 inline-asm-aarch64-flag-output.c (which invokes global-isel for instruction selection according to print-after-all output), turns out GlobalISel doesn't support flag output yet (for x86 or aarch64).

/bin/clang -cc1 -O1 -S -triple=aarch64 file.c and /bin/clang -cc1 -O2 -S -triple=aarch64 file.c works.

I filed https://github.com/llvm/llvm-project/issues/62343 to track but but think the issue is a non-blocker of this patch and D149032 for now.

After relaxing the assert condition in GlobalISel/InlineAsmLowering.cpp in the parent patch, the original crash (assertion error in global-isel) command generates valid asm outputs (the reason is that global-isel will fall back to selection-dag based isel when it couldn't select or lower an instruction), The error seen in issue 62343 could be worked around with the fallback approach.

Mentioned this change in clang release notes, and resolve comments.

clang/lib/Basic/Targets/AArch64.cpp
1216

thanks for the catch! will fix the x86 cpp code in a tiny separate patch later.

1310

s/auto/const unsiged, here and above.

nickdesaulniers accepted this revision.Apr 26 2023, 10:57 AM

Thanks for the patch!

This revision is now accepted and ready to land.Apr 26 2023, 10:57 AM
This revision was landed with ongoing or failed builds.Apr 27 2023, 9:39 AM
This revision was automatically updated to reflect the committed changes.