[llvm] 30c1633 - Revert "[Instruction] Add updateLocationAfterHoist helper"

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 11 14:54:17 PDT 2020


Author: Vedant Kumar
Date: 2020-08-11T14:54:09-07:00
New Revision: 30c1633386e7cfb01c0a54b31ccf4c3a3873e71b

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

LOG: Revert "[Instruction] Add updateLocationAfterHoist helper"

This reverts commit 4a646ca9e2caf70d6312714770f516fb83b7e3cb.

This is causing some bots to fail with "!dbg attachment points at wrong
subprogram for function", like:

http://lab.llvm.org:8011/builders/sanitizer-windows/builds/67958/steps/stage%201%20check/logs/stdio

Added: 
    

Modified: 
    llvm/include/llvm/IR/Instruction.h
    llvm/lib/IR/DebugInfo.cpp
    llvm/lib/Transforms/Scalar/GVN.cpp
    llvm/lib/Transforms/Scalar/LICM.cpp
    llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll
    llvm/test/Transforms/GVN/PRE/phi-translate.ll
    llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll
    llvm/unittests/IR/InstructionsTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index c3021f7b77e0..a03eac0ad40d 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -492,10 +492,6 @@ class Instruction : public User,
   /// merged DebugLoc.
   void applyMergedLocation(const DILocation *LocA, const DILocation *LocB);
 
-  /// Updates the debug location given that the instruction has been hoisted
-  /// from a block to a predecessor of that block.
-  void updateLocationAfterHoist();
-
 private:
   /// Return true if we have an entry in the on-the-side metadata hash.
   bool hasMetadataHashEntry() const {

diff  --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 6eefaf69adc2..190b220dc9aa 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -696,24 +696,6 @@ void Instruction::applyMergedLocation(const DILocation *LocA,
   setDebugLoc(DILocation::getMergedLocation(LocA, LocB));
 }
 
-void Instruction::updateLocationAfterHoist() {
-  const DebugLoc &DL = getDebugLoc();
-  if (!DL)
-    return;
-
-  // If we didn't hoist a call, drop the location to allow a location from a
-  // preceding instruction to propagate.
-  if (!isa<CallBase>(this)) {
-    setDebugLoc(DebugLoc());
-    return;
-  }
-
-  // Set a line 0 location for (potentially inlinable) calls. Set the scope to
-  // the parent function's scope if it's available, and drop any inlinedAt info
-  // to avoid making it look like the inlined callee was reached early.
-  setDebugLoc(DebugLoc::get(0, 0, DL.getScope()));
-}
-
 //===----------------------------------------------------------------------===//
 // LLVM C API implementations.
 //===----------------------------------------------------------------------===//

diff  --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 249b4523b586..f51596525569 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -46,6 +46,7 @@
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
@@ -1313,7 +1314,8 @@ bool GVN::PerformLoadPRE(LoadInst *LI, AvailValInBlkVect &ValuesPerBlock,
     // Instructions that have been inserted in predecessor(s) to materialize
     // the load address do not retain their original debug locations. Doing
     // so could lead to confusing (but correct) source attributions.
-    I->updateLocationAfterHoist();
+    if (const DebugLoc &DL = I->getDebugLoc())
+      I->setDebugLoc(DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt()));
 
     // FIXME: We really _ought_ to insert these value numbers into their
     // parent's availability map.  However, in doing so, we risk getting into

diff  --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 0847e732b971..4bf39ba8f151 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1658,7 +1658,10 @@ static void hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
     // Move the new node to the destination block, before its terminator.
     moveInstructionBefore(I, *Dest->getTerminator(), *SafetyInfo, MSSAU, SE);
 
-  I.updateLocationAfterHoist();
+  // Apply line 0 debug locations when we are moving instructions to 
diff erent
+  // basic blocks because we want to avoid jumpy line tables.
+  if (const DebugLoc &DL = I.getDebugLoc())
+    I.setDebugLoc(DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt()));
 
   if (isa<LoadInst>(I))
     ++NumMovedLoads;

diff  --git a/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll b/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll
index 37c642170aa3..0a13e9604f0c 100644
--- a/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll
+++ b/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll
@@ -18,8 +18,9 @@
 ; We make sure that the instruction that is hoisted into the preheader
 ; does not have a debug location.
 ; CHECK: for.body.lr.ph:
-; CHECK: getelementptr{{.*}}%p.addr, i64 4{{$}}
+; CHECK: getelementptr{{.*}}%p.addr, i64 4{{.*}} !dbg [[zero:![0-9]+]]
 ; CHECK: for.body:
