[llvm-commits] [LLVMdev] [PATCH] BasicBlock Autovectorization Pass

[Hal Finkel]

On Wed, 2012-01-25 at 11:12 +0100, Tobias Grosser wrote:
> On 01/24/2012 10:01 PM, Hal Finkel wrote:
> > I have attached the latest version of my basic-block autovectorization
> > pass.
> 
> Nice.
> 
> 
> > With regard to the non-trivial cycle checking I had mentioned
> > previously, I implemented the "late abort" solution and made it the
> > default for cases where the full cycle check would be expensive (for
> > blocks that have many candidate pairs). For blocks with fewer candidate
> > pairs, the full cycle check is used.
> 
> Good.
> 
> > I believe that I have addressed all concerns raised thus far (except for
> > the container Value* ->  Instruction* type changes, which Tobias said he
> > would be okay with having changed post commit). If I receive no
> > objections over the next few days, I'll commit.
> 
> Alright.
> 
> > I would like to thank everyone who has provided feedback, many of the
> > suggestions have proved quite valuable.
> 
> A final nitpick:
> 
> > +    if (CallInst *C = dyn_cast<CallInst>(I)) {
> > +      if (!isVectorizableIntrinsic(C))
> > +        return false;
> > +    } else if (LoadInst *L = dyn_cast<LoadInst>(I)) {
> > +      // Vectorize simple loads if possbile:
> > +      IsSimpleLoadStore = L->isSimple();
> > +      if (!IsSimpleLoadStore || NoMemOps)
> > +        return false;
> > +    } else if (StoreInst *S = dyn_cast<StoreInst>(I)) {
> > +      // Vectorize simple stores if possbile:
> > +      IsSimpleLoadStore = S->isSimple();
> > +      if (!IsSimpleLoadStore || NoMemOps)
> > +        return false;
> > +    } else if (CastInst *C = dyn_cast<CastInst>(I)) {
> > +      // We can vectorize casts, but not casts of pointer types, etc.
> > +      if (NoCasts)
> > +        return false;
> > +
> > +      Type *SrcTy = C->getSrcTy();
> > +      if (!SrcTy->isSingleValueType() || SrcTy->isPointerTy())
> > +        return false;
> > +
> > +      Type *DestTy = C->getDestTy();
> > +      if (!DestTy->isSingleValueType() || DestTy->isPointerTy())
> > +        return false;
> > +    } else if (!(I->isBinaryOp() || isa<ShuffleVectorInst>(I) ||
> > +        isa<ExtractElementInst>(I) || isa<InsertElementInst>(I)))
> > +      return false;
> 
> You may want to add braces to a single statement branch, if the other
> branches have also braces. (I think I have seen this happening a couple
> of times).

Good, I prefer it this way (updated patch attached). I'll commit this
today or tomorrow. Has anyone checked the CMake build with this patch
recently?

It looks like I'll also have to fix how the patch interacts with
instruction metadata, but I'll look at that after I commit an initial
version. If anyone has any thoughts on this, please share.

Thanks again,
Hal

> 
> Cheers
> Tobi

-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm_bb_vectorize-20120128.diff
Type: text/x-patch
Size: 128776 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120128/0b2c5970/attachment.bin>