[PATCH] D155726: [InstCombine] Test case for D155718+D154064

[Dhruv Chawla via Phabricator via llvm-commits]

0xdc03 marked an inline comment as done.
0xdc03 added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-fold-into-phi.ll:97
+declare zeroext i1 @StrCmp(ptr noundef, ptr noundef)
+declare zeroext i1 @CheckCond()
----------------
nikic wrote:
> nikic wrote:
> > This test looks too complicated for what we actually want to test. I think something like this should be sufficient:
> > ```
> > define i1 @test(i32 %x, i32 %y) {
> > entry:
> >   switch i32 %x, label %bb1 [
> >     i32 0, label %bb2
> >     i32 1, label %bb3
> >   ]
> > 
> > bb1:
> >   br label %bb2
> > 
> > bb2:
> >   %phi1 = phi i32 [ 1, %entry ], [ %y, %bb1 ]
> >   br label %bb3
> > 
> > bb3:
> >   %phi2 = phi i32 [ %phi1, %bb2 ], [ 0, %entry ]
> >   %cmp = icmp ugt i32 %phi2, 1
> >   ret i1 %cmp
> > }
> > ```
> > This would also make it independent of D154064 and allow us to land this patch first.
> Well, I guess it would make sense to //also// add your test to make sure that D154064 doesn't regress it, but in that case we should cut it down a bit. There's a lot of parts that aren't really relevant, e.g. the final block could be just this I think:
> ```
> 
>   %cond3       = phi i32 [ %cond2, %sw.bb ], [ 134, %cond.false2 ], [ 2, %cond.false ], [ 1, %entry ]
>   %cmp         = icmp ult i32 %cond3, 2
>   ret i1 %cmp
> ```
Hmm, I have added another version of this test which is simplified - I still feel that the original test highlights the fold more clearly :shrug:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155726/new/

https://reviews.llvm.org/D155726