[PATCH] D115965: [DSE] Fix a case of invalid dead store elimination

[Marianne Mailhot-Sarrasin via Phabricator via llvm-commits]

mamai created this revision.
mamai added reviewers: fhahn, dmgreen.
Herald added a subscriber: hiraditya.
mamai requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

When verifying a dependency between 'CurrentDef' and 'KillingDef' is loop invariant, we also need to confirm they are in the same loop. This was causing a problem when the 'KillingDef' was outside of the loop, while the 'CurrentDef' was inside the loop. The 'KillingDef' only overrites the definition from the last iteration of the loop, and not the ones of all iterations, therefor it does not make the 'CurrentDef' to be dead.

Fixing issue : https://github.com/llvm/llvm-project/issues/52774


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115965

Files:
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
  llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll


Index: llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/DeadStoreElimination/store-after-loop.ll
@@ -0,0 +1,66 @@
+; RUN: opt -dse -S %s | FileCheck %s
+
+target datalayout = "E-m:e-p:32:32-i64:32-f64:32:64-a:0:32-n32-S32"
+
+%struct.ilist = type { i32, %struct.ilist* }
+
+ at array = dso_local local_unnamed_addr global [10 x i32] [i32 14, i32 66, i32 12, i32 41, i32 86, i32 69, i32 19, i32 77, i32 68, i32 38], align 4
+
+; There is no dead store in this test. Make sure no store is deleted by DSE.
+
+; CHECK-LABEL: @test(
+; CHECK:		store
+; CHECK:       loop1:
+; CHECK:        store i32 [[TMP10:%.*]], i32* [[TMP11:%.*]], align 8
+; CHECK:        store %struct.ilist* [[TMP6:%.*]], %struct.ilist** [[TMP12:%.*]], align 4
+; CHECK:        br
+; CHECK:       loop2:
+; CHECK:        store %struct.ilist* [[TMP15:%.*]], %struct.ilist** [[TMP19:%.*]], align 4
+; CHECK:       ret void
+
+; Function Attrs: nofree nounwind optsize
+define dso_local void @test() local_unnamed_addr #0 {
+  br label %loop1
+
+1:
+  %2 = bitcast i8* %7 to %struct.ilist*
+  %3 = getelementptr inbounds %struct.ilist, %struct.ilist* %2, i32 0, i32 1
+  store %struct.ilist* null, %struct.ilist** %3, align 4
+  %4 = icmp eq %struct.ilist* %6, null
+  br i1 %4, label %exit, label %loop2
+
+loop1:
+  %5 = phi i32 [ 0, %0 ], [ %13, %loop1 ]
+  %6 = phi %struct.ilist* [ null, %0 ], [ %8, %loop1 ]
+  %7 = tail call align 8 dereferenceable_or_null(8) i8* @malloc(i32 8) #2
+  %8 = bitcast i8* %7 to %struct.ilist*
+  %9 = getelementptr inbounds [10 x i32], [10 x i32]* @array, i32 0, i32 %5
+  %10 = load i32, i32* %9, align 4
+  %11 = getelementptr inbounds %struct.ilist, %struct.ilist* %8, i32 0, i32 0
+  store i32 %10, i32* %11, align 8
+  %12 = getelementptr inbounds %struct.ilist, %struct.ilist* %8, i32 0, i32 1
+  store %struct.ilist* %6, %struct.ilist** %12, align 4
+  %13 = add nuw nsw i32 %5, 1
+  %14 = icmp eq i32 %13, 10
+  br i1 %14, label %1, label %loop1
+
+loop2:
+  %15 = phi %struct.ilist* [ %16, %loop2 ], [ %2, %1 ]
+  %16 = phi %struct.ilist* [ %18, %loop2 ], [ %6, %1 ]
+  %17 = getelementptr inbounds %struct.ilist, %struct.ilist* %16, i32 0, i32 1
+  %18 = load %struct.ilist*, %struct.ilist** %17, align 4
+  %19 = getelementptr inbounds %struct.ilist, %struct.ilist* %16, i32 0, i32 1
+  store %struct.ilist* %15, %struct.ilist** %19, align 4
+  %20 = icmp eq %struct.ilist* %18, null
+  br i1 %20, label %exit, label %loop2
+
+exit:                                               ; preds = %loop2, %1
+  ret void
+}
+
+; Function Attrs: inaccessiblememonly mustprogress nofree nounwind optsize willreturn
+declare dso_local noalias noundef align 8 i8* @malloc(i32 noundef) local_unnamed_addr #1
+
+attributes #0 = { nofree nounwind optsize "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+attributes #1 = { inaccessiblememonly mustprogress nofree nounwind optsize willreturn "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+attributes #2 = { optsize }
\ No newline at end of file
Index: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -1201,8 +1201,12 @@
     if (!ContainsIrreducibleLoops && CurrentLI &&
         CurrentLI == LI.getLoopFor(KillingDef->getParent()))
       return true;
-    // Otherwise check the memory location is invariant to any loops.
-    return isGuaranteedLoopInvariant(CurrentLoc.Ptr);
+    // If they are both in the same loop, check the memory location is invariant
+    // for that loop.
+    if (CurrentLI && CurrentLI->contains(KillingDef) &&
+        isGuaranteedLoopInvariant(CurrentLoc.Ptr))
+      return true;
+    return false;
   }
 
   /// Returns true if \p Ptr is guaranteed to be loop invariant for any possible


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115965.395193.patch
Type: text/x-patch
Size: 4073 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211217/a730f3c4/attachment.bin>