[llvm-dev] Suggestion / Help regarding new calling convention

[vivek pandya via llvm-dev]

On Sat, Jun 25, 2016 at 4:06 AM, Matthias Braun <matze at braunis.de> wrote:

>
> On Jun 24, 2016, at 11:35 AM, vivek pandya <vivekvpandya at gmail.com> wrote:
>
>
>
> On Tue, Jun 21, 2016 at 12:31 AM, Matthias Braun <matze at braunis.de> wrote:
>
>> I just discussed this with vivek on IRC (and I think we agreed on this):
>>
>> Let me first state the motivation clearly to ease later discussions:
>> As far as the motivation for this change goes: Changing the calling
>> convention allows us to choose whether a register is saved by the callee or
>> the caller. Usually it is best to have a mix of both as too many caller
>> saved registers leads to unnecessary save/restores when the called function
>> turns out to only touch a fraction of the registers (as is typically for
>> smaller leaf-functions of the call graph). While too many callee saved
>> registers may lead to unnecessary saves/restores of registers even though
>> the calling function didn't have a live value in the register anyway. With
>> IPRA the first problem is mitigated since we propagate the actually
>> clobbered set of registers up the callgraph instead of relying on
>> conventions, so it is best to aim for more caller saved registers (though
>> we should check for code size increases and store/restore code being
>> potentially less good than the tuned sequences generated during
>> FrameLowering).
>>
>> To the disucssion at hand:
>> - Introducing a new calling convention at the IR level is the wrong
>> approach: The calling convention is mostly a contract when calling and
>> being called across translation unit boundaries. The details about how this
>> contract is fulfilled are part of CodeGen IMO but do not need to be visible
>> at the IR level.
>> - The only thing we want to influence here is which registers are saved
>> by the callee. Changing TargetFrameLowering::determineCalleeSaves() is a
>> good place to achieve this without affecting unrelated things like
>> parameter and return value handling which would be part of the calling
>> convention.
>>
> Hello Matthias,
>
> As per our discussion, the above trick will make sure that there is no
> callee saved registers and also we have thought that
> RegUsageInfoCalculator.cpp is having regmask that will make caller to save
> restore registers if both callee and caller is using any common register
> but this would require following change in RegUsageInfoCalculator.cpp :
>
> if (!F->hasLocalLinkage() || F->hasAddressTaken()) {
>       const uint32_t *CallPreservedMask =
>         TRI->getCallPreservedMask(MF, MF.getFunction()->getCallingConv());
>       // Set callee saved register as preserved.
>       for (unsigned i = 0; i < RegMaskSize; ++i)
>         RegMask[i] = RegMask[i] | CallPreservedMask[i];
>   }
>
> because RegUsageInfoCalculator.cpp marks register as preserved if MF's CC
> preserves it. But While optimizing for callee saved register we need to
> skip above code so that register save/restore code is adder around call
> site.
>
> Indeed some adjustment there should improve your results.
>
I think before improvement I need to get this functionally correct while
 local functions are optimized not to have callee saved registers, caller
is not saving appropriate registers as
TargetFrameLowering::determineCalleeSaves()
just effects callee saved register. RegMask needs to be updated to reflect
change too.
-Vivek

> I would however recommend to use something like this to determine which
> registers got saved:
>
> const MachineFrameInfo &MFI = *MF.getFrameInfo();
> assert(MFI.isCalleeSavedInfoValid());
> for (const CalleeSavedInfo &Info : MFI.getCalleeSavedInfo()) {
>    // Mark Info.getReg() and all its subregisters as preserved here!
> }
>
> - Matthias
>
>
> Apart from that my hunch is that IPO inlining of static function also
> creates problem for this ( I have this feeling because some test case from
> test-suite fails when using -O > 0 ). I am still working on this.
>
> Please share your thoughts on this.
>
> Sincerely,
> - Vivek
>
>
>> - We could experiment with dynamically changing the number of caller
>> saved registers in the future. I could imagine heuristics like functions
>> called from many places using some callee saved registers in order to avoid
>> code size increases because of extra spills/restores at all the call sites.
>> We can hardly create new calling conventions for these combinations of
>> marking registers as callee/caller saved.
>>
>> - Matthias
>>
>> On Jun 20, 2016, at 7:39 AM, vivek pandya via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>> Dear Community,
>>
>> To improve current interprocedural register allocation (IPRA) , we have
>> planned to set callee saved registers to none for local functions,
>> currently I am doing it in following way:
>>
>> if (F->hasLocalLinkage()  && !F->hasAddressTaken()) {
>>    DEBUG(dbgs() << "Function has LocalLinkage \n");
>> F->setCallingConv(CallingConv::GHC);
>>  }
>>
>> but we think threre should be clean and properway to do this perhaps like:
>>
>> if (F->hasLocalLinkage() && !F->hasAddressTaken()) {
>>     DEBUG(dbgs() << "Function has LocalLinkage \n");
>>     F->setCallingConv(CallingConv::NO_Callee_Saved);
>>   }
>>
>> So I would like to know any better suggestions and if it is better to add
>> a new CC for this purpose then what aspects should be considered while
>> defining a new CC. Actually in this case the new CC does not really
>> required to define how parameters should be passed or any special rule for
>> return value etc , it just required to set callee saved registers to be
>> none. So what are the minimal things required to define such a CC?
>>
>> Other alternative that I have thought was to add new attribute for
>> function and use it like following in
>> TargetFrameLowering::determineCalleeSaves()
>>
>>  // In Naked functions we aren't going to save any registers.
>>   if (MF.getFunction()->hasFnAttribute(Attribute::Naked))
>>     return;
>>
>> Any suggestions / thoughts are welcomed !
>>
>> Sincerely,
>> Vivek
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160625/0d62268e/attachment-0001.html>