[llvm] b9ca73e - [DebugInfo] Correctly handle arrays with 0-width elements in GEP salvaging

[Stephen Tozer via llvm-commits]

Author: Stephen Tozer
Date: 2021-10-18T12:01:12+01:00
New Revision: b9ca73e1a8fd0c018b0b3eb313163da2b4ca4e09

URL: https://github.com/llvm/llvm-project/commit/b9ca73e1a8fd0c018b0b3eb313163da2b4ca4e09
DIFF: https://github.com/llvm/llvm-project/commit/b9ca73e1a8fd0c018b0b3eb313163da2b4ca4e09.diff

LOG: [DebugInfo] Correctly handle arrays with 0-width elements in GEP salvaging

Fixes an issue where GEP salvaging did not properly account for GEP
instructions which stepped over array elements of width 0 (effectively a
no-op). This unnecessarily produced long expressions by appending
`... + (x * 0)` and potentially extended the number of SSA values used
in the dbg.value. This also erroneously triggered an assert in the
salvage function that the element width would be strictly positive.
These issues are resolved by simply ignoring these useless operands.

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D111809

Added: 
    

Modified: 
    llvm/lib/IR/Operator.cpp
    llvm/test/DebugInfo/salvage-gep.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Operator.cpp b/llvm/lib/IR/Operator.cpp
index 18a1c84933e0a..f9dbed31c44c6 100644
--- a/llvm/lib/IR/Operator.cpp
+++ b/llvm/lib/IR/Operator.cpp
@@ -190,12 +190,14 @@ bool GEPOperator::collectOffset(
 
     if (STy || ScalableType)
       return false;
-    // Insert an initial offset of 0 for V iff none exists already, then
-    // increment the offset by IndexedSize.
-    VariableOffsets.insert({V, APInt(BitWidth, 0)});
     APInt IndexedSize =
         APInt(BitWidth, DL.getTypeAllocSize(GTI.getIndexedType()));
-    VariableOffsets[V] += IndexedSize;
+    // Insert an initial offset of 0 for V iff none exists already, then
+    // increment the offset by IndexedSize.
+    if (!IndexedSize.isZero()) {
+      VariableOffsets.insert({V, APInt(BitWidth, 0)});
+      VariableOffsets[V] += IndexedSize;
+    }
   }
   return true;
 }

diff  --git a/llvm/test/DebugInfo/salvage-gep.ll b/llvm/test/DebugInfo/salvage-gep.ll
index 6c31b0ff61de6..24db8b7d21f25 100644
--- a/llvm/test/DebugInfo/salvage-gep.ll
+++ b/llvm/test/DebugInfo/salvage-gep.ll
@@ -1,23 +1,35 @@
 ; RUN: opt %s -dce -S | FileCheck %s
 
-; Tests the salvaging of GEP instructions, specifically struct indexing and
-; non-constant array indexing.
+; Tests the salvaging of GEP instructions, specifically struct indexing,
+; non-constant array indexing, and non-constant array indexing into an array of
+; a type with width 0.
 
 %struct.S = type { i32, i32 }
+%zero = type [0 x [10 x i32]]
 
+;; The constant and variable offsets should be applied correctly.
 ; CHECK: call void @llvm.dbg.value(metadata !DIArgList(%struct.S* %ptr, i64 %offset),
 ; CHECK-SAME: ![[VAR_OFFSET_PTR:[0-9]+]],
 ; CHECK-SAME: !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 8, DW_OP_mul, DW_OP_plus, DW_OP_plus_uconst, 4, DW_OP_stack_value))
 
+;; The variable offset should be ignored, as it applies to a type of width 0,
+;; leaving only the constant offset.
+; CHECK: call void @llvm.dbg.value(metadata [0 x [10 x i32]]* %zptr,
+; CHECK-SAME: ![[VAR_ZERO_PTR:[0-9]+]],
+; CHECK-SAME: !DIExpression(DW_OP_plus_uconst, 44, DW_OP_stack_value))
+
 ; CHECK: ![[VAR_OFFSET_PTR]] = !DILocalVariable(name: "offset_ptr"
+; CHECK: ![[VAR_ZERO_PTR]] = !DILocalVariable(name: "zero_ptr"
 
-define void @"?foo@@YAXPEAUS@@_J at Z"(%struct.S* %ptr, i64 %offset) !dbg !8 {
+define void @"?foo@@YAXPEAUS@@_J at Z"(%struct.S* %ptr, %zero* %zptr, i64 %offset) !dbg !8 {
 entry:
   call void @llvm.dbg.value(metadata i64 %offset, metadata !20, metadata !DIExpression()), !dbg !24
   call void @llvm.dbg.value(metadata %struct.S* %ptr, metadata !21, metadata !DIExpression()), !dbg !24
   %arrayidx = getelementptr inbounds %struct.S, %struct.S* %ptr, i64 %offset, !dbg !25
   %b = getelementptr inbounds %struct.S, %struct.S* %arrayidx, i32 0, i32 1, !dbg !25
+  %c = getelementptr inbounds %zero, %zero* %zptr, i64 %offset, i32 1, i32 1, !dbg !25
   call void @llvm.dbg.value(metadata i32* %b, metadata !22, metadata !DIExpression()), !dbg !24
+  call void @llvm.dbg.value(metadata i32* %c, metadata !27, metadata !DIExpression()), !dbg !24
   ret void, !dbg !26
 }
 
@@ -54,3 +66,4 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
 !24 = !DILocation(line: 0, scope: !8)
 !25 = !DILocation(line: 8, scope: !8)
 !26 = !DILocation(line: 9, scope: !8)
+!27 = !DILocalVariable(name: "zero_ptr", scope: !8, file: !9, line: 8, type: !23)