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

[Ivan Krasin]

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
>