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

[Ivan Krasin]

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