[llvm-commits] [PATCH] LoopVectorizer

[Nadav Rotem]

On Oct 16, 2012, at 11:30 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
>> From: "Nadav Rotem" <nrotem at apple.com>
>> To: "llvm-commits at cs.uiuc.edu LLVM" <llvm-commits at cs.uiuc.edu>
>> Sent: Tuesday, October 16, 2012 1:05:31 PM
>> Subject: [llvm-commits] [PATCH] LoopVectorizer
>> 
>> Hi,
>> 
>> I attached a patch for a loop vectorizer. 
> 
> Great, I'm very excited about this!
> 
> A few comments:
> 
>> +class SingleBlockLoopVectorizer {
> 
> Are you planning on renaming this pass later?

This is not a pass, this is a helper class.  We may want to extract it to a pass in order to be able to test it independently. 

> 
>> +  return Builder->CreateAdd(Val, Cv, "Induction");
> 
> I think that most passes create instructions with all-lowercase names.
> 
>> +      Cloned->setName(Instr->getName() + "cloned");
> 

Done. All IR names are lowercase now.
 
> Likewise, add ".cloned" (if there is an existing name).
> 
>> +        WidenMap[Inst] = Builder->CreateBinOp(BinOp->getOpcode(), A, B);
> 
> We should probably make sure that:
> - we propagate nsw, etc. flags when possible
> - propagate debug locations and other metadata

Yes, but I plan to do it later. 

> 
>> +      case Instruction::Store: {
> 
> This block does not seem to exclude volatile stores (or atomics), was that intentional? We may just want to exclude everything that is !isSimple(). The same for loads.
> 

Good catch! Fixed. 

>> +    //Check that all of the users of the loop are inside the BB.
>> +    for (Value::use_iterator it = I->use_begin(), e = I->use_end();
>> +         it != e; ++it) {
>> +      Instruction *U = cast<Instruction>(*it);
>> +      BasicBlock *Parent = U->getParent();
>> +      if (Parent != &BB) {
>> +        DEBUG(dbgs() << "LV: Found an outside user for : "<< *U << "\n");
>> +        return false;
>> +      }
>> +    }
> 
> It seems like handling this case requires just as much code as this check. If there is an outside user, you just need to extract the last vector element and call replaceAllUsesWith(), right?
> 

This is not a legal transformation. I wrote about it in the reply to Ralf. 

Thanks for the review!


> Thanks again,
> Hal
> 
>> The loop vectorizer
>> currently supports single basic block loops, with a very simple and
>> strict set of legality rules. It only supports trip counts which are
>> multiple of the vectorization factor. It does not have a cost model
>> at all. We are now able to vectorize a good number of workloads from
>> the gcc vectorizer example page [1].  We are also able to vectorize
>> a small number of loop from the test-stuite.  Here are some
>> benchmark results of the vectorizeable loops from the gcc test
>> suite.
>> 
>> Name   		Boost
>> Example1  	2.98X
>> Example8   	0.99X
>> Example10a 1.78X
>> Example10b 1.58X
>> Example11  	0.47X
>> Example12  	0.91X
>> Example24  	1.67X
>> Example25  	6.61X
>> 
>> Thanks,
>> Nadav
>> 
>> [1] - http://gcc.gnu.org/projects/tree-ssa/vectorization.html
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 
> -- 
> Hal Finkel
> Postdoctoral Appointee
> Leadership Computing Facility
> Argonne National Laboratory