[SDAG] Avoid crash when creating debug fragments for scalable vectors

[MacDue]

Previously, we would crash in the SelectionDAGBuilder when attempting to create debug fragments for scalable vectors split across multiple registers.

It does not seem like DW_OP_LLVM_fragment supports any notion of scalable type sizes. It takes both an offset and typesize as literals, with no indication of scalability (and it also does not seem to be considered in any of the places that handle DW_OP_LLVM_fragment). So the workaround here is to drop the debug info.

Note: This is not usually an issue for IR that comes from the SVE ACLE, as we generally stick to using legal types there (that don't end up getting split).

Workaround for: #161289

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-aarch64

Author: Benjamin Maxwell (MacDue)

Changes

Previously, we would crash in the SelectionDAGBuilder when attempting to create debug fragments for scalable vectors split across multiple registers.

It does not seem like DW_OP_LLVM_fragment supports any notion of scalable type sizes. It takes both an offset and typesize as literals, with no indication of scalability (and it also does not seem to be considered in any of the places that handle DW_OP_LLVM_fragment). So the workaround here is to drop the debug info.

Note: This is not usually an issue for IR that comes from the SVE ACLE, as we generally stick to using legal types there (that don't end up getting split).

Workaround for: #161289

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+16-14)
• (added) llvm/test/CodeGen/AArch64/pr161289.ll (+48)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index a52265055c88a..c31638c2024c2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -6215,14 +6215,18 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
 
   if (!Op) {
     // Create a DBG_VALUE for each decomposed value in ArgRegs to cover Reg
-    auto splitMultiRegDbgValue = [&](ArrayRef<std::pair<Register, TypeSize>>
-                                         SplitRegs) {
+    auto splitMultiRegDbgValue =
+        [&](ArrayRef<std::pair<Register, TypeSize>> SplitRegs) -> bool {
       unsigned Offset = 0;
-      for (const auto &RegAndSize : SplitRegs) {
+      for (const auto [Reg, RegSizeInBits] : SplitRegs) {
+        // FIXME: Scalable sizes are not supported in fragment expressions.
+        if (RegSizeInBits.isScalable())
+          return false;
+
         // If the expression is already a fragment, the current register
         // offset+size might extend beyond the fragment. In this case, only
         // the register bits that are inside the fragment are relevant.
-        int RegFragmentSizeInBits = RegAndSize.second;
+        int RegFragmentSizeInBits = RegSizeInBits.getFixedValue();
         if (auto ExprFragmentInfo = Expr->getFragmentInfo()) {
           uint64_t ExprFragmentSizeInBits = ExprFragmentInfo->SizeInBits;
           // The register is entirely outside the expression fragment,
@@ -6238,7 +6242,7 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
 
         auto FragmentExpr = DIExpression::createFragmentExpression(
             Expr, Offset, RegFragmentSizeInBits);
-        Offset += RegAndSize.second;
+        Offset += RegSizeInBits.getFixedValue();
         // If a valid fragment expression cannot be created, the variable's
         // correct value cannot be determined and so it is set as poison.
         if (!FragmentExpr) {
@@ -6247,11 +6251,12 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
           DAG.AddDbgValue(SDV, false);
           continue;
         }
-        MachineInstr *NewMI =
-            MakeVRegDbgValue(RegAndSize.first, *FragmentExpr,
-                             Kind != FuncArgumentDbgValueKind::Value);
+        MachineInstr *NewMI = MakeVRegDbgValue(
+            Reg, *FragmentExpr, Kind != FuncArgumentDbgValueKind::Value);
         FuncInfo.ArgDbgValues.push_back(NewMI);
       }
+
+      return true;
     };
 
     // Check if ValueMap has reg number.
@@ -6261,18 +6266,15 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
       const auto &TLI = DAG.getTargetLoweringInfo();
       RegsForValue RFV(V->getContext(), TLI, DAG.getDataLayout(), VMI->second,
                        V->getType(), std::nullopt);
-      if (RFV.occupiesMultipleRegs()) {
-        splitMultiRegDbgValue(RFV.getRegsAndSizes());
-        return true;
-      }
+      if (RFV.occupiesMultipleRegs())
+        return splitMultiRegDbgValue(RFV.getRegsAndSizes());
 
       Op = MachineOperand::CreateReg(VMI->second, false);
       IsIndirect = Kind != FuncArgumentDbgValueKind::Value;
     } else if (ArgRegsAndSizes.size() > 1) {
       // This was split due to the calling convention, and no virtual register
       // mapping exists for the value.
-      splitMultiRegDbgValue(ArgRegsAndSizes);
-      return true;
+      return splitMultiRegDbgValue(ArgRegsAndSizes);
     }
   }
 
diff --git a/llvm/test/CodeGen/AArch64/pr161289.ll b/llvm/test/CodeGen/AArch64/pr161289.ll
new file mode 100644
index 0000000000000..fa8cec40832ec
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/pr161289.ll
@@ -0,0 +1,48 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -verify-machineinstrs < %s -mattr=+sme | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; FIXME: This is from https://github.com/llvm/llvm-project/issues/161289. The argument (%arg) is <vscale x 16 x half>,
+; which is passed as two <vscale x 8 x half> registers. LLVM wants to describe this as two DW_OP_LLVM_fragment fragments,
+; but currently, that opcode has no notion of scalable type sizes.
+;
+; We are currently working around this by dropping the debug info for %arg (so it is 'undef' below).
+
+define <vscale x 16 x half> @scalable_vector_debug_info(<vscale x 16 x half> %arg) "aarch64_pstate_sm_enabled" !dbg !7 {
+; CHECK-LABEL: scalable_vector_debug_info:
+; CHECK:       .Lfunc_begin0:
+; CHECK-NEXT:    .file 1 "/tmp" "scalable-vector-debug.c"
+; CHECK-NEXT:    .loc 1 1 0 // scalable-vector-debug.c:1:0
+; CHECK-NEXT:    .cfi_startproc
+; CHECK-NEXT:  // %bb.0:
+; CHECK-NEXT:    //DEBUG_VALUE: scalable_vector_debug_info:arg <- undef
+; CHECK-NEXT:    .loc 1 1 0 prologue_end // scalable-vector-debug.c:1:0
+; CHECK-NEXT:    ret
+    #dbg_value(<vscale x 16 x half> %arg, !13, !DIExpression(), !14)
+  %extract = tail call <vscale x 8 x half> @llvm.vector.extract.nxv8f16.nxv16f16(<vscale x 16 x half> %arg, i64 0)
+  ret <vscale x 16 x half> %arg
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "scalable-vector-debug.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"frame-pointer", i32 2}
+!7 = distinct !DISubprogram(name: "scalable_vector_debug_info", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !12)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !10}
+!10 = !DIBasicType(name: "svfloat16_t", size: 16, encoding: DW_ATE_float)
+!11 = !DIBasicType(name: "__fp16", size: 16, encoding: DW_ATE_float)
+!12 = !{!13}
+!13 = !DILocalVariable(name: "arg", arg: 1, scope: !7, file: !1, line: 1, type: !10)
+!14 = !DILocation(line: 0, scope: !7)
+!15 = distinct !DISubprogram(name: "streaming_sve_debug_info", scope: !1, file: !1, line: 5, type: !8, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !18)
+!16 = !DILocalVariable(name: "arg", arg: 1, scope: !15, file: !1, line: 5, type: !10)
+!17 = !DILocation(line: 0, scope: !15)
+!18 = !{!16}

[MacDue]

Hidden the 'undef' warning as it's a false-positive due to it matching a comment.

[aemerson]

Thanks!

[sdesmalen-arm]

Clang no longer generates LLVM IR using wide vector types, but rather represents each tuple as individual arguments that are combined together using insertvalues. This means recent versions of Clang no longer have this issue. If this is about a downstream version of Clang then the better solution would be to work around the issue downstream rather than introducing this workaround here in SelectionDAG. We shouldn't be relying on LLVM being able to handle illegal types in the ABI.

[aemerson]

That may be the case for clang, but not every user of SME is going to be adhering to the AAPCS. That's a platform level decision and not a restriction on the IR semantics. LLVM shouldn't crash on any legal IR input, and otherwise the IR verifier should reject it as malformed.