From ea8c66fea53e5039a75c4fc56041b51b79a044ac Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Mon, 25 Jul 2011 21:36:45 +0000 Subject: [PATCH] Get rid of an incorrect optimization for shuffles with PALIGNR and simplify isPALIGNRMask. Addresses PR10466, although the crash from that PR only triggers in cases where DAGCombine misses optimizing a shuffle. llvm-svn: 135980 --- llvm/lib/Target/X86/X86ISelLowering.cpp | 20 +++++------------ llvm/test/CodeGen/X86/palignr.ll | 29 +++++++++++++++++++------ 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp index c3fa9d1fa2de..d2c8778debd1 100644 --- a/llvm/lib/Target/X86/X86ISelLowering.cpp +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp @@ -3075,27 +3075,16 @@ static bool isPALIGNRMask(const SmallVectorImpl &Mask, EVT VT, if (i == e) return false; - // Determine if it's ok to perform a palignr with only the LHS, since we - // don't have access to the actual shuffle elements to see if RHS is undef. - bool Unary = Mask[i] < (int)e; - bool NeedsUnary = false; + // Make sure we're shifting in the right direction. + if (Mask[i] <= i) + return false; int s = Mask[i] - i; // Check the rest of the elements to see if they are consecutive. for (++i; i != e; ++i) { int m = Mask[i]; - if (m < 0) - continue; - - Unary = Unary && (m < (int)e); - NeedsUnary = NeedsUnary || (m < s); - - if (NeedsUnary && !Unary) - return false; - if (Unary && m != ((s+i) & (e-1))) - return false; - if (!Unary && m != (s+i)) + if (m >= 0 && m != s+i) return false; } return true; @@ -3631,6 +3620,7 @@ unsigned X86::getShufflePALIGNRImmediate(SDNode *N) { if (Val >= 0) break; } + assert(Val - i > 0 && "PALIGNR imm should be positive"); return (Val - i) * EltSize; } diff --git a/llvm/test/CodeGen/X86/palignr.ll b/llvm/test/CodeGen/X86/palignr.ll index 3812c7238c43..ddcb05c97032 100644 --- a/llvm/test/CodeGen/X86/palignr.ll +++ b/llvm/test/CodeGen/X86/palignr.ll @@ -2,6 +2,7 @@ ; RUN: llc < %s -march=x86 -mcpu=yonah | FileCheck --check-prefix=YONAH %s define <4 x i32> @test1(<4 x i32> %A, <4 x i32> %B) nounwind { +; CHECK: test1: ; CHECK: pshufd ; CHECK-YONAH: pshufd %C = shufflevector <4 x i32> %A, <4 x i32> undef, <4 x i32> < i32 1, i32 2, i32 3, i32 0 > @@ -9,6 +10,7 @@ define <4 x i32> @test1(<4 x i32> %A, <4 x i32> %B) nounwind { } define <4 x i32> @test2(<4 x i32> %A, <4 x i32> %B) nounwind { +; CHECK: test2: ; CHECK: palignr ; CHECK-YONAH: shufps %C = shufflevector <4 x i32> %A, <4 x i32> %B, <4 x i32> < i32 1, i32 2, i32 3, i32 4 > @@ -16,43 +18,56 @@ define <4 x i32> @test2(<4 x i32> %A, <4 x i32> %B) nounwind { } define <4 x i32> @test3(<4 x i32> %A, <4 x i32> %B) nounwind { +; CHECK: test3: ; CHECK: palignr %C = shufflevector <4 x i32> %A, <4 x i32> %B, <4 x i32> < i32 1, i32 2, i32 undef, i32 4 > ret <4 x i32> %C } define <4 x i32> @test4(<4 x i32> %A, <4 x i32> %B) nounwind { +; CHECK: test4: ; CHECK: palignr %C = shufflevector <4 x i32> %A, <4 x i32> %B, <4 x i32> < i32 6, i32 7, i32 undef, i32 1 > ret <4 x i32> %C } define <4 x float> @test5(<4 x float> %A, <4 x float> %B) nounwind { +; CHECK: test5: ; CHECK: palignr %C = shufflevector <4 x float> %A, <4 x float> %B, <4 x i32> < i32 6, i32 7, i32 undef, i32 1 > ret <4 x float> %C } define <8 x i16> @test6(<8 x i16> %A, <8 x i16> %B) nounwind { +; CHECK: test6: ; CHECK: palignr %C = shufflevector <8 x i16> %A, <8 x i16> %B, <8 x i32> < i32 3, i32 4, i32 undef, i32 6, i32 7, i32 8, i32 9, i32 10 > ret <8 x i16> %C } define <8 x i16> @test7(<8 x i16> %A, <8 x i16> %B) nounwind { +; CHECK: test7: ; CHECK: palignr %C = shufflevector <8 x i16> %A, <8 x i16> %B, <8 x i32> < i32 undef, i32 6, i32 undef, i32 8, i32 9, i32 10, i32 11, i32 12 > ret <8 x i16> %C } -define <8 x i16> @test8(<8 x i16> %A, <8 x i16> %B) nounwind { -; CHECK: palignr - %C = shufflevector <8 x i16> %A, <8 x i16> %B, <8 x i32> < i32 undef, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 0 > - ret <8 x i16> %C -} - -define <16 x i8> @test9(<16 x i8> %A, <16 x i8> %B) nounwind { +define <16 x i8> @test8(<16 x i8> %A, <16 x i8> %B) nounwind { +; CHECK: test8: ; CHECK: palignr %C = shufflevector <16 x i8> %A, <16 x i8> %B, <16 x i32> < i32 5, i32 6, i32 7, i32 undef, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20 > ret <16 x i8> %C } + +; Check that we don't do unary (circular on single operand) palignr incorrectly. +; (It is possible, but before this testcase was committed, it was being done +; incorrectly. In particular, one of the operands of the palignr node +; was an UNDEF.) +define <8 x i16> @test9(<8 x i16> %A, <8 x i16> %B) nounwind { +; CHECK: test9: +; CHECK-NOT: palignr +; CHECK: pshufb + %C = shufflevector <8 x i16> %B, <8 x i16> %A, <8 x i32> < i32 undef, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 0 > + ret <8 x i16> %C +} +