<div dir="ltr">I'm not sure this is a step in the right direction, and I don't quite understand what you meant in the commit log. I'm not saying it's not, but could you perhaps explain it a bit more clearly?<div><br><div><div>(To be honest, I'm not sure why we even use an explicit -1 instead of something named - enum, const, whatever). But making it even less explicit doesn't sound quite right to me, unless there's a tangible benefit.)<br></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 25 June 2016 at 12:05, Craig Topper via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ctopper<br>
Date: Sat Jun 25 14:05:29 2016<br>
New Revision: 273788<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=273788&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=273788&view=rev</a><br>
Log:<br>
[X86] Convert ==/!= comparisons with -1 for checking undef in shuffle lowering to comparisons of <0 or >=0. While there do the same for other kinds of index checks that can just check for greater than 0. No functional change intended.<br>
<br>
Modified:<br>
llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
<br>
Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=273788&r1=273787&r2=273788&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=273788&r1=273787&r2=273788&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat Jun 25 14:05:29 2016<br>
@@ -6039,7 +6039,7 @@ X86TargetLowering::LowerBUILD_VECTORvXi1<br>
Immediate |= cast<ConstantSDNode>(In)->getZExtValue() << idx;<br>
HasConstElts = true;<br>
}<br>
- if (SplatIdx == -1)<br>
+ if (SplatIdx < 0)<br>
SplatIdx = idx;<br>
else if (In != Op.getOperand(SplatIdx))<br>
IsSplat = false;<br>
@@ -7018,9 +7018,11 @@ static SDValue LowerCONCAT_VECTORS(SDVal<br>
/// ShuffleVectorSDNode mask) requires any shuffles to occur. Both undef and an<br>
/// in-place shuffle are 'no-op's.<br>
static bool isNoopShuffleMask(ArrayRef<int> Mask) {<br>
- for (int i = 0, Size = Mask.size(); i < Size; ++i)<br>
- if (Mask[i] != -1 && Mask[i] != i)<br>
+ for (int i = 0, Size = Mask.size(); i < Size; ++i) {<br>
+ assert(Mask[i] >= -1 && "Out of bound mask element!");<br>
+ if (Mask[i] >= 0 && Mask[i] != i)<br>
return false;<br>
+ }<br>
return true;<br>
}<br>
<br>
@@ -7113,8 +7115,9 @@ static bool isShuffleEquivalent(SDValue<br>
auto *BV1 = dyn_cast<BuildVectorSDNode>(V1);<br>
auto *BV2 = dyn_cast<BuildVectorSDNode>(V2);<br>
<br>
- for (int i = 0; i < Size; ++i)<br>
- if (Mask[i] != -1 && Mask[i] != ExpectedMask[i]) {<br>
+ for (int i = 0; i < Size; ++i) {<br>
+ assert(Mask[i] >= -1 && "Out of bound mask element!");<br>
+ if (Mask[i] >= 0 && Mask[i] != ExpectedMask[i]) {<br>
auto *MaskBV = Mask[i] < Size ? BV1 : BV2;<br>
auto *ExpectedBV = ExpectedMask[i] < Size ? BV1 : BV2;<br>
if (!MaskBV || !ExpectedBV ||<br>
@@ -7122,6 +7125,7 @@ static bool isShuffleEquivalent(SDValue<br>
ExpectedBV->getOperand(ExpectedMask[i] % Size))<br>
return false;<br>
}<br>
+}<br>
<br>
return true;<br>
}<br>
@@ -7168,10 +7172,10 @@ static SDValue getV4X86ShuffleImm8ForMas<br>
assert(Mask[3] >= -1 && Mask[3] < 4 && "Out of bound mask element!");<br>
<br>
unsigned Imm = 0;<br>
- Imm |= (Mask[0] == -1 ? 0 : Mask[0]) << 0;<br>
- Imm |= (Mask[1] == -1 ? 1 : Mask[1]) << 2;<br>
- Imm |= (Mask[2] == -1 ? 2 : Mask[2]) << 4;<br>
- Imm |= (Mask[3] == -1 ? 3 : Mask[3]) << 6;<br>
+ Imm |= (Mask[0] < 0 ? 0 : Mask[0]) << 0;<br>
+ Imm |= (Mask[1] < 0 ? 1 : Mask[1]) << 2;<br>
+ Imm |= (Mask[2] < 0 ? 2 : Mask[2]) << 4;<br>
+ Imm |= (Mask[3] < 0 ? 3 : Mask[3]) << 6;<br>
return DAG.getConstant(Imm, DL, MVT::i8);<br>
}<br>
<br>
@@ -7396,7 +7400,7 @@ static SDValue lowerVectorShuffleAsBitBl<br>
EltVT);<br>
SmallVector<SDValue, 16> MaskOps;<br>
for (int i = 0, Size = Mask.size(); i < Size; ++i) {<br>
- if (Mask[i] != -1 && Mask[i] != i && Mask[i] != i + Size)<br>
+ if (Mask[i] >= 0 && Mask[i] != i && Mask[i] != i + Size)<br>
return SDValue(); // Shuffled input!<br>
MaskOps.push_back(Mask[i] < Size ? AllOnes : Zero);<br>
}<br>
@@ -7594,7 +7598,7 @@ static SDValue lowerVectorShuffleAsBlend<br>
<br>
assert(Mask[i] < Size * 2 && "Shuffle input is out of bounds.");<br>
<br>
- if (BlendMask[Mask[i] % Size] == -1)<br>
+ if (BlendMask[Mask[i] % Size] < 0)<br>
BlendMask[Mask[i] % Size] = Mask[i];<br>
else if (BlendMask[Mask[i] % Size] != Mask[i])<br>
return SDValue(); // Can't blend in the needed input!<br>
@@ -7685,9 +7689,8 @@ static SDValue lowerVectorShuffleAsByteR<br>
SDValue Lo, Hi;<br>
for (int l = 0; l < NumElts; l += NumLaneElts) {<br>
for (int i = 0; i < NumLaneElts; ++i) {<br>
- if (Mask[l + i] == -1)<br>
+ if (Mask[l + i] < 0)<br>
continue;<br>
- assert(Mask[l + i] >= 0 && "Only -1 is a valid negative mask element!");<br>
<br>
// Get the mod-Size index and lane correct it.<br>
int LaneIdx = (Mask[l + i] % NumElts) - l;<br>
@@ -8195,7 +8198,7 @@ static SDValue lowerVectorShuffleAsZeroO<br>
int Matches = 0;<br>
for (int i = 0; i < NumElements; ++i) {<br>
int M = Mask[i];<br>
- if (M == -1)<br>
+ if (M < 0)<br>
continue; // Valid anywhere but doesn't tell us anything.<br>
if (i % Scale != 0) {<br>
// Each of the extended elements need to be zeroable.<br>
@@ -8656,7 +8659,7 @@ static SDValue lowerVectorShuffleAsInser<br>
}<br>
<br>
// We can only insert a single non-zeroable element.<br>
- if (V1DstIndex != -1 || V2DstIndex != -1)<br>
+ if (V1DstIndex >= 0 || V2DstIndex >= 0)<br>
return SDValue();<br>
<br>
if (Mask[i] < 4) {<br>
@@ -8669,13 +8672,13 @@ static SDValue lowerVectorShuffleAsInser<br>
}<br>
<br>
// Don't bother if we have no (non-zeroable) element for insertion.<br>
- if (V1DstIndex == -1 && V2DstIndex == -1)<br>
+ if (V1DstIndex >= 0 && V2DstIndex >= 0)<br>
return SDValue();<br>
<br>
// Determine element insertion src/dst indices. The src index is from the<br>
// start of the inserted vector, not the start of the concatenated vector.<br>
unsigned V2SrcIndex = 0;<br>
- if (V1DstIndex != -1) {<br>
+ if (V1DstIndex >= 0) {<br>
// If we have a V1 input out of place, we use V1 as the V2 element insertion<br>
// and don't use the original V2 at all.<br>
V2SrcIndex = Mask[V1DstIndex];<br>
@@ -9007,12 +9010,16 @@ static SDValue lowerV2I64VectorShuffle(c<br>
static bool isSingleSHUFPSMask(ArrayRef<int> Mask) {<br>
// This routine only handles 128-bit shufps.<br>
assert(Mask.size() == 4 && "Unsupported mask size!");<br>
+ assert(Mask[0] >= -1 && Mask[0] < 8 && "Out of bound mask element!");<br>
+ assert(Mask[1] >= -1 && Mask[1] < 8 && "Out of bound mask element!");<br>
+ assert(Mask[2] >= -1 && Mask[2] < 8 && "Out of bound mask element!");<br>
+ assert(Mask[3] >= -1 && Mask[3] < 8 && "Out of bound mask element!");<br>
<br>
// To lower with a single SHUFPS we need to have the low half and high half<br>
// each requiring a single input.<br>
- if (Mask[0] != -1 && Mask[1] != -1 && (Mask[0] < 4) != (Mask[1] < 4))<br>
+ if (Mask[0] >= 0 && Mask[1] >= 0 && (Mask[0] < 4) != (Mask[1] < 4))<br>
return false;<br>
- if (Mask[2] != -1 && Mask[3] != -1 && (Mask[2] < 4) != (Mask[3] < 4))<br>
+ if (Mask[2] >= 0 && Mask[3] >= 0 && (Mask[2] < 4) != (Mask[3] < 4))<br>
return false;<br>
<br>
return true;<br>
@@ -9040,7 +9047,7 @@ static SDValue lowerVectorShuffleWithSHU<br>
// the low bit.<br>
int V2AdjIndex = V2Index ^ 1;<br>
<br>
- if (Mask[V2AdjIndex] == -1) {<br>
+ if (Mask[V2AdjIndex] < 0) {<br>
// Handles all the cases where we have a single V2 element and an undef.<br>
// This will only ever happen in the high lanes because we commute the<br>
// vector otherwise.<br>
@@ -9465,9 +9472,9 @@ static SDValue lowerV8I16GeneralSingleIn<br>
getV4X86ShuffleImm8ForMask(PSHUFHalfMask, DL, DAG));<br>
<br>
for (int &M : Mask)<br>
- if (M != -1 && M == FixIdx)<br>
+ if (M >= 0 && M == FixIdx)<br>
M = FixFreeIdx;<br>
- else if (M != -1 && M == FixFreeIdx)<br>
+ else if (M >= 0 && M == FixFreeIdx)<br>
M = FixIdx;<br>
};<br>
if (NumFlippedBToBInputs != 0) {<br>
@@ -9492,9 +9499,9 @@ static SDValue lowerV8I16GeneralSingleIn<br>
<br>
// Adjust the mask to match the new locations of A and B.<br>
for (int &M : Mask)<br>
- if (M != -1 && M/2 == ADWord)<br>
+ if (M >= 0 && M/2 == ADWord)<br>
M = 2 * BDWord + M % 2;<br>
- else if (M != -1 && M/2 == BDWord)<br>
+ else if (M >= 0 && M/2 == BDWord)<br>
M = 2 * ADWord + M % 2;<br>
<br>
// Recurse back into this routine to re-compute state now that this isn't<br>
@@ -9563,7 +9570,7 @@ static SDValue lowerV8I16GeneralSingleIn<br>
MutableArrayRef<int> FinalSourceHalfMask, int SourceOffset,<br>
int DestOffset) {<br>
auto isWordClobbered = [](ArrayRef<int> SourceHalfMask, int Word) {<br>
- return SourceHalfMask[Word] != -1 && SourceHalfMask[Word] != Word;<br>
+ return SourceHalfMask[Word] >= 0 && SourceHalfMask[Word] != Word;<br>
};<br>
auto isDWordClobbered = [&isWordClobbered](ArrayRef<int> SourceHalfMask,<br>
int Word) {<br>
@@ -9582,7 +9589,7 @@ static SDValue lowerV8I16GeneralSingleIn<br>
// If the source half mask maps over the inputs, turn those into<br>
// swaps and use the swapped lane.<br>
if (isWordClobbered(SourceHalfMask, Input - SourceOffset)) {<br>
- if (SourceHalfMask[SourceHalfMask[Input - SourceOffset]] == -1) {<br>
+ if (SourceHalfMask[SourceHalfMask[Input - SourceOffset]] < 0) {<br>
SourceHalfMask[SourceHalfMask[Input - SourceOffset]] =<br>
Input - SourceOffset;<br>
// We have to swap the uses in our half mask in one sweep.<br>
@@ -9603,7 +9610,7 @@ static SDValue lowerV8I16GeneralSingleIn<br>
}<br>
<br>
// Map the input's dword into the correct half.<br>
- if (PSHUFDMask[(Input - SourceOffset + DestOffset) / 2] == -1)<br>
+ if (PSHUFDMask[(Input - SourceOffset + DestOffset) / 2] < 0)<br>
PSHUFDMask[(Input - SourceOffset + DestOffset) / 2] = Input / 2;<br>
else<br>
assert(PSHUFDMask[(Input - SourceOffset + DestOffset) / 2] ==<br>
@@ -9649,17 +9656,17 @@ static SDValue lowerV8I16GeneralSingleIn<br>
// the inputs, place the other input in it. We use (Index XOR 1) to<br>
// compute an adjacent index.<br>
if (!isWordClobbered(SourceHalfMask, InputsFixed[0]) &&<br>
- SourceHalfMask[InputsFixed[0] ^ 1] == -1) {<br>
+ SourceHalfMask[InputsFixed[0] ^ 1] < 0) {<br>
SourceHalfMask[InputsFixed[0]] = InputsFixed[0];<br>
SourceHalfMask[InputsFixed[0] ^ 1] = InputsFixed[1];<br>
InputsFixed[1] = InputsFixed[0] ^ 1;<br>
} else if (!isWordClobbered(SourceHalfMask, InputsFixed[1]) &&<br>
- SourceHalfMask[InputsFixed[1] ^ 1] == -1) {<br>
+ SourceHalfMask[InputsFixed[1] ^ 1] < 0) {<br>
SourceHalfMask[InputsFixed[1]] = InputsFixed[1];<br>
SourceHalfMask[InputsFixed[1] ^ 1] = InputsFixed[0];<br>
InputsFixed[0] = InputsFixed[1] ^ 1;<br>
- } else if (SourceHalfMask[2 * ((InputsFixed[0] / 2) ^ 1)] == -1 &&<br>
- SourceHalfMask[2 * ((InputsFixed[0] / 2) ^ 1) + 1] == -1) {<br>
+ } else if (SourceHalfMask[2 * ((InputsFixed[0] / 2) ^ 1)] < 0 &&<br>
+ SourceHalfMask[2 * ((InputsFixed[0] / 2) ^ 1) + 1] < 0) {<br>
// The two inputs are in the same DWord but it is clobbered and the<br>
// adjacent DWord isn't used at all. Move both inputs to the free<br>
// slot.<br>
@@ -9673,7 +9680,7 @@ static SDValue lowerV8I16GeneralSingleIn<br>
// free slot adjacent to one of the inputs. In this case, we have to<br>
// swap an input with a non-input.<br>
for (int i = 0; i < 4; ++i)<br>
- assert((SourceHalfMask[i] == -1 || SourceHalfMask[i] == i) &&<br>
+ assert((SourceHalfMask[i] < 0 || SourceHalfMask[i] == i) &&<br>
"We can't handle any clobbers here!");<br>
assert(InputsFixed[1] != (InputsFixed[0] ^ 1) &&<br>
"Cannot have adjacent inputs here!");<br>
@@ -9707,8 +9714,8 @@ static SDValue lowerV8I16GeneralSingleIn<br>
}<br>
<br>
// Now hoist the DWord down to the right half.<br>
- int FreeDWord = (PSHUFDMask[DestOffset / 2] == -1 ? 0 : 1) + DestOffset / 2;<br>
- assert(PSHUFDMask[FreeDWord] == -1 && "DWord not free");<br>
+ int FreeDWord = (PSHUFDMask[DestOffset / 2] < 0 ? 0 : 1) + DestOffset / 2;<br>
+ assert(PSHUFDMask[FreeDWord] < 0 && "DWord not free");<br>
PSHUFDMask[FreeDWord] = IncomingInputs[0] / 2;<br>
for (int &M : HalfMask)<br>
for (int Input : IncomingInputs)<br>
@@ -9771,7 +9778,7 @@ static SDValue lowerVectorShuffleAsBlend<br>
int Size = Mask.size();<br>
int Scale = 16 / Size;<br>
for (int i = 0; i < 16; ++i) {<br>
- if (Mask[i / Scale] == -1) {<br>
+ if (Mask[i / Scale] < 0) {<br>
V1Mask[i] = V2Mask[i] = DAG.getUNDEF(MVT::i8);<br>
} else {<br>
const int ZeroMask = 0x80;<br>
@@ -9968,7 +9975,7 @@ static int canLowerByDroppingEvenElement<br>
for (int i = 0, e = Mask.size(); i < e; ++i) {<br>
// Ignore undef lanes, we'll optimistically collapse them to the pattern we<br>
// want.<br>
- if (Mask[i] == -1)<br>
+ if (Mask[i] < 0)<br>
continue;<br>
<br>
bool IsAnyViable = false;<br>
@@ -10049,7 +10056,7 @@ static SDValue lowerV16I8VectorShuffle(c<br>
// i16 shuffle as well.<br>
auto canWidenViaDuplication = [](ArrayRef<int> Mask) {<br>
for (int i = 0; i < 16; i += 2)<br>
- if (Mask[i] != -1 && Mask[i + 1] != -1 && Mask[i] != Mask[i + 1])<br>
+ if (Mask[i] >= 0 && Mask[i + 1] >= 0 && Mask[i] != Mask[i + 1])<br>
return false;<br>
<br>
return true;<br>
@@ -10087,7 +10094,7 @@ static SDValue lowerV16I8VectorShuffle(c<br>
if (PreDupI16Shuffle[j] != MovingInputs[i] / 2) {<br>
// If we haven't yet mapped the input, search for a slot into which<br>
// we can map it.<br>
- while (j < je && PreDupI16Shuffle[j] != -1)<br>
+ while (j < je && PreDupI16Shuffle[j] >= 0)<br>
++j;<br>
<br>
if (j == je)<br>
@@ -10112,10 +10119,10 @@ static SDValue lowerV16I8VectorShuffle(c<br>
<br>
int PostDupI16Shuffle[8] = {-1, -1, -1, -1, -1, -1, -1, -1};<br>
for (int i = 0; i < 16; ++i)<br>
- if (Mask[i] != -1) {<br>
+ if (Mask[i] >= 0) {<br>
int MappedMask = LaneMap[Mask[i]] - (TargetLo ? 0 : 8);<br>
assert(MappedMask < 8 && "Invalid v8 shuffle mask!");<br>
- if (PostDupI16Shuffle[i / 2] == -1)<br>
+ if (PostDupI16Shuffle[i / 2] < 0)<br>
PostDupI16Shuffle[i / 2] = MappedMask;<br>
else<br>
assert(PostDupI16Shuffle[i / 2] == MappedMask &&<br>
@@ -10517,12 +10524,12 @@ static SDValue lowerVectorShuffleAsSplit<br>
int V1BroadcastIdx = -1, V2BroadcastIdx = -1;<br>
for (int M : Mask)<br>
if (M >= Size) {<br>
- if (V2BroadcastIdx == -1)<br>
+ if (V2BroadcastIdx < 0)<br>
V2BroadcastIdx = M - Size;<br>
else if (M - Size != V2BroadcastIdx)<br>
return false;<br>
} else if (M >= 0) {<br>
- if (V1BroadcastIdx == -1)<br>
+ if (V1BroadcastIdx < 0)<br>
V1BroadcastIdx = M;<br>
else if (M != V1BroadcastIdx)<br>
return false;<br>
@@ -10828,12 +10835,12 @@ static SDValue lowerVectorShuffleWithUnd<br>
<br>
// We can shuffle with up to 2 half vectors, set the new 'half'<br>
// shuffle mask accordingly.<br>
- if (-1 == HalfIdx1 || HalfIdx1 == HalfIdx) {<br>
+ if (HalfIdx1 < 0 || HalfIdx1 == HalfIdx) {<br>
HalfMask[i] = HalfElt;<br>
HalfIdx1 = HalfIdx;<br>
continue;<br>
}<br>
- if (-1 == HalfIdx2 || HalfIdx2 == HalfIdx) {<br>
+ if (HalfIdx2 < 0 || HalfIdx2 == HalfIdx) {<br>
HalfMask[i] = HalfElt + HalfNumElts;<br>
HalfIdx2 = HalfIdx;<br>
continue;<br>
@@ -26615,7 +26622,7 @@ static SDValue checkBoolTestSetCCCombine<br>
OpIdx = 1;<br>
if (isOneConstant(SetCC.getOperand(1)))<br>
OpIdx = 0;<br>
- if (OpIdx == -1)<br>
+ if (OpIdx < 0)<br>
break;<br>
SetCC = SetCC.getOperand(OpIdx);<br>
truncatedToBoolWithAnd = true;<br>
@@ -28870,7 +28877,7 @@ static SDValue combineStore(SDNode *N, S<br>
Ld->isNonTemporal(), Ld->isInvariant(),<br>
Ld->getAlignment());<br>
SDValue NewChain = NewLd.getValue(1);<br>
- if (TokenFactorIndex != -1) {<br>
+ if (TokenFactorIndex >= 0) {<br>
Ops.push_back(NewChain);<br>
NewChain = DAG.getNode(ISD::TokenFactor, LdDL, MVT::Other, Ops);<br>
}<br>
@@ -28895,7 +28902,7 @@ static SDValue combineStore(SDNode *N, S<br>
MinAlign(Ld->getAlignment(), 4));<br>
<br>
SDValue NewChain = LoLd.getValue(1);<br>
- if (TokenFactorIndex != -1) {<br>
+ if (TokenFactorIndex >= 0) {<br>
Ops.push_back(LoLd);<br>
Ops.push_back(HiLd);<br>
NewChain = DAG.getNode(ISD::TokenFactor, LdDL, MVT::Other, Ops);<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>