Skip to content

[InitUndef] Also handle inline asm #108951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Sep 17, 2024

InitUndef should also handle early-clobber / undef conflicts in inline asm operands. Do this by iterating over all_defs() instead of defs().

The newly added test was generating an "unpredictable STXP instruction, status is also a source" error prior to this change.

Fixes #106380.

InitUndef should also handle early-clobber / undef conflicts in
inline asm operands. Do this by iterating over all_defs() instead
of defs().

The newly added test was generating an "unpredictable STXP instruction,
status is also a source" error prior to this change.

Fixes llvm#106380.
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Nikita Popov (nikic)

Changes

InitUndef should also handle early-clobber / undef conflicts in inline asm operands. Do this by iterating over all_defs() instead of defs().

The newly added test was generating an "unpredictable STXP instruction, status is also a source" error prior to this change.

Fixes #106380.


Full diff: https://github.com/llvm/llvm-project/pull/108951.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/InitUndef.cpp (+1-1)
  • (modified) llvm/test/CodeGen/AArch64/arm64-ldxr-stxr.ll (+13)
diff --git a/llvm/lib/CodeGen/InitUndef.cpp b/llvm/lib/CodeGen/InitUndef.cpp
index 911e8bb7a4d9ef..d4ac131a32a959 100644
--- a/llvm/lib/CodeGen/InitUndef.cpp
+++ b/llvm/lib/CodeGen/InitUndef.cpp
@@ -98,7 +98,7 @@ INITIALIZE_PASS(InitUndef, DEBUG_TYPE, INIT_UNDEF_NAME, false, false)
 char &llvm::InitUndefID = InitUndef::ID;
 
 static bool isEarlyClobberMI(MachineInstr &MI) {
-  return llvm::any_of(MI.defs(), [](const MachineOperand &DefMO) {
+  return llvm::any_of(MI.all_defs(), [](const MachineOperand &DefMO) {
     return DefMO.isReg() && DefMO.isEarlyClobber();
   });
 }
diff --git a/llvm/test/CodeGen/AArch64/arm64-ldxr-stxr.ll b/llvm/test/CodeGen/AArch64/arm64-ldxr-stxr.ll
index 4fb0c2775a7a7a..b498611242d469 100644
--- a/llvm/test/CodeGen/AArch64/arm64-ldxr-stxr.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-ldxr-stxr.ll
@@ -364,6 +364,19 @@ define dso_local i32 @test_stxp_undef(ptr %p, i64 %x) nounwind {
   ret i32 %res
 }
 
+; Same as previous test, but using inline asm.
+define dso_local i32 @test_stxp_undef_inline_asm(ptr %p, i64 %x) nounwind {
+; CHECK-LABEL: test_stxp_undef_inline_asm:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    //APP
+; CHECK-NEXT:    stxp w8, x9, x1, [x0]
+; CHECK-NEXT:    //NO_APP
+; CHECK-NEXT:    mov w0, w8
+; CHECK-NEXT:    ret
+  %res = call i32 asm sideeffect "stxp ${0:w}, ${2}, ${3}, [${1}]", "=&r,r,r,r,~{memory}"(ptr %p, i64 undef, i64 %x)
+  ret i32 %res
+}
+
 declare i32 @llvm.aarch64.stlxr.p0(i64, ptr) nounwind
 ;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
 ; FALLBACK: {{.*}}

Copy link
Contributor

@Stylie777 Stylie777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get some tests that cover the inline asm cases for Arm? Something testing the vcaddq similar to those added in #77770 would be great.

@nikic
Copy link
Contributor Author

nikic commented Sep 18, 2024

@Stylie777 I've added one for vcaddq. I confirmed that it previously allocated q0, q0, q0 and generated an "Qd register and Qm register can't be identical" error.

Copy link
Contributor

@Stylie777 Stylie777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arm/AArch64 Changes LGTM.

Copy link
Contributor

@BeMg BeMg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikic nikic merged commit 7183771 into llvm:main Sep 19, 2024
8 checks passed
@nikic nikic deleted the init-undef-inline-asm branch September 19, 2024 07:59
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
InitUndef should also handle early-clobber / undef conflicts in inline
asm operands. Do this by iterating over all_defs() instead of defs().

The newly added ARM test was generating an "unpredictable STXP instruction,
status is also a source" error prior to this change.

Fixes llvm#106380.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AArch64] Overlapping registers allocated to input and lateout constraints
4 participants