This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Use `computeKnownBits` if `Op` is not recognized by `isKnownNeverZero`
ClosedPublic

Authored by goldstein.w.n on Apr 25 2023, 2:26 PM.

Details

Summary

The current logic is pretty limitted unless the Op is a
constant. This at least covers more obvious cases.

Diff Detail

Event Timeline

goldstein.w.n created this revision.Apr 25 2023, 2:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2023, 2:26 PM
goldstein.w.n requested review of this revision.Apr 25 2023, 2:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2023, 2:26 PM
craig.topper added inline comments.Apr 25 2023, 2:43 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5067

I think you want Depth -1 here to counteract the increment earlier. The switch in computeKnownBits should be the same depth as the switch in this function.

goldstein.w.n marked an inline comment as done.Apr 25 2023, 3:01 PM
craig.topper added inline comments.Apr 25 2023, 6:55 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5067

return computeKnownBits(Op, Depth - 1).isNonZero(). KnownBits already has a wrapper so we don't have to reference the One field directly.

goldstein.w.n marked an inline comment as done.Apr 25 2023, 9:44 PM

use isNonZero

This revision is now accepted and ready to land.Apr 25 2023, 10:07 PM
foad added inline comments.Apr 26 2023, 12:11 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5067

It would be less confusing overall to *not* increment Depth.

goldstein.w.n added inline comments.Apr 26 2023, 8:35 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5067

I think in this case is but it makes it so that in all the recursive cases you can just copy Depth rather than Depth + 1. Its also what we do in ValueTracking to think it makes sense to keep it similiar.

foad accepted this revision.Apr 27 2023, 1:16 AM
This revision was landed with ongoing or failed builds.May 13 2023, 12:37 PM
This revision was automatically updated to reflect the committed changes.