[PATCH] D107016: [DebugInfo][LoopStrengthReduction] Ensure restoration of cached DIExpression when using SCEV-based salvaging

[Chris Jackson via Phabricator via llvm-commits]

chrisjackson updated this revision to Diff 369894.
chrisjackson added a comment.

Update the text for the test that already covers restoration of the dbg.value arguments from the cache. During LSR a failed salvage attempt is made that modifies the dbg.value arguments. These arguments must be restored to their pre-LSR state in order for scev-based salvaging to succeed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107016/new/

https://reviews.llvm.org/D107016

Files:
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
  llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll


Index: llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll
===================================================================
--- llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll
+++ llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll
@@ -1,8 +1,10 @@
 ; RUN: opt < %s -loop-reduce -S | FileCheck %s
 
-; Test that LSR does not produce invalid debug info when a debug value is
-; salvaged during LSR by adding additional location operands, then becomes
-; undef, and finally recovered by SCEV salvaging.
+; Test that SCEV-based salvaging does not produce invalid debug info when during
+; LSR a salvage is attempted and fails. This attempt can modify the number of
+; location operands and the DIExpression. For SCEV-based salvaging to work
+; correctly the pre-LSR DIExpression and location operands (even if some are 
+; now undef) must be restored from the cache. 
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
Index: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -6194,15 +6194,17 @@
       // Some DVIs that were single location-op when cached are now multi-op,
       // due to LSR optimisations. However, multi-op salvaging is not yet
       // supported by SCEV salvaging. But, we can attempt a salvage by restoring
-      // the pre-LSR single-op expression.
+      // the pre-LSR single-op location and expression.
       if (DVIRec.DVI->hasArgList()) {
         if (!DVIRec.DVI->getVariableLocationOp(0))
           continue;
         llvm::Type *Ty = DVIRec.DVI->getVariableLocationOp(0)->getType();
         DVIRec.DVI->setRawLocation(
             llvm::ValueAsMetadata::get(UndefValue::get(Ty)));
-        DVIRec.DVI->setExpression(DVIRec.Expr);
       }
+      // LSR may have modified the expression during a salvage attempt, so
+      // restore to the expression that corresponds to the cached SCEV.
+      DVIRec.DVI->setExpression(DVIRec.Expr);
 
       Changed |= RewriteDVIUsingIterCount(DVIRec, IterCountExpr, SE);
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107016.369894.patch
Type: text/x-patch
Size: 2203 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210901/268cbdc4/attachment.bin>