[PATCH] Enable vararg support for intrinsics

Juergen Ributzka juergen at apple.com
Thu Aug 29 10:02:53 PDT 2013


Sure, sounds good.

Cheers,
Juergen

On Aug 28, 2013, at 3:54 PM, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:

>>> Please clang-format the patch. Please also include a test of the
>>> verifier changes.
>> 
>> Lang is working on a new intrinsic that would require vararg support, so it will be used soon. I will add a test case for the verifier, but it will be limited for now. I can only verify the changes against existing intrinsics and they are not varargs. Once Lang adds his new intrinsic I can extend the verifier test case.
> 
> Would it be OK to wait for that intrinsic and commit one patch right
> after the other? That way you should be able to add a testcase.
> 
>>> 
>>> +/// VerifyIntrinsicIsVarArg - Verify if the intrinsic has variable arguments.
>>> 
>>> Please don't repeat the name of the function. Starting it with a
>>> lowercase letter would probably also be better.
>>> 
>> 
>> I was following the coding/commenting style of the surrounding code. Do you still want me to change it in that case?
> 
> It is fuzzy. I tend to like following the style guide a bit more
> strictly, but it is OK if you want to keep the old naming until
> someone changes more of this code.
> 
> Cheers,
> Rafael





More information about the llvm-commits mailing list