[llvm] [SROA] Fix incorrect offsets for structured binding variables (PR #69007)

Tue Oct 17 05:02:41 PDT 2023

================
@@ -0,0 +1,255 @@
+; RUN: opt %s -passes=sroa -S | FileCheck %s --check-prefixes=COMMON,OLD
+; RUN: opt %s -passes=declare-to-assign,sroa -S | FileCheck %s --check-prefixes=COMMON,NEW
+
+;; C++17 source:
+;; struct two { int a, b; } gt;
+;; int fun1() {
+;;   auto [x, y] = gt;
+;;   return x + y;
+;; }
+;;
+;; struct four { two a, b; } gf;
+;; int fun2() {
+;;   auto [x, y] = gf;
+;;   return x.a + y.b;
+;; }
+;; Plus some hand-written IR.
+;;
+;; Check that SROA understands how to split dbg.declares and dbg.assigns with
+;; offsets into their storge (e.g., the second variable in a structured binding
+;; is stored at an offset into the shared alloca).
+;;
+;; Additional notes:
+;; We expect the same dbg.value intrinsics to come out of SROA whether assignment
+;; tracking is enabled or not. However, the order of the debug intrinsics may
+;; differ, and assignment tracking replaces some dbg.declares with dbg.assigns.
+;;
+;; Structured bindings produce an artificial variable that covers the entire
+;; alloca. Although they add clutter to the test, they've been preserved in
+;; order to increase coverage. These use the placehold name 'A' in comments and
+;; checks.
+
+%struct.two = type { i32, i32 }
+%struct.four = type { %struct.two, %struct.two }
+
+ at gt = dso_local global %struct.two zeroinitializer, align 4, !dbg !0
+ at gf = dso_local global %struct.four zeroinitializer, align 4, !dbg !5
+
+
+; COMMON-LABEL: @_Z4fun1v
+; COMMON-NEXT: entry
+;; 32 bit variable x (!27): value a_reg.
+;;
+;; 32 bit variable y (!28): value b_reg.
+;;
+;; 64 bit variable A (!29) bits [0,  32): value a_reg.
+;; 64 bit variable A (!29) bits [32, 64): value b_reg.
+
+; OLD-NEXT: %[[a_reg:.*]] = load i32, ptr @gt
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[a_reg]], metadata ![[x0:[0-9]+]], metadata !DIExpression())
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[a_reg]], metadata ![[A0:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
+; OLD-NEXT: %[[b_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gt, i64 4)
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[b_reg]], metadata ![[y0:[0-9]+]], metadata !DIExpression())
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[b_reg]], metadata ![[A0]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32))
+
+; NEW-NEXT: %[[a_reg:.*]] = load i32, ptr @gt
+; NEW-NEXT: %[[b_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gt, i64 4)
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[b_reg]], metadata ![[y0:[0-9]+]], metadata !DIExpression())
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[a_reg]], metadata ![[A0:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[b_reg]], metadata ![[A0]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32))
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[a_reg]], metadata ![[x0:[0-9]+]], metadata !DIExpression())
+define dso_local noundef i32 @_Z4fun1v() #0 !dbg !23 {
+entry:
+  %0 = alloca %struct.two, align 4
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !27, metadata !DIExpression()), !dbg !31
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !28, metadata !DIExpression(DW_OP_plus_uconst, 4)), !dbg !31
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !29, metadata !DIExpression()), !dbg !31
+  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %0, ptr align 4 @gt, i64 8, i1 false), !dbg !31
+  %a = getelementptr inbounds %struct.two, ptr %0, i32 0, i32 0, !dbg !31
+  %1 = load i32, ptr %a, align 4, !dbg !31
+  %b = getelementptr inbounds %struct.two, ptr %0, i32 0, i32 1, !dbg !31
+  %2 = load i32, ptr %b, align 4, !dbg !31
+  %add = add nsw i32 %1, %2, !dbg !31
+  ret i32 %add, !dbg !31
+}
+
+; COMMON-LABEL: _Z4fun2v()
+; COMMON-NEXT: entry:
+;; 64 bit variable x (!50) bits [0,  32): value aa_reg.
+;; 64 bit variable x (!50) bits [32, 64): deref ab_ba_addr
+;;
+;; 64 bit variable y (!51) bits [0,  32): deref ab_ba_addr + 32
+;; 64 bit variable y (!51) bits [32, 64): value bb_reg.
+;;
+;; 128 bit variable A (!52) bits [0,   32): value aa_reg
+;; 128 bit variable A (!52) bits [32,  64): deref ab_ba_addr
+;; 128 bit variable A (!52) bits [96, 128): value bb_reg
+;;
+;; NOTE: This 8 byte alloca contains x.b (4 bytes) and y.a (4 bytes).
+; COMMON-NEXT: %[[ab_ba_addr:.*]] = alloca [8 x i8], align 4
+; OLD-NEXT: llvm.dbg.declare(metadata ptr %[[ab_ba_addr]], metadata ![[A1:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 64))
+; OLD-NEXT: llvm.dbg.declare(metadata ptr %[[ab_ba_addr]], metadata ![[y1:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 4, DW_OP_LLVM_fragment, 0, 32))
+; OLD-NEXT: llvm.dbg.declare(metadata ptr %[[ab_ba_addr]], metadata ![[x1:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32))
+; OLD-NEXT: %[[aa_reg:.*]] = load i32, ptr @gf, align 4
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[aa_reg]], metadata ![[x1]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[aa_reg]], metadata ![[A1]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
+; OLD-NEXT: call void @llvm.memcpy{{.*}}(ptr align 4 %[[ab_ba_addr]], ptr align 4 getelementptr inbounds (i8, ptr @gf, i64 4), i64 8, i1 false)
+; OLD-NEXT: %[[bb_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gf, i64 12), align 4
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[bb_reg]], metadata ![[y1]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32))
+; OLD-NEXT: llvm.dbg.value(metadata i32 %[[bb_reg]], metadata ![[A1]], metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32))
+
+; NEW-NEXT: llvm.dbg.assign(metadata i1 undef, metadata ![[x1:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32), metadata ![[#]], metadata ptr %[[ab_ba_addr]], metadata !DIExpression())
+; NEW-NEXT: llvm.dbg.assign(metadata i1 undef, metadata ![[A1:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 64), metadata ![[#]], metadata ptr %[[ab_ba_addr]], metadata !DIExpression())
+; NEW-NEXT: llvm.dbg.declare(metadata ptr %[[ab_ba_addr]], metadata ![[y1:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 4, DW_OP_LLVM_fragment, 0, 32))
+; NEW-NEXT: %[[aa_reg:.*]] = load i32, ptr @gf, align 4
+; NEW-NEXT: llvm.memcpy{{.*}}(ptr align 4 %[[ab_ba_addr]], ptr align 4 getelementptr inbounds (i8, ptr @gf, i64 4), i64 8, i1 false){{.*}}, !DIAssignID ![[ID:[0-9]+]]
+; NEW-NEXT: %[[bb_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gf, i64 12), align 4
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[bb_reg]], metadata ![[y1:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32))
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[aa_reg]], metadata ![[A1]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
+; NEW-NEXT: llvm.dbg.assign(metadata i1 undef, metadata ![[A1]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 64), metadata ![[ID]], metadata ptr %[[ab_ba_addr]], metadata !DIExpression())
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[bb_reg]], metadata ![[A1]], metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32))
+; NEW-NEXT: llvm.dbg.value(metadata i32 %[[aa_reg]], metadata ![[x1]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
+define dso_local noundef i32 @_Z4fun2v() #0 !dbg !48 {
+entry:
+  %0 = alloca %struct.four, align 4
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !50, metadata !DIExpression()), !dbg !54
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !51, metadata !DIExpression(DW_OP_plus_uconst, 8)), !dbg !54
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !52, metadata !DIExpression()), !dbg !54
+  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %0, ptr align 4 @gf, i64 16, i1 false), !dbg !54
+  %a = getelementptr inbounds %struct.four, ptr %0, i32 0, i32 0, !dbg !54
+  %a1 = getelementptr inbounds %struct.two, ptr %a, i32 0, i32 0, !dbg !54
+  %1 = load i32, ptr %a1, align 4, !dbg !54
+  %b = getelementptr inbounds %struct.four, ptr %0, i32 0, i32 1, !dbg !54
+  %b2 = getelementptr inbounds %struct.two, ptr %b, i32 0, i32 1, !dbg !54
+  %2 = load i32, ptr %b2, align 4, !dbg !54
+  %add = add nsw i32 %1, %2, !dbg !54
+  ret i32 %add, !dbg !54
+}
+
+;; Hand-written part to test what happens when variables are smaller than the
+;; new alloca slices (i.e., check offset rewriting works correctly).  Note that
+;; mem2reg incorrectly preserves the offest in the DIExpression a variable
+;; stuffed into the upper bits of a value (that is a bug), e.g. alloca+offset
+;; becomes vreg+offest. It should either convert the offest to a shift, or
----------------
jmorse wrote:

Awkwardly, I think this wants something in all-caps with "FIXME" or similar, to draw attention to the fact that we test for incorrect behaviour. That avoids someone who's trying to work out what the correct behaviour is glazing over as they read the comments and not noticing the intention.

https://github.com/llvm/llvm-project/pull/69007
