[PATCH] D20376: Properly set CLI.NumFixedArgs for mem{set, move, cpy} builtins in SelectionDAG

[Justin Bogner via llvm-commits]

Krzysztof Parzyszek <kparzysz at codeaurora.org> writes:
> On 6/9/2016 2:02 PM, Justin Bogner wrote:
>> Krzysztof Parzyszek via llvm-commits <llvm-commits at lists.llvm.org>
>> writes:
>>>
>>> The NumFixedArgs will be set by the CLI::setCallee function, do not
>>> force it to be 0 for these builtins.
>>
>> What is the actual effect of this change? There are a lot of other
>> places that call setCallee in the same way, so shouldn't we update all
>> of them too?
>
> You are right---all of them should be updated.

Sure, but why isn't anything broken when we pass in 0 here? I'm a bit
surprised that changing these doesn't seem to affect any tests or
anything.

> Setting the number of fixed arguments to something different from the
> size of Args never makes sense, except for vararg functions.  For
> non-vararg function we shouldn't pass this argument at all.
>
>> In fact, I'm not really convinced that we need to pass this argument
>> here at all - When don't we want Args.size()?
>
> That's pretty much what this patch does, but seemingly in an
> insufficient number of places.

What I mean is that I don't think this overload of setCallee is ever
actually used for vararg functions - they seem to all use the overload
with a FunctionType. I think we can get away with dropping the FixedArgs
from the signature of setCallee completely, which will make it that much
harder to misuse.

Could you look into that?

>
> -Krzysztof