[IPConstantProp][NFCI] Improve and modernize tests

[jdoerfert]

Summary

This change is in preparation to reuse these test for the Attributor.
It mainly is to remove UB, make it clear what is tested, and use
"modern" run lines.

[jdoerfert]

Add two more that splipped through the cracks

[jdoerfert]

llvm/test/Transforms/IPConstantProp/2009-09-24-byval-ptr.ll

27

This matches closer the check lines above and it also does not force us to fold the gep into the load (which is not what is important here).

llvm/test/Transforms/IPConstantProp/PR26044.ll

62

Having the check lines at the end makes it hard to relate them to code so I went with a update of all lines here.

llvm/test/Transforms/IPConstantProp/arg-count-mismatch.ll

57

The calls were essentially dead and could be removed, make sure that is not the optimization we are looking for.

llvm/test/Transforms/IPConstantProp/solve-after-each-resolving-undefs-for-function.ll

18

Branch on undef is UB, I think, and even if not, this method did trivially return 10 even without resolving any constant branches. Make sure this is not the case anymore and add a "for sure UB-free" path while we are at it.

llvm/test/Transforms/IPConstantProp/user-with-multiple-uses.ll

33

These attributes are given already (and arguably wrong from a more fundamental standpoint) so it is questionable why we should check for them in the first place.

[fhahn]

It seems some of those files test IPSCCP while others test the IPConstantPropagation pass. Having them both in there is a bit confusing. I think most IPSCCP tests live in test/Transforms/SCCP, which they share the code with. It might be worth to move the IPSCCP tests from IPConstantPropagation to test/Transforms/SCCP or create a new IPSCCP test directory.

And FWIW, I think we might want to consider removing the IPConstantPropagation pass, because it looks like it is not used in any pipelines and IPSCCP is more powerful.

[jdoerfert]

In D69747#1731333, @fhahn wrote:

It seems some of those files test IPSCCP while others test the IPConstantPropagation pass. Having them both in there is a bit confusing. I think most IPSCCP tests live in test/Transforms/SCCP, which they share the code with. It might be worth to move the IPSCCP tests from IPConstantPropagation to test/Transforms/SCCP or create a new IPSCCP test directory.

I don't mind either way. Do you want me to do it in this commit or do it after?

And FWIW, I think we might want to consider removing the IPConstantPropagation pass, because it looks like it is not used in any pipelines and IPSCCP is more powerful.

Agreed, see: https://reviews.llvm.org/D69656#1728773

---

Any comments wrt. the changes I made?

[davide]

This patch looks good to me.

Now, on the more general discussion. There are some cases that are caught by IPCP [but not IPSCCP]. We can craft examples, if we want, but it's unclear how often they happen in practice.
Having IPCP in tree doesn't seem to be a huge maintenance burden. OTOH, the algorithm is extremely naive and the pass isn't used anywhere, so if somebody wants to remove it, I wouldn't have any objections.

Also note that the cases that are caught by IPCP and not by IPSCCP are due to the fact that IPCP isn't really an analysis, and replaces stuffs as it goes. Having a context-sensitive IPSCCP pass could alleviate some of the differences, but it's unclear whether it's worth the complexity.

[davide]

[if you probably replace opt -ipconstprop with opt -ipsccp in the tests you'll stumble upon the cases I pointed out.

[fhahn]

In D69747#1731347, @davide wrote:

This patch looks good to me.

Now, on the more general discussion. There are some cases that are caught by IPCP [but not IPSCCP]. We can craft examples, if we want, but it's unclear how often they happen in practice.
Having IPCP in tree doesn't seem to be a huge maintenance burden. OTOH, the algorithm is extremely naive and the pass isn't used anywhere, so if somebody wants to remove it, I wouldn't have any objections.

Interesting! That's unfortunate, with the pass being disabled. I'll try to take a look at those when I have a bit of time.

[jdoerfert]

In D69747#1731350, @davide wrote:

[if you probably replace opt -ipconstprop with opt -ipsccp in the tests you'll stumble upon the cases I pointed out.

I briefly tried this and it looks like the two main features ipconstprop has and ipsccp not are: 1) multi-return/extractvalue support, 2) callback support

[jdoerfert]

NFC

[merge_guards_bot]

Build result: FAILURE -
Log files: console-log.txt, CMakeCache.txt

llvm/test/Transforms/IPConstantProp/2008-06-09-WeakProp.ll

-; RUN: opt < %s -ipconstprop -S | grep "ret i32 %r"
+; RUN: opt < %s -ipconstprop -S | FileCheck %s
 ; Should not propagate the result of a weak function.
 ; PR2411
 
+; CHECK: ret i32 %r
+
 define weak i32 @foo() nounwind  {
 entry:
         ret i32 1
 }
 
 define i32 @main() nounwind  {
 entry:
         %r = call i32 @foo( ) nounwind
         ret i32 %r
 }
 

