[llvm] r342183 - [DAGCombine] Fix crash when store merging created an extract_subvector with invalid index.

[Amara Emerson via llvm-commits]

Thanks Reid, I’ll take a look.

Amara

> On Sep 14, 2018, at 9:00 PM, Reid Kleckner <rnk at google.com> wrote:
> 
> I got this reduction:
> $ cat geometry.cpp
> typedef float a __attribute__((__vector_size__(16)));
> a b();
> float c();
> struct d {
>   float e;
>   float f;
> };
> struct g {
>   g(float) : h(b()) {}
>   void i(void *j) {
>     __builtin_ia32_storelps(
>         (__attribute__((__vector_size__(8))) int *)j, h);
>   }
>   a h;
> };
> d k(g j) {
>   d l;
>   j.i(&l);
>   return l;
> }
> bool m;
> float n;
> struct o {
>   d p[];
>   void q(o[]) const;
> };
> void o::q(o *j) const {
>   d r = k(n);
>   if (m)
>     r.e = c();
>   j[0].p[1] = k(0);
>   j[1].p[0] = r;
> }
> 
> $ ./bin/clang++ -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -target-cpu x86-64 -O2 geometry.cpp |& head
> clang++: /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/CodeGen/ValueTypes.h:274: unsigned int llvm::EVT::getVectorNumElements() const: Assertion `isVector() && "Invalid vector type!"' failed.
> 
> On Fri, Sep 14, 2018 at 12:41 PM Reid Kleckner <rnk at google.com <mailto:rnk at google.com>> wrote:
> I reverted this in r342265 because it caused assertion failures while building Chromium. I'm starting a reduction.
> 
> On Thu, Sep 13, 2018 at 2:30 PM Amara Emerson via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: aemerson
> Date: Thu Sep 13 14:28:58 2018
> New Revision: 342183
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=342183&view=rev <http://llvm.org/viewvc/llvm-project?rev=342183&view=rev>
> Log:
> [DAGCombine] Fix crash when store merging created an extract_subvector with invalid index.
> 
> Differential Revision: https://reviews.llvm.org/D51831 <https://reviews.llvm.org/D51831>
> 
> Added:
>     llvm/trunk/test/CodeGen/X86/merge-vector-stores-scale-idx-crash.ll
> Modified:
>     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=342183&r1=342182&r2=342183&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=342183&r1=342182&r2=342183&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Thu Sep 13 14:28:58 2018
> @@ -13843,17 +13843,24 @@ bool DAGCombiner::MergeStoresOfConstants
>               Val.getOpcode() == ISD::EXTRACT_SUBVECTOR)) {
>            SDValue Vec = Val.getOperand(0);
>            EVT MemVTScalarTy = MemVT.getScalarType();
> +          SDValue Idx = Val.getOperand(1);
>            // We may need to add a bitcast here to get types to line up.
>            if (MemVTScalarTy != Vec.getValueType()) {
>              unsigned Elts = Vec.getValueType().getSizeInBits() /
>                              MemVTScalarTy.getSizeInBits();
> +            if (Val.getValueType().isVector()) {
> +              unsigned IdxC = cast<ConstantSDNode>(Idx)->getZExtValue();
> +              unsigned NewIdx =
> +                  ((uint64_t)IdxC * MemVT.getVectorNumElements()) / Elts;
> +              Idx = DAG.getConstant(NewIdx, SDLoc(Val), Idx.getValueType());
> +            }
>              EVT NewVecTy =
>                  EVT::getVectorVT(*DAG.getContext(), MemVTScalarTy, Elts);
>              Vec = DAG.getBitcast(NewVecTy, Vec);
>            }
>            auto OpC = (MemVT.isVector()) ? ISD::EXTRACT_SUBVECTOR
>                                          : ISD::EXTRACT_VECTOR_ELT;
> -          Val = DAG.getNode(OpC, SDLoc(Val), MemVT, Vec, Val.getOperand(1));
> +          Val = DAG.getNode(OpC, SDLoc(Val), MemVT, Vec, Idx);
>          }
>          Ops.push_back(Val);
>        }
> 
> Added: llvm/trunk/test/CodeGen/X86/merge-vector-stores-scale-idx-crash.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/merge-vector-stores-scale-idx-crash.ll?rev=342183&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/merge-vector-stores-scale-idx-crash.ll?rev=342183&view=auto>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/merge-vector-stores-scale-idx-crash.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/merge-vector-stores-scale-idx-crash.ll Thu Sep 13 14:28:58 2018
> @@ -0,0 +1,19 @@
> +; RUN: llc < %s  -mtriple=x86_64-apple-osx10.14 -mattr=+avx2 | FileCheck %s
> +
> +; Check that we don't crash due creating invalid extract_subvector indices in store merging.
> +; CHECK-LABEL: testfn
> +; CHECK: retq
> +define void @testfn(i32* nocapture %p) {
> +  %v0 = getelementptr i32, i32* %p, i64 12
> +  %1 = bitcast i32* %v0 to <2 x i64>*
> +  %2 = bitcast i32* %v0 to <4 x i32>*
> +  %3 = getelementptr <2 x i64>, <2 x i64>* %1, i64 -3
> +  store <2 x i64> undef, <2 x i64>* %3, align 16
> +  %4 = shufflevector <4 x i64> zeroinitializer, <4 x i64> undef, <2 x i32> <i32 0, i32 1>
> +  %5 = getelementptr <2 x i64>, <2 x i64>* %1, i64 -2
> +  store <2 x i64> %4, <2 x i64>* %5, align 16
> +  %6 = shufflevector <8 x i32> zeroinitializer, <8 x i32> undef, <4 x i32> <i32 4, i32 5, i32 6, i32 7>
> +  %7 = getelementptr <4 x i32>, <4 x i32>* %2, i64 -1
> +  store <4 x i32> %6, <4 x i32>* %7, align 16
> +  ret void
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180914/06707cc9/attachment.html>