This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Simplify how we drop poison flags in SimplifyDemandedBits.
ClosedPublic

Authored by craig.topper on Jul 11 2022, 12:37 PM.

Details

Summary

As far as I can tell what was happening in the original code is
that the getNode call receives the same operands as the original
node with different SDNodeFlags. The logic inside getNode detects
that the node already exists and intersects the flags into the
existing node and returns it. This results in Op and NewOp for the
TLO.CombineTo call always being the same node.

We may have already called CombineTo as part of the recursive handling.
A second call to CombineTo as we unwind the recursion overwrites
the previous CombineTo. I think this means any time we updated the
poison flags that was the only change that ends up getting made
and we relied on DAGCombiner to revisit and call SimplifyDemandedBits
again. The second time the poison flags wouldn't need to be dropped
and we would keep the CombineTo call from further down the recursion.

If this is all correct, I think we can get away with calling setFlags
(or intersectFlagsWith) to drop the flags and remove the call to
TLO.CombineTo. This way we keep the CombineTo from deeper in the
recursion which should be more efficient.

Diff Detail

Event Timeline

craig.topper created this revision.Jul 11 2022, 12:37 PM
craig.topper requested review of this revision.Jul 11 2022, 12:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 12:37 PM
Herald added a subscriber: wdng. · View Herald Transcript
spatel accepted this revision.Jul 11 2022, 1:25 PM

LGTM - I'm guessing I just copied some nearby code when I added that and never stepped through to see exactly how it proceeded internally; I just saw the asm output was as expected.

This revision is now accepted and ready to land.Jul 11 2022, 1:25 PM
This revision was landed with ongoing or failed builds.Jul 11 2022, 1:42 PM
This revision was automatically updated to reflect the committed changes.