+; CHECK: [[zero]] = !DILocation(line: 0
 ;
 ; ModuleID = 't.ll'
 source_filename = "test.c"

diff  --git a/llvm/test/Transforms/GVN/PRE/phi-translate.ll b/llvm/test/Transforms/GVN/PRE/phi-translate.ll
index 2b6a577d6678..f80e002eab0a 100644
--- a/llvm/test/Transforms/GVN/PRE/phi-translate.ll
+++ b/llvm/test/Transforms/GVN/PRE/phi-translate.ll
@@ -4,8 +4,8 @@ target datalayout = "e-p:64:64:64"
 
 ; CHECK-LABEL: @foo(
 ; CHECK: entry.end_crit_edge:
-; CHECK: %[[INDEX:[a-z0-9.]+]] = sext i32 %x to i64{{$}}
-; CHECK: %[[ADDRESS:[a-z0-9.]+]] = getelementptr [100 x i32], [100 x i32]* @G, i64 0, i64 %[[INDEX]]{{$}}
+; CHECK: %[[INDEX:[a-z0-9.]+]] = sext i32 %x to i64{{.*}} !dbg [[ZERO_LOC:![0-9]+]]
+; CHECK: %[[ADDRESS:[a-z0-9.]+]] = getelementptr [100 x i32], [100 x i32]* @G, i64 0, i64 %[[INDEX]]{{.*}} !dbg [[ZERO_LOC]]
 ; CHECK:   %n.pre = load i32, i32* %[[ADDRESS]], align 4, !dbg [[N_LOC:![0-9]+]]
 ; CHECK: br label %end
 ; CHECK: then:
@@ -14,7 +14,8 @@ target datalayout = "e-p:64:64:64"
 ; CHECK:   %n = phi i32 [ %n.pre, %entry.end_crit_edge ], [ %z, %then ], !dbg [[N_LOC]]
 ; CHECK:   ret i32 %n
 
-; CHECK: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}})
+; CHECK-DAG: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}})
+; CHECK-DAG: [[ZERO_LOC]] = !DILocation(line: 0
 
 @G = external global [100 x i32]
 define i32 @foo(i32 %x, i32 %z) !dbg !6 {

diff  --git a/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll b/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll
index 381d9c77ecd8..3721f09eef8f 100644
--- a/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll
+++ b/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -S -licm < %s | FileCheck %s
 
-; CHECK: %arrayidx4.promoted = load i32, i32* %arrayidx4, align 1, !tbaa !{{[0-9]+$}}
+; CHECK: %arrayidx4.promoted = load i32, i32* %arrayidx4, align 1, !tbaa !59
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

diff  --git a/llvm/unittests/IR/InstructionsTest.cpp b/llvm/unittests/IR/InstructionsTest.cpp
index 7f7cf52e9408..b25eee7279af 100644
--- a/llvm/unittests/IR/InstructionsTest.cpp
+++ b/llvm/unittests/IR/InstructionsTest.cpp
@@ -1304,52 +1304,5 @@ TEST(InstructionsTest, UnaryOperator) {
   I->deleteValue();
 }
 
-TEST(InstructionsTest, UpdateLocationAfterHoist) {
-  LLVMContext C;
-  std::unique_ptr<Module> M = parseIR(C,
-                                      R"(
-      declare void @callee()
-
-      define void @f() {
-        call void @callee()           ; Inst with no location.
-        call void @callee(), !dbg !11 ; Call with location.
-        ret void, !dbg !11            ; Non-call inst with location.
-      }
-
-      !llvm.dbg.cu = !{!0}
-      !llvm.module.flags = !{!3, !4}
-      !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
-      !1 = !DIFile(filename: "t2.c", directory: "foo")
-      !2 = !{}
-      !3 = !{i32 2, !"Dwarf Version", i32 4}
-      !4 = !{i32 2, !"Debug Info Version", i32 3}
-      !8 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !9, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, unit: !0, retainedNodes: !2)
-      !9 = !DISubroutineType(types: !10)
-      !10 = !{null}
-      !11 = !DILocation(line: 2, column: 7, scope: !8)
-  )");
-  ASSERT_TRUE(M);
-  Function *F = cast<Function>(M->getNamedValue("f"));
-  BasicBlock &BB = F->front();
-
-  auto *I1 = BB.getFirstNonPHI();
-  auto *I2 = I1->getNextNode();
-  auto *I3 = BB.getTerminator();
-
-  EXPECT_FALSE(bool(I1->getDebugLoc()));
-  I1->updateLocationAfterHoist();
-  EXPECT_FALSE(bool(I1->getDebugLoc()));
-
-  const MDNode *Scope = I2->getDebugLoc().getScope();
-  EXPECT_EQ(I2->getDebugLoc().getLine(), 2U);
-  I2->updateLocationAfterHoist();
-  EXPECT_EQ(I2->getDebugLoc().getLine(), 0U);
-  EXPECT_EQ(I2->getDebugLoc().getScope(), Scope);
-
-  EXPECT_EQ(I3->getDebugLoc().getLine(), 2U);
-  I3->updateLocationAfterHoist();
-  EXPECT_FALSE(bool(I3->getDebugLoc()));
-}
-
 } // end anonymous namespace
 } // end namespace llvm


        


More information about the llvm-commits mailing list