<p dir="ltr"><br>
On Jul 11, 2013 9:02 AM, "Arnold Schwaighofer" <<a href="mailto:aschwaighofer@apple.com">aschwaighofer@apple.com</a>> wrote:<br>
><br>
><br>
> On Jul 11, 2013, at 10:28 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> > On Thu, Jul 11, 2013 at 8:21 AM, Arnold Schwaighofer<br>
> > <<a href="mailto:aschwaighofer@apple.com">aschwaighofer@apple.com</a>> wrote:<br>
> >> Author: arnolds<br>
> >> Date: Thu Jul 11 10:21:55 2013<br>
> >> New Revision: 186088<br>
> >><br>
> >> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=186088&view=rev">http://llvm.org/viewvc/llvm-project?rev=186088&view=rev</a><br>
> >> Log:<br>
> >> LoopVectorize: Vectorize all accesses in address space zero with unit stride<br>
> ><br>
> > Naively (since I know very little about LLVM optimization details,<br>
> > mostly working up in Clang): do you need to limit this to unit stride?<br>
> > Any object that would include address zero would be invalid, no? (I'm<br>
> > not sure whether vectorization can have holes (eg: elements of size 1<br>
> > but stride 2),<br>
><br>
> In principle the vectorizer can vectorize this with a gather/scather. In many cases the cost model will tell it not to.<br>
><br>
> In this context the “stride” already has the element size factored in.<br>
><br>
> > if so you might need to avoid those - but if it has<br>
> > stride 2 and size 2<br>
><br>
> This is a unit stride. The unit stride I am referring to is taking the element size into account.<br>
><br>
> > and crosses zero even if zero isn't one of the<br>
> > element addresses (instead it's the address of the second byte of an<br>
> > element) should be eligible for the same optimization as size 1 stride<br>
> > 1, no?<br>
><br>
> Yes, you are right. The code already does that.<br>
><br>
> for i in ..: a[i] is a unit stride access.<br>
> while<br>
> for i in ..” a[2*i] is a non unit stride access (“stride 2”)<br>
><br>
> irrespective of the type of a.</p>
<p dir="ltr">Ah, OK. Thanks for the explanation & sorry for the noise.</p>
<p dir="ltr">><br>
><br>
><br>
> ><br>
> >><br>
> >> We can vectorize them because in the case where we wrap in the address space the<br>
> >> unvectorized code would have had to access a pointer value of zero which is<br>
> >> undefined behavior in address space zero according to the LLVM IR semantics.<br>
> >> (Thank you Duncan, for pointing this out to me).<br>
> >><br>
> >> Fixes PR16592.<br>
> >><br>
> >> Added:<br>
> >>    llvm/trunk/test/Transforms/LoopVectorize/safegep.ll<br>
> >> Modified:<br>
> >>    llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp<br>
> >><br>
> >> Modified: llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp<br>
> >> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=186088&r1=186087&r2=186088&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp?rev=186088&r1=186087&r2=186088&view=diff</a><br>

> >> ==============================================================================<br>
> >> --- llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp (original)<br>
> >> +++ llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp Thu Jul 11 10:21:55 2013<br>
> >> @@ -3223,11 +3223,12 @@ static bool isInBoundsGep(Value *Ptr) {<br>
> >> /// \brief Check whether the access through \p Ptr has a constant stride.<br>
> >> static int isStridedPtr(ScalarEvolution *SE, DataLayout *DL, Value *Ptr,<br>
> >>                         const Loop *Lp) {<br>
> >> -  const Type *PtrTy = Ptr->getType();<br>
> >> -  assert(PtrTy->isPointerTy() && "Unexpected non ptr");<br>
> >> +  const Type *Ty = Ptr->getType();<br>
> >> +  assert(Ty->isPointerTy() && "Unexpected non ptr");<br>
> >><br>
> >>   // Make sure that the pointer does not point to aggregate types.<br>
> >> -  if (cast<PointerType>(Ptr->getType())->getElementType()->isAggregateType()) {<br>
> >> +  const PointerType *PtrTy = cast<PointerType>(Ty);<br>
> >> +  if (PtrTy->getElementType()->isAggregateType()) {<br>
> >>     DEBUG(dbgs() << "LV: Bad stride - Not a pointer to a scalar type" << *Ptr<br>
> >>           << "\n");<br>
> >>     return 0;<br>
> >> @@ -3248,11 +3249,16 @@ static int isStridedPtr(ScalarEvolution<br>
> >>   }<br>
> >><br>
> >>   // The address calculation must not wrap. Otherwise, a dependence could be<br>
> >> -  // inverted. An inbounds getelementptr that is a AddRec with a unit stride<br>
> >> +  // inverted.<br>
> >> +  // An inbounds getelementptr that is a AddRec with a unit stride<br>
> >>   // cannot wrap per definition. The unit stride requirement is checked later.<br>
> >> +  // An getelementptr without an inbounds attribute and unit stride would have<br>
> >> +  // to access the pointer value "0" which is undefined behavior in address<br>
> >> +  // space 0, therefore we can also vectorize this case.<br>
> >>   bool IsInBoundsGEP = isInBoundsGep(Ptr);<br>
> >>   bool IsNoWrapAddRec = AR->getNoWrapFlags(SCEV::NoWrapMask);<br>
> >> -  if (!IsNoWrapAddRec && !IsInBoundsGEP) {<br>
> >> +  bool IsInAddressSpaceZero = PtrTy->getAddressSpace() == 0;<br>
> >> +  if (!IsNoWrapAddRec && !IsInBoundsGEP && !IsInAddressSpaceZero) {<br>
> >>     DEBUG(dbgs() << "LV: Bad stride - Pointer may wrap in the address space "<br>
> >>           << *Ptr << " SCEV: " << *PtrScev << "\n");<br>
> >>     return 0;<br>
> >> @@ -3269,7 +3275,7 @@ static int isStridedPtr(ScalarEvolution<br>
> >>     return 0;<br>
> >>   }<br>
> >><br>
> >> -  int64_t Size = DL->getTypeAllocSize(PtrTy->getPointerElementType());<br>
> >> +  int64_t Size = DL->getTypeAllocSize(PtrTy->getElementType());<br>
> >>   const APInt &APStepVal = C->getValue()->getValue();<br>
> >><br>
> >>   // Huge step value - give up.<br>
> >> @@ -3285,8 +3291,10 @@ static int isStridedPtr(ScalarEvolution<br>
> >>     return 0;<br>
> >><br>
> >>   // If the SCEV could wrap but we have an inbounds gep with a unit stride we<br>
> >> -  // know we can't "wrap around the address space".<br>
> >> -  if (!IsNoWrapAddRec && IsInBoundsGEP && Stride != 1 && Stride != -1)<br>
> >> +  // know we can't "wrap around the address space". In case of address space<br>
> >> +  // zero we know that this won't happen without triggering undefined behavior.<br>
> >> +  if (!IsNoWrapAddRec && (IsInBoundsGEP || IsInAddressSpaceZero) &&<br>
> >> +      Stride != 1 && Stride != -1)<br>
> >>     return 0;<br>
> >><br>
> >>   return Stride;<br>
> >><br>
> >> Added: llvm/trunk/test/Transforms/LoopVectorize/safegep.ll<br>
> >> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/safegep.ll?rev=186088&view=auto">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/safegep.ll?rev=186088&view=auto</a><br>

> >> ==============================================================================<br>
> >> --- llvm/trunk/test/Transforms/LoopVectorize/safegep.ll (added)<br>
> >> +++ llvm/trunk/test/Transforms/LoopVectorize/safegep.ll Thu Jul 11 10:21:55 2013<br>
> >> @@ -0,0 +1,61 @@<br>
> >> +; RUN: opt -S -loop-vectorize -force-vector-width=4 -force-vector-unroll=1  < %s |  FileCheck %s<br>
> >> +target datalayout = "e-p:32:32:32-S128-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f16:16:16-f32:32:32-f64:32:64-f128:128:128-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32"<br>
> >> +<br>
> >> +<br>
> >> +; We can vectorize this code because if the address computation would wrap then<br>
> >> +; a load from 0 would take place which is undefined behaviour in address space 0<br>
> >> +; according to LLVM IR semantics.<br>
> >> +<br>
> >> +; PR16592<br>
> >> +<br>
> >> +; CHECK: safe<br>
> >> +; CHECK: <4 x float><br>
> >> +<br>
> >> +define void @safe(float* %A, float* %B, float %K) {<br>
> >> +entry:<br>
> >> +  br label %"<bb 3>"<br>
> >> +<br>
> >> +"<bb 3>":<br>
> >> +  %i_15 = phi i32 [ 0, %entry ], [ %i_19, %"<bb 3>" ]<br>
> >> +  %pp3 = getelementptr float* %A, i32 %i_15<br>
> >> +  %D.1396_10 = load float* %pp3, align 4<br>
> >> +  %pp24 = getelementptr float* %B, i32 %i_15<br>
> >> +  %D.1398_15 = load float* %pp24, align 4<br>
> >> +  %D.1399_17 = fadd float %D.1398_15, %K<br>
> >> +  %D.1400_18 = fmul float %D.1396_10, %D.1399_17<br>
> >> +  store float %D.1400_18, float* %pp3, align 4<br>
> >> +  %i_19 = add nsw i32 %i_15, 1<br>
> >> +  %exitcond = icmp ne i32 %i_19, 64<br>
> >> +  br i1 %exitcond, label %"<bb 3>", label %return<br>
> >> +<br>
> >> +return:<br>
> >> +  ret void<br>
> >> +}<br>
> >> +<br>
> >> +; In a non-default address space we don't have this rule.<br>
> >> +<br>
> >> +; CHECK: notsafe<br>
> >> +; CHECK-NOT: <4 x float><br>
> >> +<br>
> >> +define void @notsafe(float addrspace(5) * %A, float* %B, float %K) {<br>
> >> +entry:<br>
> >> +  br label %"<bb 3>"<br>
> >> +<br>
> >> +"<bb 3>":<br>
> >> +  %i_15 = phi i32 [ 0, %entry ], [ %i_19, %"<bb 3>" ]<br>
> >> +  %pp3 = getelementptr float addrspace(5) * %A, i32 %i_15<br>
> >> +  %D.1396_10 = load float addrspace(5) * %pp3, align 4<br>
> >> +  %pp24 = getelementptr float* %B, i32 %i_15<br>
> >> +  %D.1398_15 = load float* %pp24, align 4<br>
> >> +  %D.1399_17 = fadd float %D.1398_15, %K<br>
> >> +  %D.1400_18 = fmul float %D.1396_10, %D.1399_17<br>
> >> +  store float %D.1400_18, float addrspace(5) * %pp3, align 4<br>
> >> +  %i_19 = add nsw i32 %i_15, 1<br>
> >> +  %exitcond = icmp ne i32 %i_19, 64<br>
> >> +  br i1 %exitcond, label %"<bb 3>", label %return<br>
> >> +<br>
> >> +return:<br>
> >> +  ret void<br>
> >> +}<br>
> >> +<br>
> >> +<br>
> >><br>
> >><br>
> >> _______________________________________________<br>
> >> llvm-commits mailing list<br>
> >> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
</p>