[llvm] r191122 - LoopVectorizer: Only allow vectorization of intrinsics. We can't know for sure that the functions 'abs' or 'round' are the functions from libm.

[Nadav Rotem]

Hi Arnold, Ben, Hal, 

Sorry for the delay in response. Thanks for reviewing my commit. I wanted to fix a case where we crash on a local function with the same name as one of the functions that we vectorize, but with different parameters. I understand that disabling all non-intrinsic functions is too restrictive.  I like Arnold’s solution of checking for internal linkage and validating the parameters. The patch looks excellent. 

Thanks,
Nadav


On Sep 21, 2013, at 4:19 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
>> 
>> On Sep 21, 2013, at 5:41 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>> 
>>> Arnold,
>>> 
>>> Thanks for looking at this quickly.
>>> 
>>> +static Intrinsic::ID checkBinaryFloatSignature(const CallInst &I,
>>> +                                               Intrinsic::ID
>>> ValidIntrinsicID) {
>>> +  if (I.getNumArgOperands() != 2 ||
>>> +      !I.getArgOperand(0)->getType()->isFloatingPointTy() ||
>>> +      !I.getArgOperand(1)->getType()->isFloatingPointTy() ||
>>> +      I.getType() != I.getArgOperand(0)->getType() ||
>>> +      !I.onlyReadsMemory())
>>> +    return Intrinsic::not_intrinsic;
>>> +
>>> +  return ValidIntrinsicID;
>>> +}
>>> 
>>> To be pedantic, we should check that the two arguments also have
>>> the same type (to prevent grabbing copysign(float, double)).
>> 
>> Right.
>> 
>> Now with
>> 
>> +static Intrinsic::ID checkBinaryFloatSignature(const CallInst &I,
>> +                                               Intrinsic::ID
>> ValidIntrinsicID)
>> +  if (I.getNumArgOperands() != 2 ||
>> +      !I.getArgOperand(0)->getType()->isFloatingPointTy() ||
>> +      !I.getArgOperand(1)->getType()->isFloatingPointTy() ||
>> +      I.getType() != I.getArgOperand(0)->getType() ||
>> +      I.getType() != I.getArgOperand(1)->getType() ||
>> +      !I.onlyReadsMemory())
>> 
> 
> You might be waiting for Nadav to approve, but LGTM, thanks!
> 
> -Hal
> 
>> 
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

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