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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

bump

In D65148#1644016 <https://reviews.llvm.org/D65148#1644016>, @lebedev.ri wrote:

> In D65148#1629103 <https://reviews.llvm.org/D65148#1629103>, @lebedev.ri wrote:
>
> > In D65148#1629010 <https://reviews.llvm.org/D65148#1629010>, @dmgreen wrote:
> >
> > > > That is a valid transformation: https://rise4fun.com/Alive/bxq
> > >
> > > I see, because of the truncs back into an i16. That makes sense.
> >
>
>
> Ok, and that one is done too:
>
>   $ ./bin/clang -target arm-none-eabi -mcpu=cortex-m0plus -O3 -mllvm -phi-node-folding-threshold=2 -S -o old.s /tmp/test.c 
>   $ ./bin/clang -target arm-none-eabi -mcpu=cortex-m0plus -O3 -mllvm -phi-node-folding-threshold=3 -S -o new.s /tmp/test.c 
>   $ diff old.s new.s 
>   $ <no diff>
>   $ sha512sum *.s
>   9332ab2169151fa21ab03e1fb218178aa16bed2c0dfdd0a40b178a75dbd4a865ba7a7b1b4b37f252928ea794e38776260f84f6c1ae3684d5a445118f518351e2  new.s
>   9332ab2169151fa21ab03e1fb218178aa16bed2c0dfdd0a40b178a75dbd4a865ba7a7b1b4b37f252928ea794e38776260f84f6c1ae3684d5a445118f518351e2  old.s
>
>
>
>
> In D65148#1631970 <https://reviews.llvm.org/D65148#1631970>, @lebedev.ri wrote:
>
> > Thank you for taking a look!
> >
> > In D65148#1631933 <https://reviews.llvm.org/D65148#1631933>, @dmgreen wrote:
> >
> > > Fair enough.
> >
> >
> >
> >
> > > One last issue, which might come up from this change. This time with a multiply, that was previously able to simplify one of the compares to true, I think in CVP.  Same __SSAT as before.
> > > 
> > >   void arm_mult_q7(const signed char * pSrcA, const signed char * pSrcB, signed char * pDst, unsigned  blockSize)
> > >   {
> > >     unsigned  blkCnt = blockSize;
> > >     while (blkCnt > 0U)
> > >     {
> > >       *pDst++ = (signed char) __SSAT((((short) (*pSrcA++) * (*pSrcB++)) >> 7), 8);
> > >       blkCnt--;
> > >     }
> > >   }
>


@dmgreen & others: So now the question is, do we believe this general missing fold is a blocker here?

> In D65148#1632130 <https://reviews.llvm.org/D65148#1632130>, @lebedev.ri wrote:
> 
>> In D65148#1632032 <https://reviews.llvm.org/D65148#1632032>, @nikic wrote:
>>
>> > @lebedev.ri CVP can already determine that the condition is always true:
>> >  ...
>> >  This becomes `ret i1 true` under `-correlated-propagation`.
>> >  ...
>> >  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# ?

@nikic could you please either CC me to existing bugreport, or file one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65148