[llvm-commits] X86 FastISel: Emit immediate call arguments locally to save stack size when compiling with -O0

[Eric Christopher]

:) Almost there.

Can you describe the full algorithm in the few lines before the declaration of LastLocalValue and EmitStartPt?

The rationale for the built-in is fine.

-eric

On Aug 18, 2011, at 1:12 PM, Ivan Krasin wrote:

> Friendly ping :)
> 
> On Tue, Aug 16, 2011 at 12:14 PM, Ivan Krasin <krasin at google.com> wrote:
>> Eric,
>> 
>> could you please take another look?
>> 
>> Ivan
>> 
>> On Mon, Aug 15, 2011 at 2:29 PM, Ivan Krasin <krasin at google.com> wrote:
>>> On Thu, Aug 11, 2011 at 3:20 PM, Eric Christopher <echristo at apple.com> wrote:
>>>> 
>>>> On Aug 10, 2011, at 4:01 PM, Ivan Krasin wrote:
>>>> 
>>>> <flush-local-value-map.patch>
>>>> 
>>>> In general I think the patch is OK.  A few requests:
>>>>    MachineInstr *LastLocalValue;
>>>> +  MachineInstr *OrigLocalValue;
>>>> ….
>>>>    // Start out as null, meaining no local-value instructions have
>>>>    // been emitted.
>>>> -  LastLocalValue = 0;
>>>> +  OrigLocalValue = 0;
>>>> A lot of the code you've changed hasn't had any comments written or updated
>>>> for the new behavior. It'd be good to get an updated description of how the
>>>> LocalValueMap is working and how it interacts with constants.
>>> I have renamed OrigLocalValue to EmitStartPt which is (I believe) less
>>> confusing naming, since it points to the place in the block where it's
>>> allowed to start emitting instructions.
>>> 
>>>> In particular:
>>>> +  if (!isa<IntrinsicInst>(F))
>>>> +    flushLocalValueMap();
>>>> +
>>>> Here. The location of this here doesn't make a whole lot of sense and it'd
>>>> be good if you could explain it.
>>> I've added the explanation. Thanks for the suggestion.
>>> 
>>>> A better way to do this would be, as Jakob suggested, use the LocalValueMap
>>>> as a storage for constants and locations that you've used them in the block
>>>> and then emit all of the constants at that point rather than this weird
>>>> flushing mechanism.
>>> I've tried to do that. It slows down -O0 build which is unacceptable.
>>> The problem is that we have to emit all local values to the start of
>>> the block, store last use for each of them and spread them through the
>>> block once the processing of the block is done. It makes it slower...
>>> So, I would prefer to stay with the patch that makes all the metrics
>>> slightly better on average (code size/stack size/compile time). Is it
>>> fine with you?
>>> 
>>> Please, find the updated patch attached.
>>> 
>>> Ivan Krasin
>>> 
>>>> -eric
>>> 
>>