[Polly][DebugInfo] Use getStableDebugLoc to avoid intrinsic-dependent behaviour

[SLTozer]

Polly currently uses getDebugLoc in a few places to produce diagnostic output; this is correct when interacting with specific instructions, but may be incorrect when dealing with instruction ranges if debug intrinsics are included. As a general rule, the debug locations attached to debug intrinsics may be misleading compared to the surrounding instructions, and are not generally used for anything other than determining variable scope info; the recommended approach is therefore to use getStableDebugLoc instead, which skips over debug intrinsics. This is necessary to fix test failures that occur when enabling non-instruction debug info, which removes debug intrinsics from basic blocks and thus alters the diagnostic output of Polly (despite causing no functional change).

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

Changes

Polly currently uses getDebugLoc in a few places to produce diagnostic output; this is correct when interacting with specific instructions, but may be incorrect when dealing with instruction ranges if debug intrinsics are included. As a general rule, the debug locations attached to debug intrinsics may be misleading compared to the surrounding instructions, and are not generally used for anything other than determining variable scope info; the recommended approach is therefore to use getStableDebugLoc instead, which skips over debug intrinsics. This is necessary to fix test failures that occur when enabling non-instruction debug info, which removes debug intrinsics from basic blocks and thus alters the diagnostic output of Polly (despite causing no functional change).

---

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

4 Files Affected:

• (modified) polly/lib/Analysis/ScopDetectionDiagnostic.cpp (+2-2)
• (modified) polly/lib/Support/ScopLocation.cpp (+1-1)
• (modified) polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll (+4-4)
• (modified) polly/test/ScopDetectionDiagnostics/loop_has_multiple_exits.ll (+1-1)

diff --git a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
index 364e21aef207c..30fbd17c78bfe 100644
--- a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
+++ b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
@@ -122,7 +122,7 @@ void getDebugLocations(const BBPair &P, DebugLoc &Begin, DebugLoc &End) {
       continue;
     Todo.append(succ_begin(BB), succ_end(BB));
     for (const Instruction &Inst : *BB) {
-      DebugLoc DL = Inst.getDebugLoc();
+      DebugLoc DL = Inst.getStableDebugLoc();
       if (!DL)
         continue;
 
@@ -821,7 +821,7 @@ std::string ReportUnprofitable::getEndUserMessage() const {
 const DebugLoc &ReportUnprofitable::getDebugLoc() const {
   for (const BasicBlock *BB : R->blocks())
     for (const Instruction &Inst : *BB)
-      if (const DebugLoc &DL = Inst.getDebugLoc())
+      if (const DebugLoc &DL = Inst.getStableDebugLoc())
         return DL;
 
   return R->getEntry()->getTerminator()->getDebugLoc();
diff --git a/polly/lib/Support/ScopLocation.cpp b/polly/lib/Support/ScopLocation.cpp
index 01f3d68926d88..9f9941d57ad26 100644
--- a/polly/lib/Support/ScopLocation.cpp
+++ b/polly/lib/Support/ScopLocation.cpp
@@ -25,7 +25,7 @@ void getDebugLocation(const Region *R, unsigned &LineBegin, unsigned &LineEnd,
 
   for (const BasicBlock *BB : R->blocks())
     for (const Instruction &Inst : *BB) {
-      DebugLoc DL = Inst.getDebugLoc();
+      DebugLoc DL = Inst.getStableDebugLoc();
       if (!DL)
         continue;
 
diff --git a/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll b/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll
index 618237157e084..35986b5e0b352 100644
--- a/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll
+++ b/polly/test/ScopDetectionDiagnostics/ReportLoopBound-01.ll
@@ -19,20 +19,20 @@
 
 ; If we reject non-affine loops the non-affine loop bound will be reported:
 ;
-; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:1:12: The following errors keep this region from being a Scop.
+; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:2:8: The following errors keep this region from being a Scop.
 ; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:2:8: Failed to derive an affine function from the loop bounds.
 ; REJECTNONAFFINELOOPS: remark: ReportLoopBound-01.c:3:5: Invalid Scop candidate ends here.
 
 ; If we allow non-affine loops the non-affine access will be reported:
 ;
-; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:1:12: The following errors keep this region from being a Scop.
+; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:2:8: The following errors keep this region from being a Scop.
 ; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:3:5: The array subscript of "A" is not affine
 ; ALLOWNONAFFINELOOPS: remark: ReportLoopBound-01.c:3:5: Invalid Scop candidate ends here.
 
 ; If we allow non-affine loops and non-affine accesses the region will be reported as not profitable:
 ;
-; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:1:12: The following errors keep this region from being a Scop.
-; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:1:12: No profitable polyhedral optimization found
+; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:2:8: The following errors keep this region from being a Scop.
+; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:2:8: No profitable polyhedral optimization found
 ; ALLOWNONAFFINEALL: remark: ReportLoopBound-01.c:3:5: Invalid Scop candidate ends here.
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/polly/test/ScopDetectionDiagnostics/loop_has_multiple_exits.ll b/polly/test/ScopDetectionDiagnostics/loop_has_multiple_exits.ll
index 7661bd004f5c0..a0f2704b13723 100644
--- a/polly/test/ScopDetectionDiagnostics/loop_has_multiple_exits.ll
+++ b/polly/test/ScopDetectionDiagnostics/loop_has_multiple_exits.ll
@@ -2,7 +2,7 @@
 ;
 ; Derived from test-suite/MultiSource/Benchmarks/BitBench/uuencode/uuencode.c
 ;
-; CHECK: remark: uuencode.c:75:18: The following errors keep this region from being a Scop.
+; CHECK: remark: uuencode.c:76:13: The following errors keep this region from being a Scop.
 ; CHECK: remark: uuencode.c:83:3: Loop cannot be handled because it has multiple exits.
 ; CHECK: remark: uuencode.c:95:21: Invalid Scop candidate ends here.
 

[jryans]

Looks correct to me, thanks! 😄

[dwblaikie]

Awesome to see the RemoveDIs work finding/fixing some of these debug-info-invariance issues (in this case it's not a full debug-info-affects-codegen, but variable-debug-info-changes-diagnostic behavior is an issue, if a lesser one than the former)