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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

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