This is an archive of the discontinued LLVM Phabricator instance.

Differential D146460

[SelectionDAG] Correctly reduce BV to shuffle with zero on big endian
ClosedPublic

Authored by nemanjai on Mar 20 2023, 3:04 PM.

Details

Reviewers
arsenm
efriedma
RKSimon
Group Reviewers
Restricted Project
Commits
rGe7c35d71007f: [SelectionDAG] Correctly reduce BV to shuffle with zero on big endian
Summary

This DAG combine is correct on little endian targets but is incorrect on big endian targets. Add big endian code to correct it.

Diff Detail

Event Timeline

nemanjai created this revision.Mar 20 2023, 3:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 20 2023, 3:04 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
nemanjai requested review of this revision.Mar 20 2023, 3:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 20 2023, 3:04 PM
Herald added a subscriber: wdng. · View Herald Transcript
Harbormaster completed remote builds in B220559: Diff 506739.Mar 20 2023, 4:25 PM
RKSimon accepted this revision.Mar 21 2023, 3:09 AM
RKSimon added a subscriber: RKSimon.

LGTM with one very optional minor

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22013

(very pedantic) Maybe worth pulling out this:

unsigned Low = DAG.getDataLayout().isBigEndian() ? (ZextRatio - 1) : 0;
ShufMask[i] = ((i % ZextRatio) == Low) ? Extract.getConstantOperandVal(1) : NumMaskElts;
This revision is now accepted and ready to land.Mar 21 2023, 3:09 AM
nemanjai added inline comments.Mar 24 2023, 7:19 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
22013

I don't mind getting rid of the tiny if/else statements. Thanks. I'll update it.

Closed by commit rGe7c35d71007f: [SelectionDAG] Correctly reduce BV to shuffle with zero on big endian (authored by nemanjai). · Explain WhyMar 24 2023, 7:57 AM
This revision was automatically updated to reflect the committed changes.
nemanjai added a commit: rGe7c35d71007f: [SelectionDAG] Correctly reduce BV to shuffle with zero on big endian.

Revision Contents

PathSize
llvm/
lib/
CodeGen/
SelectionDAG/
7 lines
test/
CodeGen/
PowerPC/
64 lines

