This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Add/Improve cases in `isKnownNeverZero`
ClosedPublic

Authored by goldstein.w.n on Aug 1 2023, 12:33 AM.

Details

Summary
  1. Handle casts a bit more cleanly just with a loop rather than with recursion.
  1. Add additional cases for smin/smax
  1. For shifts we can also deduce non-zero if the maximum shift amount on the known 1s is non-zero.

Diff Detail

Event Timeline

goldstein.w.n created this revision.Aug 1 2023, 12:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2023, 12:33 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
goldstein.w.n requested review of this revision.Aug 1 2023, 12:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2023, 12:33 AM
arsenm added a subscriber: arsenm.Aug 1 2023, 3:55 PM
arsenm added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5073

Looking through any_extend seems wrong, the high bits may not be 0 as they're undefined

5082

Should this just go through the recursive case like normal?

5132–5135

Should try the RHS operand first as it's canonically simpler

goldstein.w.n marked 2 inline comments as done.Aug 2 2023, 9:32 AM
goldstein.w.n added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5082

Personally think its clearer to have casts simply handled ahead of time, but no strong attachment/opinion so adding back to recursive case.

5132–5135

Hmm? In a SRL we can't canonicalize constants to RHS. Also doing LHS first gives us a fast out (no need to compute RHS if we have negative value, but we can't get same early out with RHS).

goldstein.w.n marked an inline comment as done.

Remove any_extend, add other casts back to main switch

RKSimon edited the summary of this revision. (Show Details)Aug 4 2023, 6:10 AM
RKSimon edited the summary of this revision. (Show Details)
RKSimon added inline comments.Aug 4 2023, 6:13 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5100

ult? - shift by bitwidth is undefined.

RKSimon added inline comments.Aug 4 2023, 6:18 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5101

Use isNonZero() instead?

goldstein.w.n marked an inline comment as done.

Fix shift cnt check

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5101

apint doens't have isNonZero.

RKSimon added inline comments.Aug 4 2023, 9:53 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5101

I missed that! Would it improve the analysis if we used ValKnown.shl(CntKnown).isNonZero() instead?

goldstein.w.n added inline comments.Aug 4 2023, 11:43 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
5101

I don't think so. Known bits needs to actually figure out which is set. I.e if we have:
2 << (x & 7), known bits won't be able to set anything as it doesn't know any ones (value can be 2, 4, 8,...256 but there are no overlapping bits none can be set), whereas this logic still allows us to prove non-zero.

RKSimon accepted this revision.Aug 5 2023, 9:53 AM

LGTM - cheers!

This revision is now accepted and ready to land.Aug 5 2023, 9:53 AM
This revision was landed with ongoing or failed builds.Aug 15 2023, 11:59 PM
This revision was automatically updated to reflect the committed changes.