llvm/test/Transforms/IPConstantProp/2009-09-24-byval-ptr.ll

 return:                                           ; preds = %entry
   ret void
 }
 
 define internal i32 @vfu2(%struct.MYstr* byval align 4 %u) nounwind readonly {
 entry:
   %0 = getelementptr %struct.MYstr, %struct.MYstr* %u, i32 0, i32 1 ; <i32*> [#uses=1]
   %1 = load i32, i32* %0
-; CHECK: load i32, i32* getelementptr inbounds (%struct.MYstr, %struct.MYstr* @mystr, i32 0, i32 1)
+; CHECK: %struct.MYstr* @mystr, i{{..}} 0, i32 1
   %2 = getelementptr %struct.MYstr, %struct.MYstr* %u, i32 0, i32 0 ; <i8*> [#uses=1]
   %3 = load i8, i8* %2
-; CHECK: load i8, i8* getelementptr inbounds (%struct.MYstr, %struct.MYstr* @mystr, i32 0, i32 0)
+; CHECK: %struct.MYstr* @mystr, i{{..}} 0, i32 0
   %4 = zext i8 %3 to i32
   %5 = add i32 %4, %1
   ret i32 %5
 }
 
 define i32 @unions() nounwind {
 entry:
   call void @vfu1(%struct.MYstr* byval align 4 @mystr) nounwind
   %result = call i32 @vfu2(%struct.MYstr* byval align 4 @mystr) nounwind
-
+; CHECK: ret i32 %result
   ret i32 %result
 }
 

llvm/test/Transforms/IPConstantProp/PR16052.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes 
 ; RUN: opt < %s -S -ipsccp | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 define i64 @fn2() {
+; CHECK-LABEL: define {{[^@]+}}@fn2()
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CALL2:%.*]] = call i64 @fn1(i64 undef)
+; CHECK-NEXT:    ret i64 [[CALL2]]
+;
 entry:
   %conv = sext i32 undef to i64
   %div = sdiv i64 8, %conv
   %call2 = call i64 @fn1(i64 %div)
   ret i64 %call2
 }
 
-; CHECK-DAG: define i64 @fn2(
-; CHECK: %[[CALL:.*]] = call i64 @fn1(i64 undef)
-
 define internal i64 @fn1(i64 %p1) {
+; CHECK-LABEL: define {{[^@]+}}@fn1
+; CHECK-SAME: (i64 [[P1:%.*]])
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[COND:%.*]] = select i1 undef, i64 undef, i64 undef
+; CHECK-NEXT:    ret i64 [[COND]]
+;
 entry:
   %tobool = icmp ne i64 %p1, 0
   %cond = select i1 %tobool, i64 %p1, i64 %p1
   ret i64 %cond
 }
