This is an archive of the discontinued LLVM Phabricator instance.

[LegalizeTypes][LegalDAG][X86] Copy SDNodeFlags or at least fpexcept flag when legalizing strict fp nodes
Needs ReviewPublic

Authored by craig.topper on Dec 17 2019, 12:23 AM.

Details

Summary

This is needed to preserve the FP except information. These were the obvious easy cases. More patches definitely needed.

We should consider changing our representation and the default semantics for this property. Using what is effectively a fast math flag is dangerous since the API was designed for them to be lost without hurting correctness.

Random thoughts on how we might do that
-Change the polarity and require all the existing non-strict transforms to pass noexcept everywhere. This seems like it would require touching a lot of places and carefully auditing.
-Change the polarity, but make it only have meaning on STRICT nodes. Ignore it on other nodes.
-Change the polarity, but ignore it inside functions that don't have the strictfp attribute.

Diff Detail

Event Timeline

craig.topper created this revision.Dec 17 2019, 12:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2019, 12:23 AM
craig.topper marked an inline comment as done.
craig.topper added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
7434

Not adding flags here, because none of the other getNode functions do for this same spot.

Adding some more cases where flags are being lost. I'll retitle this review later. And I still need to add tests.

craig.topper retitled this revision from [LegalizeTypes] Copy SDNodeFlags when scalarizing, splitting, or unrolling strict fp nodes. to [LegalizeTypes][LegalDAG][X86] Copy SDNodeFlags or at least fpexcept flag when legalizing strict fp nodes.
craig.topper edited the summary of this revision. (Show Details)

How about the status of handing down the flags? Do we have a solution for flags in all situation?

How about the status of handing down the flags? Do we have a solution for flags in all situation?

https://reviews.llvm.org/D87361 made things easier, but we still need to do work at every location. Every transform has to be looked at for fast math safety.

qiucf added a subscriber: qiucf.Nov 5 2020, 9:02 PM