Page MenuHomePhabricator

Add coverage for inexact constant folding for multiplication in Selection DAG
AbandonedPublic

Authored by mcberg2017 on Jun 15 2020, 5:36 PM.

Details

Summary

Guard constant folding using inexact testing before forwarding results, also allow Unsafe to gate the optimization. This functionality was initially added to GlobalIsel internally at Apple. We are porting it into SDAG for general use. Others may want to port it back into their GlobalIsel representation after the fact. The modified tests primarily record the difference in inexact constant folding.

Diff Detail

Event Timeline

mcberg2017 created this revision.Jun 15 2020, 5:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2020, 5:36 PM
arsenm added inline comments.Jun 15 2020, 6:45 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5139–5140

I'm not sure I understand why this would depend on unsafe math. Why does it matter if it was inexact? The unconstrained operations have the defined rounding mode

mcberg2017 marked an inline comment as done.Jun 15 2020, 7:26 PM
mcberg2017 added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5139–5140

With Unsafe any contracts concerning the ability to deliver infinite precision are broken, ergo Unsafe overrides and allows the result regardless as precision is relaxed and its behavior as well.

I find the patch description to be unintuitive.
It doesn't say *why* this is being done, what problems that causes, etc.

arsenm added inline comments.Jun 16 2020, 5:52 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5139–5140

But why would there be any expectation of infinite precision here? It's the precision of the fltSemantics. I also don't understand why this would be tied to the global UnsafeFPMath option, and not something more refined

mcberg2017 marked an inline comment as done.Jun 16 2020, 9:07 AM
mcberg2017 added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5139–5140

In the way of something more refined, we could use fmf:

  • Flags that would not add context: nnan, nsz, ninf, contract, arcp.
  • However: afn and NoFPExcept could easily be used. The flag reassoc implies relaxed precision, so possibly that too.

Some discussion?

mcberg2017 marked an inline comment as done.Jun 16 2020, 10:53 AM
mcberg2017 added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5139–5140

In fact NoFPExcept may be sufficient all by itself as a guard along with the test for inexact as we are talking about fp exception states and delivering a signal for inexact. I will wait to make the change to see if there is agreement.

arsenm added inline comments.Jun 16 2020, 11:04 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5139–5140

nofpexcept is implied by the fact that this isn't STRICT_FMUL

lebedev.ri requested changes to this revision.Jun 16 2020, 11:05 AM

Please can you explain what problem is this fixing.

This revision now requires changes to proceed.Jun 16 2020, 11:05 AM
mcberg2017 abandoned this revision.Jun 16 2020, 2:05 PM

From discussion, it seems we are fine with fmul and constant folding the way it is versed in the code today (unrevised).