[llvm] r250973 - [OperandBundles] Make function attributes conservatively correct
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 21 20:12:23 PDT 2015
Author: sanjoy
Date: Wed Oct 21 22:12:22 2015
New Revision: 250973
URL: http://llvm.org/viewvc/llvm-project?rev=250973&view=rev
Log:
[OperandBundles] Make function attributes conservatively correct
Summary:
This makes attribute accessors on `CallInst` and `InvokeInst` do the
(conservatively) right thing. This essentially involves, in some
cases, *not* falling back querying the attributes on the called
`llvm::Function` when operand bundles are present.
Attributes locally present on the `CallInst` or `InvokeInst` will still
override operand bundle semantics. The LangRef has been amended to
reflect this. Note: this change does not do anything prevent
`-function-attrs` from inferring `CallSite` local attributes after
inspecting the called function -- that will be done as a separate
change.
I've used `-adce` and `-early-cse` to test these changes. There is
nothing special about these passes (and they did not require any
changes) except that they seemed be the easiest way to write the tests.
This change does not add deal with `argmemonly`. That's a later change
because alias analysis requires a related fix before `argmemonly` can be
tested.
Reviewers: reames, chandlerc
Subscribers: llvm-commits
Differential Revision: http://reviews.llvm.org/D13961
Added:
llvm/trunk/test/Feature/OperandBundles/
llvm/trunk/test/Feature/OperandBundles/adce.ll
llvm/trunk/test/Feature/OperandBundles/early-cse.ll
Modified:
llvm/trunk/docs/LangRef.rst
llvm/trunk/include/llvm/IR/InstrTypes.h
llvm/trunk/include/llvm/IR/Instructions.h
llvm/trunk/lib/IR/Instructions.cpp
Modified: llvm/trunk/docs/LangRef.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/LangRef.rst?rev=250973&r1=250972&r2=250973&view=diff
==============================================================================
--- llvm/trunk/docs/LangRef.rst (original)
+++ llvm/trunk/docs/LangRef.rst Wed Oct 21 22:12:22 2015
@@ -1481,6 +1481,15 @@ operand bundle to not miscompile program
- An operand bundle at a call site cannot change the implementation
of the called function. Inter-procedural optimizations work as
usual as long as they take into account the first two properties.
+- The bundle operands for an unknown operand bundle escape in unknown
+ ways before control is transferred to the callee or invokee.
+- Calls and invokes with operand bundles have unknown read / write
+ effect on the heap on entry and exit (even if the call target is
+ ``readnone`` or ``readonly``), unless they're overriden with
+ callsite specific attributes.
+- An operand bundle at a call site cannot change the implementation
+ of the called function. Inter-procedural optimizations work as
+ usual as long as they take into account the first two properties.
.. _moduleasm:
Modified: llvm/trunk/include/llvm/IR/InstrTypes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/InstrTypes.h?rev=250973&r1=250972&r2=250973&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/InstrTypes.h (original)
+++ llvm/trunk/include/llvm/IR/InstrTypes.h Wed Oct 21 22:12:22 2015
@@ -18,6 +18,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/Twine.h"
+#include "llvm/IR/Attributes.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/Instruction.h"
#include "llvm/IR/OperandTraits.h"
@@ -1232,8 +1233,50 @@ public:
return None;
}
+ /// \brief Return true if this operand bundle user has operand bundles that
+ /// may read from the heap.
+ bool hasReadingOperandBundles() const {
+ // Implementation note: this is a conservative implementation of operand
+ // bundle semantics, where *any* operand bundle forces a callsite to be at
+ // least readonly.
+ return hasOperandBundles();
+ }
+
+ /// \brief Return true if this operand bundle user has operand bundles that
+ /// may write to the heap.
+ bool hasClobberingOperandBundles() const {
+ // Implementation note: this is a conservative implementation of operand
+ // bundle semantics, where *any* operand bundle forces a callsite to be
+ // read-write.
+ return hasOperandBundles();
+ }
protected:
+ /// \brief Is the function attribute S disallowed by some operand bundle on
+ /// this operand bundle user?
+ bool isFnAttrDisallowedByOpBundle(StringRef S) const {
+ // Operand bundles only possibly disallow readnone and readonly attributes.
+ // All String attributes are fine.
+ return false;
+ }
+
+ /// \brief Is the function attribute A disallowed by some operand bundle on
+ /// this operand bundle user?
+ bool isFnAttrDisallowedByOpBundle(Attribute::AttrKind A) const {
+ switch (A) {
+ default:
+ return false;
+
+ case Attribute::ReadNone:
+ return hasReadingOperandBundles();
+
+ case Attribute::ReadOnly:
+ return hasClobberingOperandBundles();
+ }
+
+ llvm_unreachable("switch has a default case!");
+ }
+
/// \brief Used to keep track of an operand bundle. See the main comment on
/// OperandBundleUser above.
struct BundleOpInfo {
Modified: llvm/trunk/include/llvm/IR/Instructions.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instructions.h?rev=250973&r1=250972&r2=250973&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Instructions.h (original)
+++ llvm/trunk/include/llvm/IR/Instructions.h Wed Oct 21 22:12:22 2015
@@ -1742,6 +1742,12 @@ private:
template <typename AttrKind> bool hasFnAttrImpl(AttrKind A) const {
if (AttributeList.hasAttribute(AttributeSet::FunctionIndex, A))
return true;
+
+ // Operand bundles override attributes on the called function, but don't
+ // override attributes directly present on the call instruction.
+ if (isFnAttrDisallowedByOpBundle(A))
+ return false;
+
if (const Function *F = getCalledFunction())
return F->getAttributes().hasAttribute(AttributeSet::FunctionIndex, A);
return false;
Modified: llvm/trunk/lib/IR/Instructions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instructions.cpp?rev=250973&r1=250972&r2=250973&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Instructions.cpp (original)
+++ llvm/trunk/lib/IR/Instructions.cpp Wed Oct 21 22:12:22 2015
@@ -563,6 +563,12 @@ void InvokeInst::setSuccessorV(unsigned
bool InvokeInst::hasFnAttrImpl(Attribute::AttrKind A) const {
if (AttributeList.hasAttribute(AttributeSet::FunctionIndex, A))
return true;
+
+ // Operand bundles override attributes on the called function, but don't
+ // override attributes directly present on the invoke instruction.
+ if (isFnAttrDisallowedByOpBundle(A))
+ return false;
+
if (const Function *F = getCalledFunction())
return F->getAttributes().hasAttribute(AttributeSet::FunctionIndex, A);
return false;
Added: llvm/trunk/test/Feature/OperandBundles/adce.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Feature/OperandBundles/adce.ll?rev=250973&view=auto
==============================================================================
--- llvm/trunk/test/Feature/OperandBundles/adce.ll (added)
+++ llvm/trunk/test/Feature/OperandBundles/adce.ll Wed Oct 21 22:12:22 2015
@@ -0,0 +1,41 @@
+; RUN: opt -S -adce < %s | FileCheck %s
+
+; While it is normally okay to DCE out calls to @readonly_function and
+; @readnone_function, we cannot do that if they're carrying operand
+; bundles since the presence of unknown operand bundles implies
+; arbitrary memory effects.
+
+declare void @readonly_function() readonly nounwind
+declare void @readnone_function() readnone nounwind
+
+define void @test0() {
+; CHECK-LABEL: @test0(
+ entry:
+ call void @readonly_function() [ "tag"() ]
+; CHECK: call void @readonly_function
+ ret void
+}
+
+define void @test1() {
+; CHECK-LABEL: @test1(
+ entry:
+ call void @readnone_function() [ "tag"() ]
+; CHECK: call void @readnone_function
+ ret void
+}
+
+define void @test2() {
+; CHECK-LABEL: @test2(
+ entry:
+; CHECK-NOT: @readonly_function(
+ call void @readonly_function() readonly [ "tag"() ]
+ ret void
+}
+
+define void @test3() {
+; CHECK-LABEL: @test3(
+ entry:
+; CHECK-NOT: @readnone_function(
+ call void @readnone_function() readnone [ "tag"() ]
+ ret void
+}
Added: llvm/trunk/test/Feature/OperandBundles/early-cse.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Feature/OperandBundles/early-cse.ll?rev=250973&view=auto
==============================================================================
--- llvm/trunk/test/Feature/OperandBundles/early-cse.ll (added)
+++ llvm/trunk/test/Feature/OperandBundles/early-cse.ll Wed Oct 21 22:12:22 2015
@@ -0,0 +1,71 @@
+; RUN: opt -S -early-cse < %s | FileCheck %s
+
+; While it is normally okay to do memory optimizations over calls to
+; @readonly_function and @readnone_function, we cannot do that if
+; they're carrying operand bundles since the presence of unknown
+; operand bundles implies arbitrary memory effects.
+
+declare void @readonly_function() readonly nounwind
+declare void @readnone_function() readnone nounwind
+
+define i32 @test0(i32* %x) {
+; CHECK-LABEL: @test0(
+ entry:
+ store i32 100, i32* %x
+; CHECK: store i32 100, i32* %x
+ call void @readonly_function() [ "tag"() ]
+; CHECK: call void @readonly_function()
+
+ %v = load i32, i32* %x
+; CHECK: %v = load i32, i32* %x
+; CHECK: ret i32 %v
+ ret i32 %v
+}
+
+define i32 @test1(i32* %x) {
+; CHECK: @test1(
+ entry:
+ store i32 100, i32* %x
+; CHECK: store i32 100, i32* %x
+ call void @readonly_function() readonly [ "tag"() ]
+; CHECK-NOT: call void @readonly_function
+ %v = load i32, i32* %x
+ ret i32 %v
+; CHECK: ret i32 100
+}
+
+define i32 @test3(i32* %x) {
+; CHECK-LABEL: @test3(
+ entry:
+ store i32 100, i32* %x
+; CHECK: store i32 100, i32* %x
+ call void @readonly_function()
+; CHECK-NOT: call void @readonly_function
+ %v = load i32, i32* %x
+ ret i32 %v
+; CHECK: ret i32 100
+}
+
+define void @test4(i32* %x) {
+; CHECK-LABEL: @test4(
+ entry:
+ store i32 100, i32* %x
+; CHECK: store i32 100, i32* %x
+ call void @readnone_function() [ "tag"() ]
+; CHECK: call void @readnone_function
+ store i32 200, i32* %x
+; CHECK: store i32 200, i32* %x
+ ret void
+}
+
+define void @test5(i32* %x) {
+; CHECK-LABEL: @test5(
+ entry:
+ store i32 100, i32* %x
+; CHECK-NOT: store i32 100, i32* %x
+; CHECK-NOT: call void @readnone_function
+ call void @readnone_function() readnone [ "tag"() ]
+ store i32 200, i32* %x
+; CHECK: store i32 200, i32* %x
+ ret void
+}
More information about the llvm-commits
mailing list