Skip to content

Commit be4016e

Browse files
goldsteinntru
authored andcommitted
[X86] Fix logic for optimizing movmsk(bitcast(shuffle(x))); PR67287
Prior logic would remove the shuffle iff all of the elements in `x` where used. This is incorrect. The issue is `movmsk` only cares about the highbits, so if the width of the elements in `x` is smaller than the width of the elements for the `movmsk`, then the shuffle, even if it preserves all the elements, may change which ones are used by the highbits. For example: `movmsk64(bitcast(shuffle32(x, (1,0,3,2))))` Even though the shuffle mask `(1,0,3,2)` preserves all the elements, it flips which will be relevant to the `movmsk64` (x[1] and x[3] before and x[0] and x[2] after). The fix here, is to ensure that the shuffle mask can be scaled to the element width of the `movmsk` instruction. This ensure that the "high" elements stay "high". This is overly conservative as it misses cases like `(1,1,3,3)` where the "high" elements stay intact despite not be scalable, but for an relatively edge-case optimization that should generally be handled during simplifyDemandedBits, it seems okay. (cherry picked from commit 1684c65)
1 parent 496b174 commit be4016e

File tree

2 files changed

+22
-6
lines changed

2 files changed

+22
-6
lines changed

llvm/lib/Target/X86/X86ISelLowering.cpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48539,13 +48539,28 @@ static SDValue combineSetCCMOVMSK(SDValue EFLAGS, X86::CondCode &CC,
4853948539
}
4854048540

4854148541
// MOVMSK(SHUFFLE(X,u)) -> MOVMSK(X) iff every element is referenced.
48542-
SmallVector<int, 32> ShuffleMask;
48542+
// Since we peek through a bitcast, we need to be careful if the base vector
48543+
// type has smaller elements than the MOVMSK type. In that case, even if
48544+
// all the elements are demanded by the shuffle mask, only the "high"
48545+
// elements which have highbits that align with highbits in the MOVMSK vec
48546+
// elements are actually demanded. A simplification of spurious operations
48547+
// on the "low" elements take place during other simplifications.
48548+
//
48549+
// For example:
48550+
// MOVMSK64(BITCAST(SHUF32 X, (1,0,3,2))) even though all the elements are
48551+
// demanded, because we are swapping around the result can change.
48552+
//
48553+
// To address this, we check that we can scale the shuffle mask to MOVMSK
48554+
// element width (this will ensure "high" elements match). Its slightly overly
48555+
// conservative, but fine for an edge case fold.
48556+
SmallVector<int, 32> ShuffleMask, ScaledMaskUnused;
4854348557
SmallVector<SDValue, 2> ShuffleInputs;
4854448558
if (NumElts <= CmpBits &&
4854548559
getTargetShuffleInputs(peekThroughBitcasts(Vec), ShuffleInputs,
4854648560
ShuffleMask, DAG) &&
4854748561
ShuffleInputs.size() == 1 && !isAnyZeroOrUndef(ShuffleMask) &&
48548-
ShuffleInputs[0].getValueSizeInBits() == VecVT.getSizeInBits()) {
48562+
ShuffleInputs[0].getValueSizeInBits() == VecVT.getSizeInBits() &&
48563+
scaleShuffleElements(ShuffleMask, NumElts, ScaledMaskUnused)) {
4854948564
unsigned NumShuffleElts = ShuffleMask.size();
4855048565
APInt DemandedElts = APInt::getZero(NumShuffleElts);
4855148566
for (int M : ShuffleMask) {

llvm/test/CodeGen/X86/movmsk-cmp.ll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4458,13 +4458,14 @@ define i32 @PR39665_c_ray_opt(<2 x double> %x, <2 x double> %y) {
44584458
define i32 @pr67287(<2 x i64> %broadcast.splatinsert25) {
44594459
; SSE2-LABEL: pr67287:
44604460
; SSE2: # %bb.0: # %entry
4461-
; SSE2-NEXT: movl $3, %eax
4462-
; SSE2-NEXT: testl %eax, %eax
4463-
; SSE2-NEXT: jne .LBB97_2
4464-
; SSE2-NEXT: # %bb.1: # %entry
44654461
; SSE2-NEXT: pand {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
44664462
; SSE2-NEXT: pxor %xmm1, %xmm1
44674463
; SSE2-NEXT: pcmpeqd %xmm0, %xmm1
4464+
; SSE2-NEXT: pshufd {{.*#+}} xmm0 = xmm1[1,0,3,2]
4465+
; SSE2-NEXT: movmskpd %xmm0, %eax
4466+
; SSE2-NEXT: testl %eax, %eax
4467+
; SSE2-NEXT: jne .LBB97_2
4468+
; SSE2-NEXT: # %bb.1: # %entry
44684469
; SSE2-NEXT: movd %xmm1, %eax
44694470
; SSE2-NEXT: testb $1, %al
44704471
; SSE2-NEXT: jne .LBB97_2

0 commit comments

Comments
 (0)