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

[Tobias Grosser]

On 01/28/2012 04:20 PM, Hal Finkel wrote:
> 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?

I checked on Friday and at least for me it compiles fine. Also 'make 
check' finishes without errors.

Cheers
Tobi