Add a todo and tests to Address a review commnt from D50925 [NFC]

llvm-svn: 340978

lib/Transforms/Scalar/LICM.cpp

@@ -737,7 +737,9 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
 
     // We can only hoist a store that we can prove writes a value which is not
     // read or overwritten within the loop.  For those cases, we fallback to
-    // load store promotion instead.
+    // load store promotion instead.  TODO: We can extend this to cases where
+    // there is exactly one write to the location and that write dominates an
+    // arbitrary number of reads in the loop.
     auto &AS = CurAST->getAliasSetFor(MemoryLocation::get(SI));
 
     if (AS.isRef() || !AS.isMustAlias())

test/Transforms/LICM/store-hoisting.ll

@@ -309,3 +309,120 @@ loop:
 exit:
   ret void
 }
+
+; TODO: could validly hoist the store here since we know what value
+; the load must observe.
+define i32 @test_dominated_read(i32* %loc) {
+; CHECK-LABEL: @test_dominated_read
+; CHECK-LABEL: exit:
+; CHECK: store i32 0, i32* %loc
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i32 [0, %entry], [%iv.next, %loop]
+  store i32 0, i32* %loc
+  %reload = load i32, i32* %loc
+  %iv.next = add i32 %iv, 1
+  %cmp = icmp slt i32 %iv, 200
+  br i1 %cmp, label %loop, label %exit
+
+exit:
+  ret i32 %reload
+}
+
+; TODO: could validly hoist the store since we already hoisted the load and
+; it's no longer in the loop.
+define i32 @test_dominating_read(i32* %loc) {
+; CHECK-LABEL: @test_dominating_read
+; CHECK-LABEL: exit:
+; CHECK: store i32 0, i32* %loc
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i32 [0, %entry], [%iv.next, %loop]
+  %reload = load i32, i32* %loc
+  store i32 0, i32* %loc
+  %iv.next = add i32 %iv, 1
+  %cmp = icmp slt i32 %iv, 200
+  br i1 %cmp, label %loop, label %exit
+
+exit:
+  ret i32 %reload
+}
+
+declare void @readonly() readonly
+
+; TODO: can legally hoist since value read by call is known
+define void @test_dominated_readonly(i32* %loc) {
+; CHECK-LABEL: @test_dominated_readonly
+; CHECK-LABEL: loop:
+; CHECK: store i32 0, i32* %loc
+; CHECK-LABEL: exit:
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i32 [0, %entry], [%iv.next, %loop]
+  store i32 0, i32* %loc
+  call void @readonly()
+  %iv.next = add i32 %iv, 1
+  %cmp = icmp slt i32 %iv, 200
+  br i1 %cmp, label %loop, label %exit
+
+exit:
+  ret void
+}
+
+; While technically possible to hoist the store to %loc, this runs across
+; a funemental limitation of alias sets since both stores and the call are
+; within the same alias set and we can't distinguish them cheaply.
+define void @test_aliasset_fn(i32* %loc, i32* %loc2) {
+; CHECK-LABEL: @test_aliasset_fn
+; CHECK-LABEL: loop:
+; CHECK: store i32 0, i32* %loc
+; CHECK-LABEL: exit:
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i32 [0, %entry], [%iv.next, %loop]
+  store i32 0, i32* %loc
+  call void @readonly()
+  store i32 %iv, i32* %loc2
+  %iv.next = add i32 %iv, 1
+  %cmp = icmp slt i32 %iv, 200
+  br i1 %cmp, label %loop, label %exit
+
+exit:
+  ret void
+}
+
+
+; If we can't tell if the value is read before the write, we can't hoist the
+; write over the potential read (since we don't know the value read)
+define void @neg_may_read(i32* %loc, i1 %maybe) {
+; CHECK-LABEL: @neg_may_read
+; CHECK-LABEL: loop:
+; CHECK: store i32 0, i32* %loc
+; CHECK-LABEL: exit:
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i32 [0, %entry], [%iv.next, %merge]
+  ;; maybe is a placeholder for an unanalyzable condition
+  br i1 %maybe, label %taken, label %merge
+taken:
+  call void @readonly()
+  br label %merge
+merge:
+  store i32 0, i32* %loc
+  %iv.next = add i32 %iv, 1
+  %cmp = icmp slt i32 %iv, 200
+  br i1 %cmp, label %loop, label %exit
+
+exit:
+  ret void
+}