This is an archive of the discontinued LLVM Phabricator instance.

[FPEnv][AMDGPU] Disable FSUB(-0,X)->FNEG(X) DAGCombine when subnormals are flushed
ClosedPublic

Authored by cameron.mcinally on Dec 14 2020, 2:01 PM.

Details

Summary

One more push on this before the year is over...

This patch disables the FSUB(-0,X)->FNEG(X) DAG combine when we're flushing subnormals. It requires updating the existing AMDGPU tests to use the fneg IR instruction, in place of the old fsub(-0,X) canonical form, since AMDGPU is the only backend currently checking the DenormalMode flags.

Note that this will require follow-up optimizations to make sure the fsub(-0,X) form is handled appropriately. But I think that can be done in a separate patch.

Diff Detail

Event Timeline

cameron.mcinally requested review of this revision.Dec 14 2020, 2:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 14 2020, 2:01 PM
cameron.mcinally retitled this revision from [FPEnv][AMDGPU] Disable FSUB(-0,X)->FNEG(X) DAGCombine when subnormals are preserved to [FPEnv][AMDGPU] Disable FSUB(-0,X)->FNEG(X) DAGCombine when subnormals are flushed.Dec 14 2020, 2:02 PM
cameron.mcinally edited the summary of this revision. (Show Details)
arsenm added inline comments.Dec 14 2020, 3:03 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
13376

This will also change the behavior on signaling nans, which will no longer quiet

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
13376

Good catch.

I'm playing with a change to only allow this transform when NoNaNs are present, but it's breaking a lot of tests across a number of targets. How would you feel about leaving that change to a separate patch? I think this subnormal Diff is a step in the right direction, and doesn't regress from the current behavior.

arsenm added inline comments.Dec 15 2020, 2:02 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
13376

Should get a fixme at least (we also have an isKnownNeverSNan although it's fairly limited)

Add FIXME comment.

cameron.mcinally marked 2 inline comments as done.Dec 17 2020, 8:52 AM
arsenm accepted this revision.Jan 4 2021, 10:05 AM
This revision is now accepted and ready to land.Jan 4 2021, 10:05 AM