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

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

mamai updated this revision to Diff 395448.
mamai edited the summary of this revision.
mamai added a comment.

I realized I didn't ran the tests correctly when I ran it locally, and my  patch was breaking a lot of tests. I have modified the patch to not affect the code stores that are not in loops.


Repository:
  rG LLVM Github Monorepo

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

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
@@ -1202,7 +1202,10 @@
         CurrentLI == LI.getLoopFor(KillingDef->getParent()))
       return true;
     // Otherwise check the memory location is invariant to any loops.
-    return isGuaranteedLoopInvariant(CurrentLoc.Ptr);
+    if (isGuaranteedLoopInvariant(CurrentLoc.Ptr))
+      // To be valid, if their is a loop, they must be in the same loop.
+      return (!CurrentLI || CurrentLI->contains(KillingDef));
+    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.395448.patch
Type: text/x-patch
Size: 3977 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211220/c788461f/attachment-0001.bin>