Author: Fraser Cormack Date: 2022-04-25T20:27:35-07:00 New Revision: 9efcce92b55bc87627b52dd1139fb9031e400b52
URL: https://github.com/llvm/llvm-project/commit/9efcce92b55bc87627b52dd1139fb9031e400b52 DIFF: https://github.com/llvm/llvm-project/commit/9efcce92b55bc87627b52dd1139fb9031e400b52.diff LOG: [RISCV] Fix lowering of BUILD_VECTORs as VID sequences This patch fixes a bug when lowering BUILD_VECTOR via VID sequences. After adding support for fractional steps in D106533, elements with zero steps may be skipped if no step has yet been computed. This allowed certain sequences to slip through the cracks, being identified as VID sequences when in fact they are not. The fix for this is to perform a second loop over the BUILD_VECTOR to validate the entire sequence once the step has been computed. This isn't the most efficient, but on balance the code is more readable and maintainable than doing back-validation during the first loop. Fixes the tests introduced in D123785. Reviewed By: craig.topper Differential Revision: https://reviews.llvm.org/D123786 (cherry picked from commit c5cac48549ed254cd5ad5eef770ebfb22ccd9f64) Added: Modified: llvm/lib/Target/RISCV/RISCVISelLowering.cpp llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-buildvec.ll Removed: ################################################################################ diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index e7672a7652cdd..73a29a1d9dcc3 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -1908,37 +1908,27 @@ static Optional<VIDSequence> isSimpleVIDSequence(SDValue Op) { // A zero-value value diff erence means that we're somewhere in the middle // of a fractional step, e.g. <0,0,0*,0,1,1,1,1>. Wait until we notice a // step change before evaluating the sequence. - if (ValDiff != 0) { - int64_t Remainder = ValDiff % IdxDiff; - // Normalize the step if it's greater than 1. - if (Remainder != ValDiff) { - // The diff erence must cleanly divide the element span. - if (Remainder != 0) - return None; - ValDiff /= IdxDiff; - IdxDiff = 1; - } - - if (!SeqStepNum) - SeqStepNum = ValDiff; - else if (ValDiff != SeqStepNum) - return None; + if (ValDiff == 0) + continue; - if (!SeqStepDenom) - SeqStepDenom = IdxDiff; - else if (IdxDiff != *SeqStepDenom) + int64_t Remainder = ValDiff % IdxDiff; + // Normalize the step if it's greater than 1. + if (Remainder != ValDiff) { + // The diff erence must cleanly divide the element span. + if (Remainder != 0) return None; + ValDiff /= IdxDiff; + IdxDiff = 1; } - } - // Record and/or check any addend. - if (SeqStepNum && SeqStepDenom) { - uint64_t ExpectedVal = - (int64_t)(Idx * (uint64_t)*SeqStepNum) / *SeqStepDenom; - int64_t Addend = SignExtend64(Val - ExpectedVal, EltSizeInBits); - if (!SeqAddend) - SeqAddend = Addend; - else if (SeqAddend != Addend) + if (!SeqStepNum) + SeqStepNum = ValDiff; + else if (ValDiff != SeqStepNum) + return None; + + if (!SeqStepDenom) + SeqStepDenom = IdxDiff; + else if (IdxDiff != *SeqStepDenom) return None; } @@ -1946,11 +1936,29 @@ static Optional<VIDSequence> isSimpleVIDSequence(SDValue Op) { if (!PrevElt || PrevElt->first != Val) PrevElt = std::make_pair(Val, Idx); } - // We need to have logged both a step and an addend for this to count as - // a legal index sequence. - if (!SeqStepNum || !SeqStepDenom || !SeqAddend) + + // We need to have logged a step for this to count as a legal index sequence. + if (!SeqStepNum || !SeqStepDenom) return None; + // Loop back through the sequence and validate elements we might have skipped + // while waiting for a valid step. While doing this, log any sequence addend. + for (unsigned Idx = 0; Idx < NumElts; Idx++) { + if (Op.getOperand(Idx).isUndef()) + continue; + uint64_t Val = Op.getConstantOperandVal(Idx) & + maskTrailingOnes<uint64_t>(EltSizeInBits); + uint64_t ExpectedVal = + (int64_t)(Idx * (uint64_t)*SeqStepNum) / *SeqStepDenom; + int64_t Addend = SignExtend64(Val - ExpectedVal, EltSizeInBits); + if (!SeqAddend) + SeqAddend = Addend; + else if (Addend != SeqAddend) + return None; + } + + assert(SeqAddend && "Must have an addend if we have a step"); + return VIDSequence{*SeqStepNum, *SeqStepDenom, *SeqAddend}; } diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-buildvec.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-buildvec.ll index 6fb669feb462a..085f5bc1d8493 100644 --- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-buildvec.ll +++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-buildvec.ll @@ -705,23 +705,24 @@ define <8 x i16> @splat_idx_v8i16(<8 x i16> %v, i64 %idx) { ret <8 x i16> %splat } -; FIXME: This is not a vid sequence! define <4 x i8> @buildvec_not_vid_v4i8_1() { ; CHECK-LABEL: buildvec_not_vid_v4i8_1: ; CHECK: # %bb.0: +; CHECK-NEXT: lui a0, %hi(.LCPI37_0) +; CHECK-NEXT: addi a0, a0, %lo(.LCPI37_0) ; CHECK-NEXT: vsetivli zero, 4, e8, mf4, ta, mu -; CHECK-NEXT: vid.v v8 +; CHECK-NEXT: vle8.v v8, (a0) ; CHECK-NEXT: ret ret <4 x i8> <i8 0, i8 0, i8 2, i8 3> } -; FIXME: This is not a vid sequence! define <4 x i8> @buildvec_not_vid_v4i8_2() { ; CHECK-LABEL: buildvec_not_vid_v4i8_2: ; CHECK: # %bb.0: +; CHECK-NEXT: lui a0, %hi(.LCPI38_0) +; CHECK-NEXT: addi a0, a0, %lo(.LCPI38_0) ; CHECK-NEXT: vsetivli zero, 4, e8, mf4, ta, mu -; CHECK-NEXT: vid.v v8 -; CHECK-NEXT: vrsub.vi v8, v8, 3 +; CHECK-NEXT: vle8.v v8, (a0) ; CHECK-NEXT: ret ret <4 x i8> <i8 3, i8 3, i8 1, i8 0> } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
