[PATCH] D104247: [DAGCombine] reassoc flag shouldn't enable contract

Pengfei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 18 19:13:23 PDT 2021


pengfei added a comment.

In D104247#2827116 <https://reviews.llvm.org/D104247#2827116>, @jsji wrote:

> In D104247#2827089 <https://reviews.llvm.org/D104247#2827089>, @spatel wrote:
>
>> In code that I've looked at (mostly C compiled with -ffast-math), we always have `contract` when we have `reassoc`, so I don't see much practical difference.
>> Can you explain more how we could benefit from this change - in the example in the description, we would have 4 instructions rather than 3 if we use FMA - is that better?
>
> Yes, you are right, the performance of having FMA should be better. However, we have quite some scenarios that users care about precision more than performance, they want to precise control of when FMA can be generated. So The major motivation of this is to ensure that we respect the IR semantics. For users that care about performance, we still can get them through default global option or emitting respect flag in IR.

AFAIU, the use of FMA doesn't decline the precision (but may improve a bit, which is also unexpected in some circumstance). But `reassoc` flag does affect precision much. We had some discussion on D99675 <https://reviews.llvm.org/D99675>, `contract` is acceptable even we want to disable `reassoc` for the sake of precision. That said, `contract` has much less influence on precision than `reassoc`. So I don't see any necessity to use `reassoc` without `contract`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104247



More information about the llvm-commits mailing list