[llvm] r278913 - Fix bug in DAGBuilder for getelementptr with expanded vector.

[Musa, Ayman via llvm-commits]

Hi,

As you can see in the test, the problem is exposed when using the getelementptr instruction with a type that is not a Machine Value Type.
For example in the test, with the x86 64bit architecture, the value type of the argument %tmp is 64v64i which is not a machine value type. So when asking to get the VT of this variable (by calling MVT::getVectorVT(Ptr, VectorWidth)) the value returned is MVT::INVALID_SIMPLE_VALUE_TYPE.
This is not the wanted behavior as it leads to compilation error (Assertion fails):
llc: llvm/include/llvm/Support/Casting.h:95: static bool llvm::isa_impl_cl<To, const From*>::doit(const From*) [with To = llvm::VectorType; From = llvm::Type]: Assertion `Val && "isa<> used on a null pointer"' failed.

Thanks,
Ayman

-----Original Message-----
From: Quentin Colombet [mailto:qcolombet at apple.com] 
Sent: Thursday, August 18, 2016 02:06
To: Musa, Ayman <ayman.musa at intel.com>
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r278913 - Fix bug in DAGBuilder for getelementptr with expanded vector.

Hi Ayman,

I am not seeing a PR related to this (I am not asking for you to create one :)).
But could you comment on:
- When was the problem introduced?
- What does it take to expose it? I.e., what are the different conditions that need to line up for it to show up?
- What is the symptom when the bug is hit? I am guessing miscompile.

In the future, please put that information in the commit message.

Thanks,
-Quentin
> On Aug 17, 2016, at 12:52 AM, Ayman Musa via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: aymanmus
> Date: Wed Aug 17 02:52:15 2016
> New Revision: 278913
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=278913&view=rev
> Log:
> Fix bug in DAGBuilder for getelementptr with expanded vector.
> 
> Replacing the usage of MVT with EVT in case the vector type is expanded.
> Differential Revision: https://reviews.llvm.org/D23306
> 
> Added:
>    llvm/trunk/test/CodeGen/X86/gep-expanded-vector.ll
> Modified:
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDA
> G/SelectionDAGBuilder.cpp?rev=278913&r1=278912&r2=278913&view=diff
> ======================================================================
> ========
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp 
> (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Wed 
> +++ Aug 17 02:52:15 2016
> @@ -3329,8 +3329,9 @@ void SelectionDAGBuilder::visitGetElemen
>         if (CI->isZero())
>           continue;
>         APInt Offs = ElementSize * 
> CI->getValue().sextOrTrunc(PtrSize);
> +        LLVMContext &Context = *DAG.getContext();
>         SDValue OffsVal = VectorWidth ?
> -          DAG.getConstant(Offs, dl, MVT::getVectorVT(PtrTy, VectorWidth)) :
> +          DAG.getConstant(Offs, dl, EVT::getVectorVT(Context, PtrTy, VectorWidth)) :
>           DAG.getConstant(Offs, dl, PtrTy);
> 
>         // In an inbouds GEP with an offset that is nonnegative even 
> when
> 
> Added: llvm/trunk/test/CodeGen/X86/gep-expanded-vector.ll
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/gep-ex
> panded-vector.ll?rev=278913&view=auto
> ======================================================================
> ========
> --- llvm/trunk/test/CodeGen/X86/gep-expanded-vector.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/gep-expanded-vector.ll Wed Aug 17 
> +++ 02:52:15 2016
> @@ -0,0 +1,24 @@
> +; RUN: llc < %s -O2 -mattr=avx512f -mtriple=x86_64-unknown | 
> +FileCheck %s
> +
> +%struct.S1 = type { %struct.S1*, %struct.S1* }
> +
> +define %struct.S1** @malloc_init_state(<64 x %struct.S1**> %tmp, i32 
> +%ind) {
> +entry:
> +  %Vec = getelementptr inbounds %struct.S1*, <64 x %struct.S1**> %tmp 
> +, i64 2
> +  %ptr = extractelement <64 x %struct.S1**> %Vec, i32 %ind
> +  ret %struct.S1** %ptr
> +}
> +
> +; CHECK: .LCPI0_0:
> +; CHECK: .quad 16
> +; CHECK: vpbroadcastq    .LCPI0_0(%rip), [[Z1:%zmm[0-9]]]
> +; CHECK-NEXT: vpaddq  [[Z1]], [[Z2:%zmm[0-9]]], [[Z2]] ; CHECK-NEXT: 
> +vpaddq  [[Z1]], [[Z3:%zmm[0-9]]], [[Z3]] ; CHECK-NEXT: vpaddq  
> +[[Z1]], [[Z4:%zmm[0-9]]], [[Z4]] ; CHECK-NEXT: vpaddq  [[Z1]], 
> +[[Z5:%zmm[0-9]]], [[Z5]] ; CHECK-NEXT: vpaddq  [[Z1]], 
> +[[Z6:%zmm[0-9]]], [[Z6]] ; CHECK-NEXT: vpaddq  [[Z1]], 
> +[[Z7:%zmm[0-9]]], [[Z7]] ; CHECK-NEXT: vpaddq  [[Z1]], 
> +[[Z8:%zmm[0-9]]], [[Z8]] ; CHECK-NEXT: vpaddq  [[Z1]], 
> +[[Z9:%zmm[0-9]]], [[Z9]]
> +
> +
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.