-
-; CHECK-DAG: define internal i64 @fn1(
-; CHECK: %[[SEL:.*]] = select i1 undef, i64 undef, i64 undef
-; CHECK: ret i64 %[[SEL]]

llvm/test/Transforms/IPConstantProp/PR26044.ll

-; RUN: opt < %s -S -ipsccp | FileCheck %s
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
+; RUN: opt < %s -S -ipsccp | FileCheck %s --check-prefixes=IPSCCP
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-define void @fn2() {
+define void @fn2(i32* %P) {
+; IPSCCP-LABEL: define {{[^@]+}}@fn2
+; IPSCCP-SAME: (i32* [[P:%.*]])
+; IPSCCP-NEXT:  entry:
+; IPSCCP-NEXT:    br label [[IF_END:%.*]]
+; IPSCCP:       for.cond1:
+; IPSCCP-NEXT:    br i1 false, label [[IF_END]], label [[IF_END]]
+; IPSCCP:       if.end:
+; IPSCCP-NEXT:    [[CALL:%.*]] = call i32 @fn1(i32 undef)
+; IPSCCP-NEXT:    store i32 [[CALL]], i32* [[P]]
+; IPSCCP-NEXT:    br label [[FOR_COND1:%.*]]
+;
 entry:
   br label %if.end
 
 for.cond1:                                        ; preds = %if.end, %for.end
   br i1 undef, label %if.end, label %if.end
 
 if.end:                                           ; preds = %lbl, %for.cond1
   %e.2 = phi i32* [ undef, %entry ], [ null, %for.cond1 ], [ null, %for.cond1 ]
   %0 = load i32, i32* %e.2, align 4
   %call = call i32 @fn1(i32 %0)
+  store i32 %call, i32* %P
   br label %for.cond1
 }
 
 define internal i32 @fn1(i32 %p1) {
+; IPSCCP-LABEL: define {{[^@]+}}@fn1
+; IPSCCP-SAME: (i32 [[P1:%.*]])
+; IPSCCP-NEXT:  entry:
+; IPSCCP-NEXT:    [[COND:%.*]] = select i1 undef, i32 undef, i32 undef
+; IPSCCP-NEXT:    ret i32 [[COND]]
+;
 entry:
   %tobool = icmp ne i32 %p1, 0
   %cond = select i1 %tobool, i32 %p1, i32 %p1
   ret i32 %cond
 }
 
-define void @fn_no_null_opt() #0 {
+define void @fn_no_null_opt(i32* %P) #0 {
+; IPSCCP-LABEL: define {{[^@]+}}@fn_no_null_opt
+; IPSCCP-SAME: (i32* [[P:%.*]])
+; IPSCCP-NEXT:  entry:
+; IPSCCP-NEXT:    br label [[IF_END:%.*]]
+; IPSCCP:       for.cond1:
+; IPSCCP-NEXT:    br i1 false, label [[IF_END]], label [[IF_END]]
+; IPSCCP:       if.end:
+; IPSCCP-NEXT:    [[TMP0:%.*]] = load i32, i32* null, align 4
+; IPSCCP-NEXT:    [[CALL:%.*]] = call i32 @fn0(i32 [[TMP0]])
+; IPSCCP-NEXT:    store i32 [[CALL]], i32* [[P]]
+; IPSCCP-NEXT:    br label [[FOR_COND1:%.*]]
+;
 entry:
   br label %if.end
 
 for.cond1:                                        ; preds = %if.end, %for.end
   br i1 undef, label %if.end, label %if.end
 
 if.end:                                           ; preds = %lbl, %for.cond1
   %e.2 = phi i32* [ undef, %entry ], [ null, %for.cond1 ], [ null, %for.cond1 ]
   %0 = load i32, i32* %e.2, align 4
   %call = call i32 @fn0(i32 %0)
+  store i32 %call, i32* %P
   br label %for.cond1
 }
 
 define internal i32 @fn0(i32 %p1) {
+; IPSCCP-LABEL: define {{[^@]+}}@fn0
+; IPSCCP-SAME: (i32 [[P1:%.*]])
+; IPSCCP-NEXT:  entry:
+; IPSCCP-NEXT:    [[TOBOOL:%.*]] = icmp ne i32 [[P1]], 0
+; IPSCCP-NEXT:    [[COND:%.*]] = select i1 [[TOBOOL]], i32 [[P1]], i32 [[P1]]
+; IPSCCP-NEXT:    ret i32 [[COND]]
+;
 entry:
   %tobool = icmp ne i32 %p1, 0
   %cond = select i1 %tobool, i32 %p1, i32 %p1
   ret i32 %cond
 }
 
 attributes #0 = { "null-pointer-is-valid"="true" }
-
-; CHECK-LABEL: define void @fn2(
-; CHECK: call i32 @fn1(i32 undef)
-
-; CHECK-LABEL: define internal i32 @fn1(
-; CHECK:%[[COND:.*]] = select i1 undef, i32 undef, i32 undef
-; CHECK: ret i32 %[[COND]]
-
-; CHECK-LABEL: define void @fn_no_null_opt(
-; CHECK: call i32 @fn0(i32 %0)
-
-; CHECK-LABEL: define internal i32 @fn0(
-; CHECK:%[[TOBOOL:.*]] = icmp ne i32 %p1, 0
-; CHECK:%[[COND:.*]] = select i1 %[[TOBOOL]], i32 %p1, i32 %p1
-; CHECK: ret i32 %[[COND]]

llvm/test/Transforms/IPConstantProp/arg-count-mismatch.ll

;
   ret i16 0
 }
 
 ;-------------------------------------------------------------------------------
 ; Additional tests to verify that we still optimize when having a mismatch
 ; in argument count due to varargs (as long as all non-variadic arguments have
 ; been provided),
 
-define dso_local void @vararg_tests(i16 %a) {
+define dso_local i16 @vararg_tests(i16 %a) {
   %call1 = call i16 (i16, ...) @vararg_prop(i16 7, i16 8, i16 %a)
   %call2 = call i16 bitcast (i16 (i16, i16, ...) * @vararg_no_prop to i16 (i16) *) (i16 7)
-  ret void
+  %add = add i16 %call1, %call2
+  ret i16 %add
 }
 
 define internal i16 @vararg_prop(i16 %p1, ...) {
 ; CHECK-LABEL: define internal i16 @vararg_prop(
 ; CHECK-NEXT:    ret i16 7
 ;
   ret i16 %p1
 }
 
 define internal i16 @vararg_no_prop(i16 %p1, i16 %p2, ...) {
 ; CHECK-LABEL: define internal i16 @vararg_no_prop(
 ; CHECK-NEXT:    ret i16 [[P1:%.*]]
 ;
   ret i16 %p1
 }
 

llvm/test/Transforms/IPConstantProp/recursion.ll

-; RUN: opt < %s -ipconstprop -deadargelim -S | not grep %X
+; RUN: opt < %s -ipconstprop -deadargelim -S | FileCheck %s
+
+; CHECK-NOT: %X
+
 define internal i32 @foo(i32 %X) {
         %Y = call i32 @foo( i32 %X )            ; <i32> [#uses=1]
         %Z = add i32 %Y, 1              ; <i32> [#uses=1]
         ret i32 %Z
 }
 
 define void @bar() {
         call i32 @foo( i32 17 )         ; <i32>:1 [#uses=0]
         ret void
 }
 

llvm/test/Transforms/IPConstantProp/return-argument.ll

-; RUN: opt < %s -ipconstprop -S > %t
-; RUN: cat %t | grep "store i32 %Z, i32\* %Q"
-; RUN: cat %t | grep "add i32 1, 3"
+; RUN: opt < %s -ipconstprop -S | FileCheck %s
+
+; CHECK: add i32 1, 3
+; CHECK: store i32 %Z, i32* %Q
 
 ;; This function returns its second argument on all return statements
 define internal i32* @incdec(i1 %C, i32* %V) {
         %X = load i32, i32* %V
         br i1 %C, label %T, label %F
 
 T:              ; preds = %0
         %X1 = add i32 %X, 1

llvm/test/Transforms/IPConstantProp/return-constant.ll

-; RUN: opt < %s -ipconstprop -instcombine -S | grep "ret i1 true" | count 2
+; RUN: opt < %s -ipconstprop -instcombine -S | FileCheck %s
+
+; CHECK: ret i1 true
+; CHECK: ret i1 true
+; CHECK-NOT: ret i1 true
+
 define internal i32 @foo(i1 %C) {
         br i1 %C, label %T, label %F
 
 T:              ; preds = %0
         ret i32 52
 
 F:              ; preds = %0
         ret i32 52

llvm/test/Transforms/IPConstantProp/return-constants.ll

-; RUN: opt < %s -ipconstprop -S > %t
+; RUN: opt < %s -ipconstprop -S | FileCheck %s
+
 ;; Check that the 21 constants got propagated properly
-; RUN: cat %t | grep "%M = add i32 21, 21"
+; CHECK: %M = add i32 21, 21
 ;; Check that the second return values didn't get propagated
-; RUN: cat %t | grep "%N = add i32 %B, %D"
+; CHECK: %N = add i32 %B, %D
 
 %0 = type { i32, i32 }
 
 define internal %0 @foo(i1 %Q) {
   br i1 %Q, label %T, label %F
 
 T:                                                ; preds = %0
   %mrv = insertvalue %0 undef, i32 21, 0

llvm/test/Transforms/IPConstantProp/solve-after-each-resolving-undefs-for-function.ll

 ; RUN: opt < %s -ipsccp -S | FileCheck %s
 
 ; CHECK-LABEL: @testf(
 ; CHECK:         ret i32 undef
 ;
-define internal i32 @testf() {
+define internal i32 @testf(i1 %c) {
 entry:
+  br i1 %c, label %if.cond, label %if.end
+
+if.cond:
   br i1 undef, label %if.then, label %if.end
 
 if.then:                                          ; preds = %entry, %if.then
-  br label %if.end
+  ret i32 11
 
 if.end:                                          ; preds = %if.then1, %entry
   ret i32 10
 }
 
 ; CHECK-LABEL: @test1(
 ; CHECK:         ret i32 undef
 ;
-define internal i32 @test1() {
+define internal i32 @test1(i1 %c) {
 entry:
   br label %if.then
 
 if.then:                                          ; preds = %entry, %if.then
-  %call = call i32 @testf()
+  %call = call i32 @testf(i1 %c)
   %res = icmp eq i32 %call, 10
   br i1 %res, label %ret1, label %ret2
 
 ret1:                                           ; preds = %if.then, %entry
   ret i32 99
 
 ret2:                                           ; preds = %if.then, %entry
   ret i32 0
 }
 
 ; CHECK-LABEL: @main(
-; CHECK-NEXT:    %res = call i32 @test1()
+; CHECK-NEXT:    %res = call i32 @test1(
 ; CHECK-NEXT:    ret i32 99
 ;
-define i32 @main() {
-  %res = call i32 @test1()
+define i32 @main(i1 %c) {
+  %res = call i32 @test1(i1 %c)
   ret i32 %res
 }

llvm/test/Transforms/IPConstantProp/user-with-multiple-uses.ll

entry:
   ]
 
 sw.default:
   ret i32 123
 
 return:
   ret i32 0
 }
-
-; CHECK: attributes #0 = { noreturn nounwind }
-; CHECK: attributes #1 = { nounwind }