<div dir="ltr">I believe 

ShuffleVectorInst::isValidOperands checks for the mask already. It's called by Verifier::visitShuffleVectorInst. It's also called by the LL Parser and the an assert in the shuffle vector constructor.<div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><br></div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 27, 2019 at 9:49 AM Nuno Lopes via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Ok, makes sense.<br>
My suggestion is that we patch the IR Verifier to ensure that the mask is<br>
indeed a vector of constants and/or undefs. Right now it only runs the<br>
standard checks for instructions.<br>
We will also run Alive2 on the test suite to make sure undef is never<br>
replaced in practice.<br>
<br>
Thanks,<br>
Nuno<br>
<br>
-----Original Message-----<br>
From: Eli Friedman <<a href="mailto:efriedma@quicinc.com" target="_blank">efriedma@quicinc.com</a>> <br>
Sent: 27 de novembro de 2019 01:10<br>
To: Nuno Lopes <<a href="mailto:nuno.lopes@ist.utl.pt" target="_blank">nuno.lopes@ist.utl.pt</a>>; LLVMdev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
Cc: <a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>; Juneyoung Lee <<a href="mailto:juneyoung.lee@sf.snu.ac.kr" target="_blank">juneyoung.lee@sf.snu.ac.kr</a>>;<br>
<a href="mailto:zhengyang-liu@hotmail.com" target="_blank">zhengyang-liu@hotmail.com</a>; John Regehr <<a href="mailto:regehr@cs.utah.edu" target="_blank">regehr@cs.utah.edu</a>><br>
Subject: RE: LangRef semantics for shufflevector with undef mask is<br>
incorrect<br>
<br>
The shuffle mask of a shufflevector is special: it's required to be a<br>
constant in a specific form.  From LangRef: "The shuffle mask operand is<br>
required to be a constant vector with either constant integer or undef<br>
values."  So really, we can resolve this any way we want; "undef" in this<br>
context doesn't have to mean the same thing as "undef" in other contexts.<br>
Formally, at the LangRef level, we can state that the shuffle mask is not an<br>
operand of a shufflevector; instead, it's not a value at all.  It's just a<br>
description of the shuffle, defined with a grammar similar to a vector<br>
constant.  Then we can talk about shuffle masks where an element is the<br>
string "undef", unrelated to the general notion of an undef value.<br>
<br>
In that context, the existing LangRef description of the result of<br>
shufflevector can be interpreted to mean exactly what it says. This would<br>
imply it's forbidden to transform the shuffle mask "<2 x i32> <i32 undef,<br>
i32 0>" to "<2 x i32> <i32 1, i32 0>" if the input might contain poison.<br>
(And this is the same logic that led to <a href="https://reviews.llvm.org/D70246" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70246</a> .)<br>
<br>
That said, long-term, we probably want to switch shufflevector to produce<br>
poison.<br>
<br>
-Eli<br>
<br>
> -----Original Message-----<br>
> From: Nuno Lopes <<a href="mailto:nuno.lopes@ist.utl.pt" target="_blank">nuno.lopes@ist.utl.pt</a>><br>
> Sent: Tuesday, November 26, 2019 3:20 PM<br>
> To: LLVMdev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
> Cc: <a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>; Eli Friedman <<a href="mailto:efriedma@quicinc.com" target="_blank">efriedma@quicinc.com</a>>; <br>
> Juneyoung Lee <<a href="mailto:juneyoung.lee@sf.snu.ac.kr" target="_blank">juneyoung.lee@sf.snu.ac.kr</a>>; <a href="mailto:zhengyang-liu@hotmail.com" target="_blank">zhengyang-liu@hotmail.com</a>; <br>
> John Regehr <<a href="mailto:regehr@cs.utah.edu" target="_blank">regehr@cs.utah.edu</a>><br>
> Subject: [EXT] LangRef semantics for shufflevector with undef mask is <br>
> incorrect<br>
><br>
> Hi,<br>
><br>
> This is a follow up on a discussion around shufflevector with undef <br>
> mask in<br>
> <a href="https://reviews.llvm.org/D70641" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70641</a> and<br>
> <a href="https://bugs.llvm.org/show_bug.cgi?id=43958" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=43958</a>.<br>
><br>
> The current semantics of shufflevector in <br>
> <a href="http://llvm.org/docs/LangRef.html#shufflevector-instruction" rel="noreferrer" target="_blank">http://llvm.org/docs/LangRef.html#shufflevector-instruction</a> states:<br>
> "If the shuffle mask is undef, the result vector is undef. If any <br>
> element of the mask operand is undef, that element of the result is<br>
undef."<br>
><br>
> We found this semantics to be problematic. TL;DR: instructions can't <br>
> detect if an operand is undef.<br>
> Long story:<br>
> Undef can be replaced with any value at any time. It's valid to <br>
> replace undef with 0 or 1 or anything else.<br>
><br>
> A possible sequence of optimizations with sufflevector could be as<br>
follows:<br>
> %v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 <br>
> undef,<br>
> i32 0><br>
> -><br>
> %v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 <br>
> 2, i32<br>
> 0><br>
> -><br>
> %v = <undef, %x[0]><br>
><br>
> So this respects the semantics in LangRef: the mask is undef, so the <br>
> resulting element is undef.<br>
><br>
> However, there's an alternative sequence of optimizations:<br>
> %v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 <br>
> undef,<br>
> i32 0><br>
> -><br>
> %v = shufflevector <2 x float> %x, <2 x float> undef, <2 x i32> <i32 <br>
> 1, i32<br>
> 0><br>
> -><br>
> %v = <%x[1], %x[0]><br>
><br>
> So now it depends on what the value of %x[1] is. If it's poison, we<br>
obtain:<br>
> %v = <poison, %x[0]><br>
><br>
> This result contradicts the semantics in LangRef, even though no <br>
> individual transformation we did above is wrong.<br>
> In summary, an instruction cannot detect undef and have special <br>
> semantics for it.<br>
><br>
> AFAICT, the only way to fix the semantics of shufflevector is to say <br>
> that if the mask is out-of-bounds, we yield poison. That's correct <br>
> because there's nothing worse than poison.<br>
> Since we can replace undef with an OOB index, an undef mask can safely <br>
> yield poison. Or it would yield one of the input elements, which is <br>
> poison in the worst case. So we get poison in both cases.<br>
><br>
> I guess the issue to make this semantics a reality is that we would <br>
> need to introduce a poison value (which is a good thing IMHO). <br>
> Otherwise we can't continue doing some of the folds we have today <br>
> since we don't have a poison constant to replace undef when folding.<br>
><br>
> Any comments/suggestions appreciated!<br>
><br>
> Thanks,<br>
> Nuno<br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>