[PATCH] Enable vararg support for intrinsics

Rafael EspĂ­ndola rafael.espindola at gmail.com
Wed Aug 28 15:54:13 PDT 2013


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