[SelectionDAG] Fix assertion failure on inline asm register type mismatch #166615

ahmednoursphinx · 2025-11-05T18:53:06Z

Resolves #166057

…atch

llvmbot · 2025-11-05T18:53:43Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-selectiondag

Author: Ahmed Nour (ahmednoursphinx)

Changes

Resolves #166057

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

3 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+11-1)
(modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+2)
(added) llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll (+20)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index fa0c899dfcc27..9a6a76f73f8fe 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -365,7 +365,17 @@ static SDValue getCopyFromPartsVector(SelectionDAG &DAG, const SDLoc &DL,
 
     assert(NumRegs == NumParts && "Part count doesn't match vector breakdown!");
     NumParts = NumRegs; // Silence a compiler warning.
-    assert(RegisterVT == PartVT && "Part type doesn't match vector breakdown!");
+    
+    // Check if the register type matches the part type
+    if (RegisterVT != PartVT) {
+      diagnosePossiblyInvalidConstraint(
+          *DAG.getContext(), V,
+          "register type (" + EVT(RegisterVT).getEVTString() +
+              ") doesn't match operand type (" + EVT(PartVT).getEVTString() +
+              ")");
+      return DAG.getUNDEF(ValueVT);
+    }
+    
     assert(RegisterVT.getSizeInBits() ==
            Parts[0].getSimpleValueType().getSizeInBits() &&
            "Part type sizes don't match!");
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 6edf0185df813..bf432f1d56d4a 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -61825,6 +61825,8 @@ X86TargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
       break;
     case 'y':   // MMX_REGS if MMX allowed.
       if (!Subtarget.hasMMX()) break;
+      // MMX registers are 64-bit only
+      if (VT.getSizeInBits() != 64) break;
       return std::make_pair(0U, &X86::VR64RegClass);
     case 'v':
     case 'x':   // SSE_REGS if SSE1 allowed or AVX_REGS if AVX allowed
diff --git a/llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll b/llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll
new file mode 100644
index 0000000000000..9f4ed28a4d312
--- /dev/null
+++ b/llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll
@@ -0,0 +1,20 @@
+; RUN: not llc -mtriple=x86_64-unknown-linux-gnu < %s 2>&1 | FileCheck %s
+
+; Test that using MMX register constraint 'y' (64-bit) with a 256-bit vector
+; produces a proper error message instead of an assertion failure.
+
+; CHECK: error: couldn't allocate output register for constraint 'y'
+
+define <8 x i32> @test_mmx_constraint_size_mismatch() {
+entry:
+  %out = tail call <8 x i32> asm "something $0", "=y"()
+  ret <8 x i32> %out
+}
+
+; Also test with a different vector size
+define <4 x i32> @test_mmx_constraint_128bit() {
+entry:
+  %out = tail call <4 x i32> asm "something $0", "=y"()
+  ret <4 x i32> %out
+}
+

llvmbot · 2025-11-05T18:53:43Z

@llvm/pr-subscribers-backend-x86

Author: Ahmed Nour (ahmednoursphinx)

Changes

Resolves #166057

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

3 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+11-1)
(modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+2)
(added) llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll (+20)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index fa0c899dfcc27..9a6a76f73f8fe 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -365,7 +365,17 @@ static SDValue getCopyFromPartsVector(SelectionDAG &DAG, const SDLoc &DL,
 
     assert(NumRegs == NumParts && "Part count doesn't match vector breakdown!");
     NumParts = NumRegs; // Silence a compiler warning.
-    assert(RegisterVT == PartVT && "Part type doesn't match vector breakdown!");
+    
+    // Check if the register type matches the part type
+    if (RegisterVT != PartVT) {
+      diagnosePossiblyInvalidConstraint(
+          *DAG.getContext(), V,
+          "register type (" + EVT(RegisterVT).getEVTString() +
+              ") doesn't match operand type (" + EVT(PartVT).getEVTString() +
+              ")");
+      return DAG.getUNDEF(ValueVT);
+    }
+    
     assert(RegisterVT.getSizeInBits() ==
            Parts[0].getSimpleValueType().getSizeInBits() &&
            "Part type sizes don't match!");
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 6edf0185df813..bf432f1d56d4a 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -61825,6 +61825,8 @@ X86TargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
       break;
     case 'y':   // MMX_REGS if MMX allowed.
       if (!Subtarget.hasMMX()) break;
+      // MMX registers are 64-bit only
+      if (VT.getSizeInBits() != 64) break;
       return std::make_pair(0U, &X86::VR64RegClass);
     case 'v':
     case 'x':   // SSE_REGS if SSE1 allowed or AVX_REGS if AVX allowed
diff --git a/llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll b/llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll
new file mode 100644
index 0000000000000..9f4ed28a4d312
--- /dev/null
+++ b/llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll
@@ -0,0 +1,20 @@
+; RUN: not llc -mtriple=x86_64-unknown-linux-gnu < %s 2>&1 | FileCheck %s
+
+; Test that using MMX register constraint 'y' (64-bit) with a 256-bit vector
+; produces a proper error message instead of an assertion failure.
+
+; CHECK: error: couldn't allocate output register for constraint 'y'
+
+define <8 x i32> @test_mmx_constraint_size_mismatch() {
+entry:
+  %out = tail call <8 x i32> asm "something $0", "=y"()
+  ret <8 x i32> %out
+}
+
+; Also test with a different vector size
+define <4 x i32> @test_mmx_constraint_128bit() {
+entry:
+  %out = tail call <4 x i32> asm "something $0", "=y"()
+  ret <4 x i32> %out
+}
+

github-actions · 2025-11-05T18:54:57Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

arsenm · 2025-11-05T18:58:32Z

llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll

+; Test that using MMX register constraint 'y' (64-bit) with a 256-bit vector
+; produces a proper error message instead of an assertion failure.
+
+; CHECK: error: couldn't allocate output register for constraint 'y'


This isn't the tested error message

What would be the best way to test the specific diagnostic path? The register
allocator's validation runs before getCopyFromPartsVector, so the specific message
from the fix is only reached if earlier checks are bypassed.

No? The register allocation is very far removed and much later than the DAG construction where getCopyFromPartsVector is used

llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll

arsenm · 2025-11-05T18:58:57Z

llvm/test/CodeGen/X86/inline-asm-y-constraint-size-mismatch.ll

+}
+
+; Also test with a different vector size
+define <4 x i32> @test_mmx_constraint_128bit() {


Check errors in each instance

Can you please elaborate more about the instances we need to test? Currently we have two test cases covering different vector size mismatches: a 256-bit vector (<8 x i32>) and a 128-bit vector (<4 x i32>), both using the MMX 'y' constraint which expects 64-bit operands.

Each of these cases will emit an error, check all errors that are in stderr

phoebewang · 2025-11-06T06:54:06Z

This is not what I expected. We should do the check as early as in the front end. I assume we just need to add back

  bool IsMMXCons = llvm::StringSwitch<bool>(Constraint)
                     .Cases("y", "&y", "^Ym", true)
                     .Default(false);
  if (IsMMXCons && Ty->isVectorTy()) {
    if (cast<llvm::VectorType>(Ty)->getPrimitiveSizeInBits().getFixedValue() !=
        64) {
      // Invalid MMX constraint
      return nullptr;
    }
  }

in X86AdjustInlineAsmType which was removed in e59a619

ahmednoursphinx · 2025-11-06T10:02:02Z

This is not what I expected. We should do the check as early as in the front end. I assume we just need to add back
  bool IsMMXCons = llvm::StringSwitch<bool>(Constraint)
                     .Cases("y", "&y", "^Ym", true)
                     .Default(false);
  if (IsMMXCons && Ty->isVectorTy()) {
    if (cast<llvm::VectorType>(Ty)->getPrimitiveSizeInBits().getFixedValue() !=
        64) {
      // Invalid MMX constraint
      return nullptr;
    }
  }
in X86AdjustInlineAsmType which was removed in e59a619

Thanks @phoebewang i have reverted the frontend check , please check when you have time

ahmednoursphinx · 2025-11-06T10:03:39Z

Hey @arsenm i have updated the code based on @phoebewang recommendations , please have a look again when you have time

The size check in X86ISelLowering::getRegForInlineAsmConstraint was too restrictive. MMX registers can legitimately hold 32-bit integers (i32), not just 64-bit vectors. The frontend validation in X86AdjustInlineAsmType is sufficient for catching invalid vector sizes.

clang/test/CodeGen/X86/mmx-inline-asm-error.c

clang/lib/CodeGen/Targets/X86.cpp

ahmednoursphinx · 2025-11-06T11:30:13Z

@phoebewang updated please check when you have time

clang/test/CodeGen/X86/mmx-inline-asm-error.c

Co-authored-by: Phoebe Wang <phoebe.wang@intel.com>

clang/test/CodeGen/X86/mmx-inline-asm-error.c

phoebewang

LGTM.

ahmednoursphinx · 2025-11-09T09:43:49Z

Hey @RKSimon @phoebewang can you merge this please?

[SelectionDAG] Fix assertion failure on inline asm register type mism…

684e005

…atch

llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Nov 5, 2025

chore: Fix formatting

36a7461

arsenm reviewed Nov 5, 2025

View reviewed changes

PR Feedback

e81a714

ahmednoursphinx requested a review from arsenm November 6, 2025 06:05

Revert to frontend check

91650a4

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Nov 6, 2025

Fix formatting

f6bc33e

ahmednoursphinx added 2 commits November 6, 2025 12:44

Fix deprecation warning

c69c49c

phoebewang reviewed Nov 6, 2025

View reviewed changes

clang/test/CodeGen/X86/mmx-inline-asm-error.c Outdated Show resolved Hide resolved

phoebewang reviewed Nov 6, 2025

View reviewed changes

clang/lib/CodeGen/Targets/X86.cpp Outdated Show resolved Hide resolved

phoebewang reviewed Nov 6, 2025

View reviewed changes

clang/lib/CodeGen/Targets/X86.cpp Outdated Show resolved Hide resolved

ahmednoursphinx added 2 commits November 6, 2025 13:26

PR Feedback

44da3cc

Revert test file

3ec0ee3

ahmednoursphinx requested a review from phoebewang November 6, 2025 11:29

Update mmx-inline-asm-error.c

a56ed01

phoebewang reviewed Nov 6, 2025

View reviewed changes

clang/test/CodeGen/X86/mmx-inline-asm-error.c Outdated Show resolved Hide resolved

phoebewang reviewed Nov 6, 2025

View reviewed changes

clang/test/CodeGen/X86/mmx-inline-asm-error.c Outdated Show resolved Hide resolved

Update tests

5b74ede

ahmednoursphinx requested a review from phoebewang November 6, 2025 17:42

phoebewang reviewed Nov 7, 2025

View reviewed changes

clang/test/CodeGen/X86/mmx-inline-asm-error.c Show resolved Hide resolved

Remove =omp from the run line

5523696

ahmednoursphinx requested a review from phoebewang November 7, 2025 06:33

Merge branch 'main' into issue_166057

52cf73a

phoebewang reviewed Nov 7, 2025

View reviewed changes

clang/test/CodeGen/X86/mmx-inline-asm-error.c Outdated Show resolved Hide resolved

Add -verify

aa8cce6

ahmednoursphinx requested a review from phoebewang November 7, 2025 07:04

phoebewang reviewed Nov 7, 2025

View reviewed changes

clang/test/CodeGen/X86/mmx-inline-asm-error.c Outdated Show resolved Hide resolved

Update clang/test/CodeGen/X86/mmx-inline-asm-error.c

fb199a1

Co-authored-by: Phoebe Wang <phoebe.wang@intel.com>

ahmednoursphinx requested a review from phoebewang November 7, 2025 07:44

phoebewang reviewed Nov 7, 2025

View reviewed changes

clang/test/CodeGen/X86/mmx-inline-asm-error.c Outdated Show resolved Hide resolved

Add =omp back

ce0c563

ahmednoursphinx requested a review from phoebewang November 7, 2025 16:15

phoebewang approved these changes Nov 8, 2025

View reviewed changes

phoebewang merged commit cc3a505 into llvm:main Nov 9, 2025
10 checks passed

[SelectionDAG] Fix assertion failure on inline asm register type mismatch #166615

[SelectionDAG] Fix assertion failure on inline asm register type mismatch #166615

Conversation

ahmednoursphinx commented Nov 5, 2025

Uh oh!

llvmbot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 5, 2025

Uh oh!

github-actions bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

arsenm Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

ahmednoursphinx Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arsenm Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

ahmednoursphinx Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

phoebewang commented Nov 6, 2025

Uh oh!

ahmednoursphinx commented Nov 6, 2025

Uh oh!

ahmednoursphinx commented Nov 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahmednoursphinx commented Nov 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phoebewang left a comment

Choose a reason for hiding this comment

Uh oh!

ahmednoursphinx commented Nov 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

llvmbot commented Nov 5, 2025 •

edited

Loading

github-actions bot commented Nov 5, 2025 •

edited

Loading