[llvm-commits] [llvm] r147308 - in /llvm/trunk: lib/Target/X86/X86ISelLowering.cpp test/CodeGen/X86/avx-shuffle.ll
Chad Rosier
mcrosier at apple.com
Tue Jan 3 17:35:30 PST 2012
On Jan 3, 2012, at 5:30 PM, Chad Rosier wrote:
> Hi Elena,
> Just a few comments below.
>
> On Dec 28, 2011, at 12:14 AM, Elena Demikhovsky wrote:
>
>> Author: delena
>> Date: Wed Dec 28 02:14:01 2011
>> New Revision: 147308
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=147308&view=rev
>> Log:
>> Fixed a bug in LowerVECTOR_SHUFFLE and LowerBUILD_VECTOR.
>> Matching MOVLP mask for AVX (265-bit vectors) was wrong.
>> The failure was detected by conformance tests.
>
> As far as I can tell this patch is addressing two bugs, one in LowerBUILD_VECTOR and another with LowerVECTOR_SHUFFLE/MOVLP. These should really be fixed in two separate commits.
>
>>
>> Modified:
>> llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> llvm/trunk/test/CodeGen/X86/avx-shuffle.ll
>>
>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=147308&r1=147307&r2=147308&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Wed Dec 28 02:14:01 2011
>> @@ -3448,6 +3448,11 @@
>> /// isMOVLPMask - Return true if the specified VECTOR_SHUFFLE operand
>> /// specifies a shuffle of elements that is suitable for input to MOVLP{S|D}.
>> bool X86::isMOVLPMask(ShuffleVectorSDNode *N) {
>> + EVT VT = N->getValueType(0);
>> +
>> + if (VT.getSizeInBits() != 128)
>> + return false;
>> +
>
> I'm not sure I agree with the approach taken here...
>
>> unsigned NumElems = N->getValueType(0).getVectorNumElements();
>>
>> if (NumElems != 2 && NumElems != 4)
>> @@ -3666,6 +3671,8 @@
>> static bool isMOVLMask(const SmallVectorImpl<int> &Mask, EVT VT) {
>> if (VT.getVectorElementType().getSizeInBits() < 32)
>> return false;
>> + if (VT.getSizeInBits() == 256)
>> + return false;
>
> ..and here. Aren't these functions behaving correctly? I believe the real problem is the code that is predicated upon these functions; they don't fully support codegen when AVX is enabled.
>
> And one moot point.. The conditional statements that checks the size of the vectors could be more consistent (i.e., both (VT.getSizeInBits != 128) or (VT.getSizeInBits != 128), but not a combination of the two).
(i.e., both (VT.getSizeInBits != 128) or (VT.getSizeInBits == 256), ...)
>> int NumElts = VT.getVectorNumElements();
>>
>> @@ -5158,16 +5165,30 @@
>> return DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, VT, Item);
>> } else if (ExtVT == MVT::i32 || ExtVT == MVT::f32 || ExtVT == MVT::f64 ||
>> (ExtVT == MVT::i64 && Subtarget->is64Bit())) {
>> + if (VT.getSizeInBits() == 256) {
>> +
>> + EVT VT128 = EVT::getVectorVT(*DAG.getContext(), ExtVT, NumElems / 2);
>> + Item = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, VT128, Item);
>> + SDValue ZeroVec = getZeroVector(VT, true, DAG, dl);
>> + return Insert128BitVector(ZeroVec, Item, DAG.getConstant(0, MVT::i32),
>> + DAG, dl);
>> + }
>> Item = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, VT, Item);
>> // Turn it into a MOVL (i.e. movss, movsd, or movd) to a zero vector.
>> return getShuffleVectorZeroOrUndef(Item, 0, true,Subtarget->hasXMMInt(),
>> DAG);
>> } else if (ExtVT == MVT::i16 || ExtVT == MVT::i8) {
>> Item = DAG.getNode(ISD::ZERO_EXTEND, dl, MVT::i32, Item);
>> - unsigned NumBits = VT.getSizeInBits();
>> - assert((NumBits == 128 || NumBits == 256) &&
>> - "Expected an SSE or AVX value type!");
>> - EVT MiddleVT = NumBits == 128 ? MVT::v4i32 : MVT::v8i32;
>> + if (VT.getSizeInBits() == 256) {
>> +
>> + EVT VT128 = EVT::getVectorVT(*DAG.getContext(), ExtVT, NumElems / 2);
>> + Item = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, VT128, Item);
>> + SDValue ZeroVec = getZeroVector(VT, true, DAG, dl);
>> + return Insert128BitVector(ZeroVec, Item, DAG.getConstant(0, MVT::i32),
>> + DAG, dl);
>> + }
>> + assert (VT.getSizeInBits() == 128 || "Expected an SSE value type!");
>> + EVT MiddleVT = MVT::v4i32;
>> Item = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, MiddleVT, Item);
>> Item = getShuffleVectorZeroOrUndef(Item, 0, true,
>> Subtarget->hasXMMInt(), DAG);
>>
>> Modified: llvm/trunk/test/CodeGen/X86/avx-shuffle.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx-shuffle.ll?rev=147308&r1=147307&r2=147308&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/avx-shuffle.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/avx-shuffle.ll Wed Dec 28 02:14:01 2011
>> @@ -13,8 +13,22 @@
>> define <3 x i64> @test2(<2 x i64> %v) nounwind readnone {
>> ; CHECK: test2:
>> ; CHECK: vxorpd
>> -; CHECK: vmovsd
>> +; CHECK: vperm2f128
>> %1 = shufflevector <2 x i64> %v, <2 x i64> %v, <3 x i32> <i32 0, i32 1, i32 undef>
>> %2 = shufflevector <3 x i64> zeroinitializer, <3 x i64> %1, <3 x i32> <i32 3, i32 4, i32 2>
>> ret <3 x i64> %2
>> }
>> +
>> +define <4 x i64> @test3(<4 x i64> %a, <4 x i64> %b) nounwind {
>> + %c = shufflevector <4 x i64> %a, <4 x i64> %b, <4 x i32> <i32 4, i32 5, i32 2, i32 undef>
>> + ret <4 x i64> %c
>> +; CHECK: test3:
>> +; CHECK: vperm2f128
>> +}
>> +
>> +define <8 x float> @test4(float %a) nounwind {
>> + %b = insertelement <8 x float> zeroinitializer, float %a, i32 0
>> + ret <8 x float> %b
>> +; CHECK: test4:
>> +; CHECK: vinsertf128
>> +}
>> \ No newline at end of file
>>
>
> Please make sure your test case ends with a newline. This was fixed in r147481.
>
> Chad
>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list