[PATCH] D17279: [Sink] Don't move calls to readonly functions across stores
Nicolai Hähnle via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 15 14:47:01 PST 2016
nhaehnle created this revision.
nhaehnle added reviewers: hfinkel, majnemer, tstellarAMD, sunfish.
nhaehnle added a subscriber: llvm-commits.
This is conservative because it doesn't consider the argmemonly attribute.
Perhaps somebody who knows this part of the code better can improve this
in a separate patch. For now, it fixes a real miscompilation I encountered
in the AMDGPU backend.
http://reviews.llvm.org/D17279
Files:
lib/Transforms/Scalar/Sink.cpp
test/Transforms/Sink/call.ll
Index: test/Transforms/Sink/call.ll
===================================================================
--- /dev/null
+++ test/Transforms/Sink/call.ll
@@ -0,0 +1,68 @@
+; RUN: opt < %s -basicaa -sink -S | FileCheck %s
+
+declare i32 @f_load_global() nounwind readonly
+declare void @f_store_global(i32) nounwind
+declare i32 @f_readnone(i32) nounwind readnone
+
+ at A = external global i32
+
+; Sink readonly call if no stores are in the way.
+;
+; CHECK-LABEL: @test1(
+; CHECK: true:
+; CHECK-NEXT: %l = call i32 @f_load_global
+; CHECK-NEXT: ret i32 %l
+define i32 @test1(i1 %z) {
+ %l = call i32 @f_load_global()
+ br i1 %z, label %true, label %false
+true:
+ ret i32 %l
+false:
+ ret i32 0
+}
+
+; But don't sink if there is a store ...
+;
+; CHECK-LABEL: @test2(
+; CHECK: call i32 @f_load_global
+; CHECK-NEXT: store i32
+define i32 @test2(i1 %z) {
+ %l = call i32 @f_load_global()
+ store i32 0, i32* @A
+ br i1 %z, label %true, label %false
+true:
+ ret i32 %l
+false:
+ ret i32 0
+}
+
+; ... or a non-readonly call
+;
+; CHECK-LABEL: @test3(
+; CHECK: call i32 @f_load_global
+; CHECK-NEXT: call void @f_store_global
+define i32 @test3(i1 %z) {
+ %l = call i32 @f_load_global()
+ call void @f_store_global(i32 0)
+ br i1 %z, label %true, label %false
+true:
+ ret i32 %l
+false:
+ ret i32 0
+}
+
+; readnone calls are sunk across stores.
+;
+; CHECK-LABEL: @test4(
+; CHECK: true:
+; CHECK-NEXT: %l = call i32 @f_readnone(
+; CHECK-NEXT: ret i32 %l
+define i32 @test4(i1 %z) {
+ %l = call i32 @f_readnone(i32 0)
+ store i32 0, i32* @A
+ br i1 %z, label %true, label %false
+true:
+ ret i32 %l
+false:
+ ret i32 0
+}
Index: lib/Transforms/Scalar/Sink.cpp
===================================================================
--- lib/Transforms/Scalar/Sink.cpp
+++ lib/Transforms/Scalar/Sink.cpp
@@ -173,11 +173,14 @@
Inst->mayThrow())
return false;
- // Convergent operations cannot be made control-dependent on additional
- // values.
if (auto CS = CallSite(Inst)) {
+ // Convergent operations cannot be made control-dependent on additional
+ // values.
if (CS.hasFnAttr(Attribute::Convergent))
return false;
+
+ if (Inst->mayReadFromMemory() && !Stores.empty())
+ return false;
}
return true;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D17279.48017.patch
Type: text/x-patch
Size: 2272 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160215/bd921b92/attachment.bin>
More information about the llvm-commits
mailing list