Diff 508096

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 22,004 Lines • ▼ Show 20 Linesstatic SDValue reduceBuildVecToShuffleWithZero(SDNode *BV, SelectionDAG &DAG) {
int NumMaskElts = NumBVOps * ZextRatio; int NumMaskElts = NumBVOps * ZextRatio;
SmallVector<int, 32> ShufMask(NumMaskElts, -1); SmallVector<int, 32> ShufMask(NumMaskElts, -1);
for (int i = 0; i != NumMaskElts; ++i) { for (int i = 0; i != NumMaskElts; ++i) {
if (i / ZextRatio == ZextElt) { if (i / ZextRatio == ZextElt) {
// The low bits of the (potentially translated) extracted element map to // The low bits of the (potentially translated) extracted element map to
// the source vector. The high bits map to zero. We will use a zero vector // the source vector. The high bits map to zero. We will use a zero vector
// as the 2nd source operand of the shuffle, so use the 1st element of // as the 2nd source operand of the shuffle, so use the 1st element of
// that vector (mask value is number-of-elements) for the high bits. // that vector (mask value is number-of-elements) for the high bits.
if (i % ZextRatio == 0) int Low = DAG.getDataLayout().isBigEndian() ? (ZextRatio - 1) : 0;
RKSimonUnsubmitted
Not Done
ReplyInline Actions

(very pedantic) Maybe worth pulling out this:

unsigned Low = DAG.getDataLayout().isBigEndian() ? (ZextRatio - 1) : 0;
ShufMask[i] = ((i % ZextRatio) == Low) ? Extract.getConstantOperandVal(1) : NumMaskElts;
RKSimon: (very pedantic) Maybe worth pulling out this: ``` unsigned Low = DAG.getDataLayout().
nemanjaiAuthorUnsubmitted
Done
ReplyInline Actions

I don't mind getting rid of the tiny if/else statements. Thanks. I'll update it.

nemanjai: I don't mind getting rid of the tiny `if/else` statements. Thanks. I'll update it.
ShufMask[i] = Extract.getConstantOperandVal(1); ShufMask[i] = (i % ZextRatio == Low) ? Extract.getConstantOperandVal(1)
else : NumMaskElts;
ShufMask[i] = NumMaskElts;
} }
// Undef elements of the build vector remain undef because we initialize // Undef elements of the build vector remain undef because we initialize
// the shuffle mask with -1. // the shuffle mask with -1.
} }
// buildvec undef, ..., (zext (extractelt V, IndexC)), undef... --> // buildvec undef, ..., (zext (extractelt V, IndexC)), undef... -->
// bitcast (shuffle V, ZeroVec, VectorMask) // bitcast (shuffle V, ZeroVec, VectorMask)
▲ Show 20 Lines • Show All 5,089 Lines • Show Last 20 Lines

llvm/test/CodeGen/PowerPC/pr61315.ll

; RUN: llc -verify-machineinstrs -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \ ; RUN: llc -verify-machineinstrs -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \
; RUN: -mcpu=pwr9 -mtriple=powerpc64 < %s | FileCheck %s ; RUN: -mcpu=pwr9 -mtriple=powerpc64 < %s | FileCheck %s
define dso_local <16 x i8> @ConvertExtractedMaskBitsToVect(<16 x i8> noundef %0) local_unnamed_addr #0 { define dso_local <16 x i8> @ConvertExtractedMaskBitsToVect(<16 x i8> noundef %0) local_unnamed_addr #0 {
; CHECK: .LCPI0_0: ; CHECK: .LCPI0_0:
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-LABEL: ConvertExtractedMaskBitsToVect: ; CHECK-LABEL: ConvertExtractedMaskBitsToVect:
; CHECK: # %bb.0: ; CHECK: # %bb.0:
; CHECK-NEXT: addis r3, r2, .LCPI0_0@toc@ha ; CHECK-NEXT: addis r3, r2, .LCPI0_0@toc@ha
; CHECK-NEXT: xxlxor v4, v4, v4 ; CHECK-NEXT: xxlxor v4, v4, v4
; CHECK-NEXT: xxlxor v3, v3, v3 ; CHECK-NEXT: xxlxor v3, v3, v3
; CHECK-NEXT: addi r3, r3, .LCPI0_0@toc@l ; CHECK-NEXT: addi r3, r3, .LCPI0_0@toc@l
; CHECK-NEXT: lxv vs0, 0(r3) ; CHECK-NEXT: lxv vs0, 0(r3)
; CHECK-NEXT: addis r3, r2, .LCPI0_1@toc@ha ; CHECK-NEXT: addis r3, r2, .LCPI0_1@toc@ha
Show All 12 Lines; CHECK-NEXT: blr
%a9 = and <16 x i8> %a8, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128> %a9 = and <16 x i8> %a8, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128>
%a10 = icmp eq <16 x i8> %a9, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128> %a10 = icmp eq <16 x i8> %a9, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128>
%a11 = sext <16 x i1> %a10 to <16 x i8> %a11 = sext <16 x i1> %a10 to <16 x i8>
ret <16 x i8> %a11 ret <16 x i8> %a11
} }
define dso_local <16 x i8> @ConvertExtractedMaskBitsToVect2(<16 x i8> noundef %0) local_unnamed_addr #0 { define dso_local <16 x i8> @ConvertExtractedMaskBitsToVect2(<16 x i8> noundef %0) local_unnamed_addr #0 {
; CHECK: .LCPI1_0: ; CHECK: .LCPI1_0:
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 7 # 0x7 ; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-LABEL: ConvertExtractedMaskBitsToVect2: ; CHECK-LABEL: ConvertExtractedMaskBitsToVect2:
; CHECK: # %bb.0: ; CHECK: # %bb.0:
; CHECK-NEXT: addis r3, r2, .LCPI1_0@toc@ha ; CHECK-NEXT: addis r3, r2, .LCPI1_0@toc@ha
; CHECK-NEXT: xxlxor v4, v4, v4 ; CHECK-NEXT: xxlxor v4, v4, v4
; CHECK-NEXT: xxlxor v3, v3, v3 ; CHECK-NEXT: xxlxor v3, v3, v3
; CHECK-NEXT: addi r3, r3, .LCPI1_0@toc@l ; CHECK-NEXT: addi r3, r3, .LCPI1_0@toc@l
; CHECK-NEXT: lxv vs0, 0(r3) ; CHECK-NEXT: lxv vs0, 0(r3)
; CHECK-NEXT: addis r3, r2, .LCPI1_1@toc@ha ; CHECK-NEXT: addis r3, r2, .LCPI1_1@toc@ha
Show All 12 Lines; CHECK-NEXT: blr
%a9 = and <16 x i8> %a8, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128> %a9 = and <16 x i8> %a8, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128>
%a10 = icmp eq <16 x i8> %a9, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128> %a10 = icmp eq <16 x i8> %a9, <i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128, i8 1, i8 2, i8 4, i8 8, i8 16, i8 32, i8 64, i8 -128>
%a11 = sext <16 x i1> %a10 to <16 x i8> %a11 = sext <16 x i1> %a10 to <16 x i8>
ret <16 x i8> %a11 ret <16 x i8> %a11
} }
define dso_local <16 x i8> @ConvertExtractedMaskBitsToVect3(<8 x i16> noundef %0) local_unnamed_addr #0 { define dso_local <16 x i8> @ConvertExtractedMaskBitsToVect3(<8 x i16> noundef %0) local_unnamed_addr #0 {
; CHECK: .LCPI2_0: ; CHECK: .LCPI2_0:
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 17 # 0x11
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 17 # 0x11
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 17 # 0x11
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 17 # 0x11
; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 6 # 0x6 ; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 6 # 0x6 ; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 6 # 0x6 ; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 6 # 0x6 ; CHECK-NEXT: .byte 6 # 0x6
; CHECK-NEXT: .byte 7 # 0x7
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-NEXT: .byte 16 # 0x10
; CHECK-LABEL: ConvertExtractedMaskBitsToVect3: ; CHECK-LABEL: ConvertExtractedMaskBitsToVect3:
; CHECK: # %bb.0: ; CHECK: # %bb.0:
; CHECK-NEXT: addis r3, r2, .LCPI2_0@toc@ha ; CHECK-NEXT: addis r3, r2, .LCPI2_0@toc@ha
; CHECK-NEXT: xxlxor v4, v4, v4 ; CHECK-NEXT: xxlxor v4, v4, v4
; CHECK-NEXT: xxlxor v3, v3, v3 ; CHECK-NEXT: xxlxor v3, v3, v3
; CHECK-NEXT: addi r3, r3, .LCPI2_0@toc@l ; CHECK-NEXT: addi r3, r3, .LCPI2_0@toc@l
; CHECK-NEXT: lxv vs0, 0(r3) ; CHECK-NEXT: lxv vs0, 0(r3)
; CHECK-NEXT: addis r3, r2, .LCPI2_1@toc@ha ; CHECK-NEXT: addis r3, r2, .LCPI2_1@toc@ha
Show All 17 Lines