[llvm] r247128 - Fix vector splitting for extract_vector_elt and vector elements of <8-bits.
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 14 03:42:40 PDT 2015
Thanks. Merged in r247539
> -----Original Message-----
> From: Owen Anderson [mailto:resistor at mac.com]
> Sent: 10 September 2015 17:38
> To: Tom Stellard
> Cc: Daniel Sanders; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r247128 - Fix vector splitting for extract_vector_elt and
> vector elements of <8-bits.
>
> Fine with me.
>
> —Owen
>
> > On Sep 10, 2015, at 6:56 AM, Tom Stellard via llvm-commits <llvm-
> commits at lists.llvm.org> wrote:
> >
> > On Thu, Sep 10, 2015 at 12:35:49PM +0000, Daniel Sanders wrote:
> >> Hi,
> >>
> >> Is it ok to merge this to the 3.7 branch so that it's in 3.7.1?
> >>
> >
> > It's OK with me as long as the code owner approver, Owen, approves.
> >
> >>> -----Original Message-----
> >>> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
> Behalf
> >>> Of Daniel Sanders via llvm-commits
> >>> Sent: 09 September 2015 10:53
> >>> To: llvm-commits at lists.llvm.org
> >>> Subject: [llvm] r247128 - Fix vector splitting for extract_vector_elt and
> vector
> >>> elements of <8-bits.
> >>>
> >>> Author: dsanders
> >>> Date: Wed Sep 9 04:53:20 2015
> >>> New Revision: 247128
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=247128&view=rev
> >>> Log:
> >>> Fix vector splitting for extract_vector_elt and vector elements of <8-bits.
> >>>
> >>> Summary:
> >>> One of the vector splitting paths for extract_vector_elt tries to lower:
> >>> define i1 @via_stack_bug(i8 signext %idx) {
> >>> %1 = extractelement <2 x i1> <i1 false, i1 true>, i8 %idx
> >>> ret i1 %1
> >>> }
> >>> to:
> >>> define i1 @via_stack_bug(i8 signext %idx) {
> >>> %base = alloca <2 x i1>
> >>> store <2 x i1> <i1 false, i1 true>, <2 x i1>* %base
> >>> %2 = getelementptr <2 x i1>, <2 x i1>* %base, i32 %idx
> >>> %3 = load i1, i1* %2
> >>> ret i1 %3
> >>> }
> >>> However, the elements of <2 x i1> are not byte-addressible. The result
> of
> >>> this
> >>> is that the getelementptr expands to '%base + %idx * (1 / 8)' which
> simplifies
> >>> to '%base + %idx * 0', and then simply '%base' causing all values of %idx
> to
> >>> extract element zero.
> >>>
> >>> This commit fixes this by promoting the vector elements of <8-bits to i8
> >>> before
> >>> splitting the vector.
> >>>
> >>> This fixes a number of test failures in pocl.
> >>>
> >>> Reviewers: pekka.jaaskelainen
> >>>
> >>> Subscribers: pekka.jaaskelainen, llvm-commits
> >>>
> >>> Differential Revision: http://reviews.llvm.org/D12591
> >>>
> >>> Added:
> >>> llvm/trunk/test/CodeGen/Mips/llvm-ir/extractelement.ll
> >>> Modified:
> >>> llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
> >>> llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
> >>>
> >>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
> >>> URL: http://llvm.org/viewvc/llvm-
> >>>
> project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp?rev=2471
> >>> 28&r1=247127&r2=247128&view=diff
> >>>
> ==========================================================
> >>> ====================
> >>> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp (original)
> >>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp Wed Sep
> 9
> >>> 04:53:20 2015
> >>> @@ -1007,6 +1007,8 @@ SDValue DAGTypeLegalizer::GetVectorEleme
> >>>
> >>> // Calculate the element offset and add it to the pointer.
> >>> unsigned EltSize = EltVT.getSizeInBits() / 8; // FIXME: should be ABI size.
> >>> + assert(EltSize * 8 == EltVT.getSizeInBits() &&
> >>> + "Converting bits to bytes lost precision");
> >>>
> >>> Index = DAG.getNode(ISD::MUL, dl, Index.getValueType(), Index,
> >>> DAG.getConstant(EltSize, dl, Index.getValueType()));
> >>>
> >>> Modified:
> llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
> >>> URL: http://llvm.org/viewvc/llvm-
> >>>
> project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp?re
> >>> v=247128&r1=247127&r2=247128&view=diff
> >>>
> ==========================================================
> >>> ====================
> >>> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
> (original)
> >>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
> Wed
> >>> Sep 9 04:53:20 2015
> >>> @@ -1554,9 +1554,25 @@ SDValue DAGTypeLegalizer::SplitVecOp_EXT
> >>> if (CustomLowerNode(N, N->getValueType(0), true))
> >>> return SDValue();
> >>>
> >>> - // Store the vector to the stack.
> >>> - EVT EltVT = VecVT.getVectorElementType();
> >>> + // Make the vector elements byte-addressable if they aren't already.
> >>> SDLoc dl(N);
> >>> + EVT EltVT = VecVT.getVectorElementType();
> >>> + if (EltVT.getSizeInBits() < 8) {
> >>> + SmallVector<SDValue, 4> ElementOps;
> >>> + for (unsigned i = 0; i < VecVT.getVectorNumElements(); ++i) {
> >>> + ElementOps.push_back(DAG.getAnyExtOrTrunc(
> >>> + DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, EltVT, Vec,
> >>> + DAG.getConstant(i, dl, MVT::i8)),
> >>> + dl, MVT::i8));
> >>> + }
> >>> +
> >>> + EltVT = MVT::i8;
> >>> + VecVT = EVT::getVectorVT(*DAG.getContext(), EltVT,
> >>> + VecVT.getVectorNumElements());
> >>> + Vec = DAG.getNode(ISD::BUILD_VECTOR, dl, VecVT, ElementOps);
> >>> + }
> >>> +
> >>> + // Store the vector to the stack.
> >>> SDValue StackPtr = DAG.CreateStackTemporary(VecVT);
> >>> SDValue Store = DAG.getStore(DAG.getEntryNode(), dl, Vec, StackPtr,
> >>> MachinePointerInfo(), false, false, 0);
> >>>
> >>> Added: llvm/trunk/test/CodeGen/Mips/llvm-ir/extractelement.ll
> >>> URL: http://llvm.org/viewvc/llvm-
> >>> project/llvm/trunk/test/CodeGen/Mips/llvm-
> >>> ir/extractelement.ll?rev=247128&view=auto
> >>>
> ==========================================================
> >>> ====================
> >>> --- llvm/trunk/test/CodeGen/Mips/llvm-ir/extractelement.ll (added)
> >>> +++ llvm/trunk/test/CodeGen/Mips/llvm-ir/extractelement.ll Wed Sep
> 9
> >>> 04:53:20 2015
> >>> @@ -0,0 +1,19 @@
> >>> +; RUN: llc < %s -march=mips -mcpu=mips2 | FileCheck %s -check-
> prefix=ALL
> >>> +
> >>> +; This test triggered a bug in the vector splitting where the type legalizer
> >>> +; attempted to extract the element with by storing the vector, then
> reading
> >>> +; an element back. However, the address calculation was:
> >>> +; Base + Index * (EltSizeInBits / 8)
> >>> +; and EltSizeInBits was 1. This caused the index to be forgotten.
> >>> +define i1 @via_stack_bug(i8 signext %idx) {
> >>> + %1 = extractelement <2 x i1> <i1 false, i1 true>, i8 %idx
> >>> + ret i1 %1
> >>> +}
> >>> +
> >>> +; ALL-LABEL: via_stack_bug:
> >>> +; ALL-DAG: addiu [[ONE:\$[0-9]+]], $zero, 1
> >>> +; ALL-DAG: sb [[ONE]], 7($sp)
> >>> +; ALL-DAG: sb $zero, 6($sp)
> >>> +; ALL-DAG: addiu [[VPTR:\$[0-9]+]], $sp, 6
> >>> +; ALL-DAG: addu [[EPTR:\$[0-9]+]], $4, [[VPTR]]
> >>> +; ALL: lbu $2, 0([[EPTR]])
> >>>
> >>>
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list