[PATCH] D65148: [SimplifyCFG] Bump phi-node-folding-threshold from 2 to 3

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Thank you for taking a look!

In D65148#1632032 <https://reviews.llvm.org/D65148#1632032>, @nikic wrote:

> @lebedev.ri CVP can already determine that the condition is always true:
>
>   define i1 @test(i8 %a, i8 %b) {
>     %conv1 = sext i8 %a to i32
>     %conv3 = sext i8 %b to i32
>     %mul = mul nsw i32 %conv3, %conv1
>     %shr = ashr i32 %mul, 7
>     br label %split
>  
>   split:
>     %icmp = icmp sgt i32 %shr, -128
>     ret i1 %icmp
>   }
>
>
> This becomes `ret i1 true` under `-correlated-propagation`.


Hmm, and unlike `computeKnownBits()`, the `computeConstantRange()` is not recursive, i thought it was.
So using `computeConstantRangeIncludingKnownBits()` instead of `computeConstantRange()` in `simplifyICmpWithConstant()` does not help, we only get:

  LHS_CR
  [-16777216,16777216)

I suppose making it recursive (with depth limit of course) would solve it, but i i'm not sure if it would be too costly?
This *sounds* like a more general fix rather than changing CVP.
Is there third alternative i'm not seeing?

> Unfortunately CVP has some limitations on icmp simplification that makes it only work in cross-BB scenarios.
>  Relaxing those is not entirely straightforward (iirc it had some negative effects due to LVI query order changes).

is there a bug# ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65148