Skip to content

DAG: Fix asserting on invalid inline asm constraints #95935

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 1 commit into from
Jun 18, 2024

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jun 18, 2024

No description provided.

@arsenm arsenm added the llvm:SelectionDAG SelectionDAGISel as well label Jun 18, 2024 — with Graphite App
Copy link
Contributor Author

arsenm commented Jun 18, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @arsenm and the rest of your teammates on Graphite Graphite

@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2024

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-selectiondag

Author: Matt Arsenault (arsenm)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+6-3)
  • (added) llvm/test/CodeGen/AMDGPU/invalid-inline-asm-constraint-crash.ll (+52)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 453bc7129b5b0..3f5d1a5047a42 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -10023,9 +10023,12 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
         break;
       }
 
-      assert((OpInfo.ConstraintType == TargetLowering::C_RegisterClass ||
-              OpInfo.ConstraintType == TargetLowering::C_Register) &&
-             "Unknown constraint type!");
+      if (OpInfo.ConstraintType != TargetLowering::C_RegisterClass &&
+          OpInfo.ConstraintType != TargetLowering::C_Register) {
+        emitInlineAsmError(Call, "unknown asm constraint '" +
+                                     Twine(OpInfo.ConstraintCode) + "'");
+        return;
+      }
 
       // TODO: Support this.
       if (OpInfo.isIndirect) {
diff --git a/llvm/test/CodeGen/AMDGPU/invalid-inline-asm-constraint-crash.ll b/llvm/test/CodeGen/AMDGPU/invalid-inline-asm-constraint-crash.ll
new file mode 100644
index 0000000000000..ec58686630bf2
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/invalid-inline-asm-constraint-crash.ll
@@ -0,0 +1,52 @@
+; RUN: not llc -march=amdgcn < %s 2>&1 | FileCheck -check-prefix=ERR %s
+
+; ERR: error: couldn't allocate output register for constraint 'q'
+define void @crash_use_invalid_output_constraint_block(ptr addrspace(1) %arg) {
+bb:
+  %v = call i32 asm sideeffect "", "=q"()
+  br label %use
+
+use:
+  store i32 %v, ptr addrspace(1) %arg
+  ret void
+}
+
+; ERR: error: unknown asm constraint 'q'
+define void @invalid_input_constraint() {
+  call void asm sideeffect "", "q"(i32 1)
+  ret void
+}
+
+; ERR: error: unknown asm constraint 'q'
+define void @invalid_input_constraint_multi() {
+  call void asm sideeffect "", "q,q"(i32 1, i16 2)
+  ret void
+}
+
+; ERR: error: unknown asm constraint 'q'
+define void @invalid_input_constraint_multi_valid() {
+  call void asm sideeffect "", "q,v"(i32 1, i64 2)
+  ret void
+}
+
+; ERR: error: couldn't allocate output register for constraint 'q'
+define void @crash_use_invalid_output_constraint_block_multi(ptr addrspace(1) %arg) {
+bb:
+  %v = call { i32, i32 } asm sideeffect "", "=q,=q"()
+  br label %use
+
+use:
+  store { i32, i32 } %v, ptr addrspace(1) %arg
+  ret void
+}
+
+; ERR: error: couldn't allocate output register for constraint 'q'
+define void @crash_use_invalid_output_constraint_block_multi_valid(ptr addrspace(1) %arg) {
+bb:
+  %v = call { i32, i32 } asm sideeffect "", "=q,=v"()
+  br label %use
+
+use:
+  store { i32, i32 } %v, ptr addrspace(1) %arg
+  ret void
+}

@arsenm arsenm marked this pull request as ready for review June 18, 2024 14:41
@rengolin
Copy link
Member

This makes sense to me.

@arsenm arsenm merged commit 62b5196 into main Jun 18, 2024
10 of 11 checks passed
@arsenm arsenm deleted the users/arsenm/dag-no-assert-invalid-asm-constraint branch June 18, 2024 16:51
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants