Page MenuHomePhabricator

[LangRef] make per-element poison behavior explicit
ClosedPublic

Authored by spatel on Nov 24 2019, 7:49 AM.

Details

Summary

As discussed in D70246 and PR43958:
https://bugs.llvm.org/show_bug.cgi?id=43958

The LangRef seems ambiguous about the behavior of poison with respect to vectors.

We could go further with text and/or examples - suggestions welcome.

Side note: the LangRef displayed at http://llvm.org/docs/LangRef.html is out-of-date (doesn't show 'freeze' as of right now).

Diff Detail

Event Timeline

spatel created this revision.Nov 24 2019, 7:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 24 2019, 7:49 AM
Herald added a subscriber: mcrosier. · View Herald Transcript

The first bit looks ok to me, thanks!

The semantics for shufflevector doesn't. TL;DR: instructions can't detect undef and have some special behavior dedicated to undef.
(I said before this semantics for shufflevector could be ok, but I was wrong, sorry)

Longer version:
undef can be replaced with any number, including in the shufflevector mask. An undef mask may be replaced with an out-of-bounds index, or point to any element in either of the input elements. If one of those input elements is poison, then the result of shufflevector with undef mask may be poison.
Example:
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 undef, i32 0>
->
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 1, i32 0>
->
%v = <%x[1], %x[0]>

If %x[1] is poison, we get: %v = <poison, %x[0]>

Alternative optimization for the same instruction:
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 undef, i32 0>
->
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 2, i32 0>
->
%v = <undef, %x[0]>

The point is that if mask is undef, we can't promise to give undef.
AFAICT, the only way to fix this is to say that if the mask is out-of-bounds, we yield poison. That's correct because there's nothing worse than poison.

The first bit looks ok to me, thanks!

The semantics for shufflevector doesn't. TL;DR: instructions can't detect undef and have some special behavior dedicated to undef.
(I said before this semantics for shufflevector could be ok, but I was wrong, sorry)

Longer version:
undef can be replaced with any number, including in the shufflevector mask. An undef mask may be replaced with an out-of-bounds index, or point to any element in either of the input elements. If one of those input elements is poison, then the result of shufflevector with undef mask may be poison.
Example:
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 undef, i32 0>
->
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 1, i32 0>
->
%v = <%x[1], %x[0]>

If %x[1] is poison, we get: %v = <poison, %x[0]>

Alternative optimization for the same instruction:
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 undef, i32 0>
->
%v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 2, i32 0>
->
%v = <undef, %x[0]>

The point is that if mask is undef, we can't promise to give undef.
AFAICT, the only way to fix this is to say that if the mask is out-of-bounds, we yield poison. That's correct because there's nothing worse than poison.

Can you raise this on llvm-dev (if it wasn't already)? I'll see if I can isolate the transforms that are currently in conflict and the potential optimization fallout.

This revision is now accepted and ready to land.Nov 29 2019, 6:42 AM
This revision was automatically updated to reflect the committed changes.