[CallSiteSplit] Make sure we remove nonnull if the parameter turns out to be a constant.

[trentxintong]

Summary

We do not need nonull attribute if we know an argument is going to be constant.

[junbuml]

Thanks for handling this. Just curious if you see any issue with the non-null attribute in a constant ?

test/Transforms/CallSiteSplitting/callsite-split.ll

44

Can you move this in callsite-split-or-phi.ll where we run only callsite-splitting pass. Looks like you can also use just like i32* instead of %struct.bitmap*.

[trentxintong]

Address comments.

[trentxintong]

In D45608#1067141, @junbuml wrote:

Thanks for handling this. Just curious if you see any issue with the non-null attribute in a constant ?

Probably no problem other than it bloats the IR a bit with unnecessary attribute.

[trentxintong]

Remove empty lines.

[trentxintong]

Ping. Thanks.

[fhahn]

LGTM, with the test simplification. Unfortunately there is no easy way to prevent adding the unnecessary nonnull attribute in first place.

test/Transforms/CallSiteSplitting/callsite-split-or-phi.ll

577 ↗

(On Diff #143492)

Do we need inttoptr here?

590 ↗

(On Diff #143492)

I don't think we need a complex callee2 function that's called here. Could you just use the existing callee function?

[trentxintong]

Simplify test. I could not get rid of the inttoptr though.

lib/Transforms/Scalar/CallSiteSplitting.cpp

for (auto &I : CS.args()) {
     ++ArgNo;
   }
 }
 
 static void setConstantInArgument(CallSite CS, Value *Op,
                                   Constant *ConstValue) {
   unsigned ArgNo = 0;
   for (auto &I : CS.args()) {
-    if (&*I == Op)
+    if (&*I == Op) {
+      // It is possible we have already added the non-null attribute to the
+      // parameter by using an earlier constraining condition.
+      CS.removeParamAttr(ArgNo, Attribute::NonNull);
       CS.setArgument(ArgNo, ConstValue);
+    }
     ++ArgNo;
   }
 }
 
 static bool isCondRelevantToAnyCallArgument(ICmpInst *Cmp, CallSite CS) {
   assert(isa<Constant>(Cmp->getOperand(1)) && "Expected a constant operand.");
   Value *Op0 = Cmp->getOperand(0);
   unsigned ArgNo = 0;

test/Transforms/CallSiteSplitting/callsite-split.ll

CallSiteBB:
   %p = phi i1 [0, %Top], [%c, %NextCond]
   call void @callee(%struct.bitmap* %a_elt, %struct.bitmap* %a_elt, %struct.bitmap* %b_elt, i1 %p)
   br label %End
 
 End:
   ret void
 }
 
+; Make sure we remove the non-nullness on constant paramater.
+;
+;CHECK-LABEL: @caller2
+;CHECK-LABEL: Top1.split:
+;CHECK: call void @callee(%struct.bitmap* inttoptr (i64 4643 to %struct.bitmap*), %struct.bitmap* inttoptr (i64 4643 to %struct.bitmap*), %struct.bitmap* %b_elt, i1 false)
+
+define void @caller2(i1 %c, %struct.bitmap* %a_elt, %struct.bitmap* %b_elt) {
+entry:
+  br label %Top0
+
+Top0:
+  %tobool1 = icmp eq %struct.bitmap* %a_elt, inttoptr (i64 4643 to  %struct.bitmap*) 
+  br i1 %tobool1, label %Top1, label %NextCond
+
+Top1:
+  %tobool2 = icmp ne %struct.bitmap* %a_elt, null
+  br i1 %tobool2, label %CallSiteBB, label %NextCond
+
+NextCond:
+  %cmp = icmp ne %struct.bitmap* %b_elt, null
+  br i1 %cmp, label %CallSiteBB, label %End
+
+CallSiteBB:
+  %p = phi i1 [0, %Top1], [%c, %NextCond]
+  call void @callee(%struct.bitmap* %a_elt, %struct.bitmap* %a_elt, %struct.bitmap* %b_elt, i1 %p)
+  br label %End
+
+End:
+  ret void
+}
+
+
 define void @callee(%struct.bitmap* %dst_elt, %struct.bitmap* %a_elt, %struct.bitmap* %b_elt, i1 %c) {
 entry:
   %tobool = icmp ne %struct.bitmap* %a_elt, null
   %tobool1 = icmp ne %struct.bitmap* %b_elt, null
   %or.cond = and i1 %tobool, %tobool1
   br i1 %or.cond, label %Cond, label %Big
 
 Cond:

 End:
   ret void
 }
 
 declare void @dummy2(%struct.bitmap*)
 declare void @dummy1(%struct.bitmap*, %struct.bitmap*, %struct.bitmap*, %struct.bitmap*, %struct.bitmap*, %struct.bitmap*)
 
 
-;CHECK-LABEL: @caller2
+;CHECK-LABEL: @caller3
 ;CHECK-LABEL: Top.split:
 ;CHECK: call void @dummy4()
 ;CHECK-LABEL: NextCond.split:
 ;CHECK: call void @dummy3()
 ;CheCK-LABEL: CallSiteBB:
 ;CHECK: %phi.call = phi i1 [ true, %NextCond.split ], [ false, %Top.split ]
 ;CHECK: call void @foo(i1 %phi.call)
-define void @caller2(i1 %c, %struct.bitmap* %a_elt, %struct.bitmap* %b_elt, %struct.bitmap* %c_elt) {
+define void @caller3(i1 %c, %struct.bitmap* %a_elt, %struct.bitmap* %b_elt, %struct.bitmap* %c_elt) {
 entry:
   br label %Top
 
 Top:
   %tobool1 = icmp eq %struct.bitmap* %a_elt, %b_elt
   br i1 %tobool1, label %CallSiteBB, label %NextCond
 
 NextCond: