Page MenuHomePhabricator

[SelectionDAG] Add helper guard to automatically insert flags
ClosedPublic

Authored by qiucf on Sep 9 2020, 4:30 AM.

Details

Summary

The idea is from review of D87037, @spatel 's comments.

This is like FastMathFlagGuard in IR. Since we use SDAG instance to get values, it's with SelectionDAG. By creating a FlagInserter in current scope, all values created by getNode will get the flags, unless explicitly use Inserter->disable().

It's a prototype so in this revision I only enabled in visitFMA to show the same flag propagation as D87037.

Diff Detail

Event Timeline

qiucf created this revision.Sep 9 2020, 4:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2020, 4:30 AM
qiucf requested review of this revision.Sep 9 2020, 4:30 AM
steven.zhang added inline comments.Sep 9 2020, 5:28 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
4413

This is not right as you override the Flags secretly. How about having another version that didn't have the Flags parameter and we will get it from Inserter. And one version that has the Flags if people really want to set the flags manually.(I wonder if there is such case)

qiucf updated this revision to Diff 291463.Sep 13 2020, 10:15 AM

Keep flags in current methods and introduce new methods to auto-insert flags. (maybe can use variadic template methods?)

qiucf updated this revision to Diff 292146.Sep 16 2020, 2:02 AM

Remove flag passing in methods visiting floating-point nodes.

spatel accepted this revision.Sep 22 2020, 1:04 PM

LGTM - this makes it more likely that we propagate as expected and cleans up the current code.
We probably have more cases like D88063, but that can be handled as follow-up patches.

This revision is now accepted and ready to land.Sep 22 2020, 1:04 PM
This revision was automatically updated to reflect the committed changes.