Fix Function Attributes computation for llvm.assume

[boazo]

Summary

When running the function attributes pass llvm.assume is marked as readnone.
Running instcombine after this pass will remove the llvm.assume causing a potential optimization from happening.
This required after commit rL268068

[boazo]

whitespace fixes

[hfinkel]

LGTM.

[majnemer]

lib/Transforms/IPO/FunctionAttrs.cpp

208–211

Sentences start with a capital letter and a period.

test/Transforms/FunctionAttrs/assume.ll

2

Why do we need to run InstCombine to test this change?

[majnemer]

@sanjoy Doesn't your intrinsic need the same treatment?

[sanjoy]

I think guards are fine since rL268068 only touches assumes. Also guards are not nounwind so even if we do a rL268068 like thing for them, ideally LLVM should not be removing them even after proving readonly (guards are also readonly in a precise sense, not readnone).

[pcc]

D20612 solves the same problem in (IMHO) a slightly nicer way.

[boazo]

Hi All,

I agree that D20612 solves it better.
Thanks for the review and the fix.
I will abandon this revision.

Boaz

lib/Transforms/IPO/FunctionAttrs.cpp

for (Function *F : SCCNodes) {
     if (F->doesNotAccessMemory())
       // Already perfect!
       continue;
 
     if (F->onlyReadsMemory() && ReadsMemory)
       // No change.
       continue;
 
+    if (F->getIntrinsicID() == Intrinsic::assume)
+      // don't change attributes of the assume intrinsic, 
+      // so that later on, it is not thrown away by other optimizations
+      continue;
+
     MadeChange = true;
 
     // Clear out any existing attributes.
     AttrBuilder B;
     B.addAttribute(Attribute::ReadOnly).addAttribute(Attribute::ReadNone);
     F->removeAttributes(
         AttributeSet::FunctionIndex,
         AttributeSet::get(F->getContext(), AttributeSet::FunctionIndex, B));

test/Transforms/FunctionAttrs/assume.ll

+; RUN: opt -functionattrs -S < %s | FileCheck %s
+; RUN: opt -functionattrs -instcombine -S < %s | FileCheck %s -check-prefix=ASSUME
+;
+; Ensures that llvm.assume:
+;  is not marked with readnone attribute.
+;  is not considered as accessing memory for the purposes of function attributes computation.
+
+;CHECK: declare void @llvm.assume(i1) #0
+declare void @llvm.assume(i1)
+
+;CHECK: define i32 @foo(i32 %x) #1
+;ASSUME: define i32 @foo(i32 %x)
+define i32 @foo(i32 %x) {
+entry:
+  %cmp = icmp eq i32 %x, 2
+  call void @llvm.assume(i1 %cmp)
+;ASSUME: ret i32 2
+  ret i32 %x
+}
+
+;CHECK: define i32 @foo2(i32 %x) #1
+define i32 @foo2(i32 %x) {
+entry:
+  %foo.x = call i32 @foo(i32 %x)
+  ret i32 %foo.x
+}
+
+;CHECK: attributes #0 = { nounwind }
+;CHECK: attributes #1 = { readnone }