[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]

Thanks :) 

On Sep 23, 2013, at 8:00 AM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:

> Committed as 191206
> 
> On Sep 23, 2013, at 1:03 AM, Nadav Rotem <nrotem at apple.com> wrote:
> 
>> 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
>> 
>