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

Thu Aug 18 12:04:37 PDT 2016

Thanks for the details.

Q.
> On Aug 18, 2016, at 12:22 AM, Musa, Ayman <ayman.musa at intel.com> wrote:
> 
> 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 <mailto:qcolombet at apple.com>] 
> Sent: Thursday, August 18, 2016 02:06
> To: Musa, Ayman <ayman.musa at intel.com <mailto:ayman.musa at intel.com>>
> Cc: llvm-commits at lists.llvm.org <mailto: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.

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