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

[Krzysztof Parzyszek via llvm-commits]

On 6/9/2016 3:16 PM, Justin Bogner wrote:
>
> 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.

I was working on some code in LowerCall (in an out-of-tree project) and 
my calling convention definition was using NumFixedParams to see if it 
needs to start storing arguments to the stack.  It hit a case with a 
call to memset that was automatically generated from a builtin.  Since 
the NumFixedParams was set to 0, all arguments (incorrectly) ended up on 
the stack.  This is how I found this issue.

The allocation routine was like this:
   if (#of current argument < NumFixedParams && still have registers)
     allocate next register for the argument
   else
     put it on the stack

None of the existing code does it this way.  The concept of the argument 
index from the source code no longer exists in an obvious way in the DAG 
(large arguments get broken up into smaller pieces), so targets usually 
handle it in different ways (most use NumFixedParams only for vararg 
functions).


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

Ah, I see.  Sure, that makes sense.


-Krzysztof


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation