This is an archive of the discontinued LLVM Phabricator instance.

[ValueTracking] Handle bitcasts between vec-int-ptr in `isKnownNonZero`
ClosedPublic

Authored by goldstein.w.n on Apr 27 2023, 11:19 PM.

Details

Summary

We where missing these cases so something like:
(bitcast to i32 (or v216 x, <2, 1>))

would not be found to be non-zero.

Diff Detail

Event Timeline

goldstein.w.n created this revision.Apr 27 2023, 11:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 11:19 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
goldstein.w.n requested review of this revision.Apr 27 2023, 11:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 11:19 PM
nikic requested changes to this revision.Apr 28 2023, 1:30 AM

This needs to be much more careful about bitcasts between different lane counts. For your scalar to vector example it's fine, because if each vector element is non-zero then the scalar is also non-zero. The converse, however, is not true.

This revision now requires changes to proceed.Apr 28 2023, 1:30 AM

Fix correctness issue with casting for larger element type to smaller element type

This needs to be much more careful about bitcasts between different lane counts. For your scalar to vector example it's fine, because if each vector element is non-zero then the scalar is also non-zero. The converse, however, is not true.

Done. Only do bitcast now if its from smaller scalar type -> larger scalar type.

nikic requested changes to this revision.Apr 29 2023, 10:01 AM
nikic added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
2775

I think this special case is subsumed by the code below, via isPtrOtPtrVectorTy.

2779

two -> to

2795

I don't think this condition is sufficient. Consider a bitcast from <4 x i3> to <3 x i4>. The new element size needs to be a multiple.

This revision now requires changes to proceed.Apr 29 2023, 10:01 AM

Fix case where src element size is not multiple of dst element size

goldstein.w.n marked 3 inline comments as done.Apr 29 2023, 11:45 AM
goldstein.w.n added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
2795

Fixed + added test

nikic accepted this revision.Apr 29 2023, 11:48 AM

LGTM

llvm/lib/Analysis/ValueTracking.cpp
2770

guranteed -> guaranteed

2776

I believe we can return false here now.

This revision is now accepted and ready to land.Apr 29 2023, 11:48 AM
goldstein.w.n marked an inline comment as done.Apr 29 2023, 11:57 AM
goldstein.w.n added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
2776

Think it makes sense to let computeKnownBits run. More future proof and could help in some cases (i.e the <4xi3> -> <3xi4> is provable in some cases (lsb + msb set in all i3 elements) and computeKnownBits could (although granted doesn't at the moment) get that.

nikic added inline comments.Apr 29 2023, 12:04 PM
llvm/lib/Analysis/ValueTracking.cpp
2776

Okay, fair enough. We'll almost always take this code path anyway.