[PATCH] D76439: [SDAG] fix crash in getNegatedExpression() from altered number of uses

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D76439#1932659 <https://reviews.llvm.org/D76439#1932659>, @steven.zhang wrote:

> Shall we also need to update the cost evaluation for FMUL/FDIV, that is calculating both operands and select the cheapest cost of them ?  And it seems that FADD also have such issues. -(A+B)->-A - B or -B-A
>  Technical speaking, we can't have the same expensive cost for both operands when negating the expression but now, we indeed have sometimes. And we choose one by chance which also would have problems.


Yes, this whole API is unsound - we need to rewrite it to be completely correct. But I think this change makes it at least a little bit safer and more similar to the FMA logic. I can add a FIXME comment for FADD, and try to come up with a similar failure case as this one.

> I am not clear on the origin design but I wonder if we can remove the check of cost before calling the getNegateExpression, but call it directly, which allow to be null value to avoid such kind of out of sync between these two calls.

Yes, if I understand correctly, we will need to integrate the cost+rewrite functions, so they are always in sync.


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

https://reviews.llvm.org/D76439