[PATCH] D76638: [SDAG] fix crash in getNegatedExpression() by ignoring transient fadd

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added a comment.

In D76638#1958124 <https://reviews.llvm.org/D76638#1958124>, @spatel wrote:

> In D76638#1957541 <https://reviews.llvm.org/D76638#1957541>, @steven.zhang wrote:
>
> > In D76638#1956952 <https://reviews.llvm.org/D76638#1956952>, @spatel wrote:
> >
> > > Ping. I think this change is useful regardless of whether/how we make a larger fix for getNegatibleCost+getNegatedExpression.
> >
> >
> > I have posted a patch(https://reviews.llvm.org/D77319) to remove the getNegatibleCost. It is almost ready now. I am not sure if it is the best way , and welcome for the comments in advance. Regarding to the opportunities for this patch, can you double confirm it together with my patch to see if there is any improvement ?
>
>
> Thanks for improving it!
>  I'm seeing something interesting with that patch applied for the test that is currently crashing - we re-use a common term in the equation, so that eliminates an instruction. For PPC, it looks like this:
>
>   xssubsp f0, f1, f2
>   xsmulsp f2, f0, f3
>   xssubsp f0, f3, f0
>   xsresp f4, f2
>   xsmulsp f1, f0, f4
>   xsnmsubasp f0, f2, f1
>   xsmaddasp f1, f4, f0
>  
>
>
> But this seems to just be a lucky case because the other test (which is the same math except the fdiv operands are reversed) is not affected:
>
>   xssubsp f0, f1, f2
>   xssubsp f1, f2, f1
>   xsmulsp f0, f0, f3
>   xsaddsp f3, f1, f3
>   xsresp f2,f 0
>   xsmulsp f1, f3, f2
>   xsnmsubasp f3, f0, f1
>   xsmaddasp f1, f2, f3
>  
>


Yeah, it is sensitive to how we combine the nodes. I tried your patch(add FENG check for FADD), and see two extra instructions generated as you mentioned,  on X86. So, we need some positive tests for this patch if it helps the codegen. What do you think ?


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

https://reviews.llvm.org/D76638