Request for merge: GHC/ARM calling convention.

Hi Duncan,

> const unsigned*
> ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
> + bool ghcCall = false;
> +
> + if (MF) {
> + const Function *F = MF->getFunction();
> + ghcCall = (F ? F->getCallingConv() == CallingConv::GHC : false);
> + }

> This bit looks dubious. Why do you need to do it?

What exactly? We need to test if this is GHC calling convention or not and if its, then return different set of callee saved regs. What exactly don't you like on this piece?

Thanks!
Karel

Hi Karel,

> const unsigned*
> ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
> + bool ghcCall = false;
> +
> + if (MF) {
> + const Function *F = MF->getFunction();
> + ghcCall = (F ? F->getCallingConv() == CallingConv::GHC : false);
> + }

> This bit looks dubious. Why do you need to do it?

What exactly? We need to test if this is GHC calling convention or not and if
its, then return different set of callee saved regs. What exactly don't you like
on this piece?

I don't see anyone else rummaging around inside the original function for the
calling convention, so why do you need to? If this information is useful to
have, why isn't it in the machine function?

Ciao, Duncan.

Hi Duncan,

Hi Karel,

> const unsigned*
> ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF)
const {
> + bool ghcCall = false;
> +
> + if (MF) {
> + const Function *F = MF->getFunction();
> + ghcCall = (F ? F->getCallingConv() == CallingConv::GHC : false);
> + }

> This bit looks dubious. Why do you need to do it?

What exactly? We need to test if this is GHC calling convention or not
and if
its, then return different set of callee saved regs. What exactly
don't you like
on this piece?

I don't see anyone else rummaging around inside the original function
for the
calling convention, so why do you need to?

Based on knowledge of calling convention we return different set of callee saved registers. For example on ARM and when GHC CC is used, we return empty set.

If this information is useful to
have, why isn't it in the machine function?

Honestly speaking, I don't know. The code in question is not written by me, but IMHO the motivation was just to get job done and allow later someone with better knowledge of LLVM internals to do required refactoring -- if you are pointing on the fact why we have not refactored calling convention call from Function to MachineFuction yet.

Thanks,
Karel

Hi Karel,

> const unsigned*
> ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF)
const {
> + bool ghcCall = false;
> +
> + if (MF) {
> + const Function *F = MF->getFunction();
> + ghcCall = (F ? F->getCallingConv() == CallingConv::GHC : false);
> + }

> This bit looks dubious. Why do you need to do it?

What exactly? We need to test if this is GHC calling convention or not
and if
its, then return different set of callee saved regs. What exactly
don't you like
on this piece?

I don't see anyone else rummaging around inside the original function
for the
calling convention, so why do you need to?

Based on knowledge of calling convention we return different set of callee saved
registers. For example on ARM and when GHC CC is used, we return empty set.

yeah, but here MF is (presumably) the callee. The callee shouldn't be relevant
to how arguments are marshalled at the call-site. Especially as there might not
even be a callee function (case of an indirect call). Since calls themselves
are annotated with the calling convention to use, I guess the calling convention
should be grabbed from the call instruction itself somehow.

If this information is useful to
have, why isn't it in the machine function?

Honestly speaking, I don't know. The code in question is not written by me, but
IMHO the motivation was just to get job done and allow later someone with better
knowledge of LLVM internals to do required refactoring -- if you are pointing on
the fact why we have not refactored calling convention call from Function to
MachineFuction yet.

If it's because MF is represents the callee then it hasn't been done because
using it would almost certainly be wrong I guess :frowning:

Ciao, Duncan.

Hi Duncan, Karel,

Hi Karel,

> const unsigned*
> ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF)
const {
> + bool ghcCall = false;
> +
> + if (MF) {
> + const Function *F = MF->getFunction();
> + ghcCall = (F ? F->getCallingConv() == CallingConv::GHC : false);
> + }

> This bit looks dubious. Why do you need to do it?

What exactly? We need to test if this is GHC calling convention or not
and if
its, then return different set of callee saved regs. What exactly
don't you like
on this piece?

I don't see anyone else rummaging around inside the original function
for the
calling convention, so why do you need to?

So I wrote the first version of the GHC calling convention for X86,
and also basically the code in question above as its copied from the
X86 implementation. I had a look over the code and will try to explain
what I believe is going on but I may be mistaken as this is code I
haven't looked at for over a year.

The code above is needed as the GHC calling convention redefines what
registers are considered callee save. No one else rummages in to the
original function as all the other calling conventions use the same
set of callee and caller save registers, so GHC is the only one that
needs to differentiate.

Based on knowledge of calling convention we return different set of callee saved
registers. For example on ARM and when GHC CC is used, we return empty set.

yeah, but here MF is (presumably) the callee. The callee shouldn't be relevant
to how arguments are marshalled at the call-site. Especially as there might not
even be a callee function (case of an indirect call). Since calls themselves
are annotated with the calling convention to use, I guess the calling convention
should be grabbed from the call instruction itself somehow.

This isn't about the call-site. My understanding is that when the
prologue / epilogue code for a function (MF) is being generated this
'getCalleeSavedRegs' function is consulted to determine what to push
and pop. So yes MF is callee but I don't think this function is
affecting the call site marshaling. One thing to note is that the GHC
calling convention is intended to be used as in a tail call
optimization fashion (the code GHC generates with LLVM never returns,
its CPS code) so maybe this is complicating matters a little. We don't
really have 'call sites' using the GHC call convention since they're
all tail calls that are jumps and will never return.

Cheers,
David

TargetRegisterInfo::getCalleeSavedRegs() is called with a pointer to the machine function currently being compiled.

It is used by PEI to determine the registers to spill in the prolog, and it is used by RegisterClassInfo to rearrange allocation orders so the register allocator will use volatile registers before callee-saved registers.

Its behavior depends on the calling convention of the function being compiled. It is not related to call instructions in the function.

/jakob

Hi David,

The code above is needed as the GHC calling convention redefines what
registers are considered callee save. No one else rummages in to the
original function as all the other calling conventions use the same
set of callee and caller save registers, so GHC is the only one that
needs to differentiate.

shouldn't the caller also know what registers are callee saved so that it
can avoid saving the contents of those registers to the stack?

Based on knowledge of calling convention we return different set of callee saved
registers. For example on ARM and when GHC CC is used, we return empty set.

yeah, but here MF is (presumably) the callee. The callee shouldn't be relevant
to how arguments are marshalled at the call-site. Especially as there might not
even be a callee function (case of an indirect call). Since calls themselves
are annotated with the calling convention to use, I guess the calling convention
should be grabbed from the call instruction itself somehow.

This isn't about the call-site.

I see, thanks for explaining. But then can the MachineFunction really be null?

Ciao, Duncan.

Hi David,

The code above is needed as the GHC calling convention redefines what
registers are considered callee save. No one else rummages in to the
original function as all the other calling conventions use the same
set of callee and caller save registers, so GHC is the only one that
needs to differentiate.

shouldn't the caller also know what registers are callee saved so that it
can avoid saving the contents of those registers to the stack?

I assume so, I'm not an expert on LLVM or this area at all really.
What's the follow on logic / question here?

Based on knowledge of calling convention we return different set of
callee saved
registers. For example on ARM and when GHC CC is used, we return empty
set.

yeah, but here MF is (presumably) the callee. The callee shouldn't be
relevant
to how arguments are marshalled at the call-site. Especially as there
might not
even be a callee function (case of an indirect call). Since calls
themselves
are annotated with the calling convention to use, I guess the calling
convention
should be grabbed from the call instruction itself somehow.

This isn't about the call-site.

I see, thanks for explaining. But then can the MachineFunction really be
null?

Yes. For the X86 equiv of this code (X86RegisterInfo.cpp) the 'if
(MF)' lines already existed before my patch. So I assume the existing
code was correct. A quick grep for calls to 'getCalleeSavedRegs' also
shows some calls in some optimisation passes that don't pass in an
argument and the default argument is null.

Hope this all helps. Is there someone who 'owns' this section of the
code that we can rope into the discussion though as I'm not that
qualified.

Cheers,
David

Hi Duncan,

Any word on this making 3.0?

Cheers,
David

Hi David,

Any word on this making 3.0?

3.0 already branched, and since this is not a regression, this will
most probably go into 3.1.

Maybe Bill (CC'ed) being the release manager has other opinion on this.

Here's one GHC user indicating interest..

Marcus

Hello Bill,

could you be so kind and reconsider merge of GHC/ARM calling convention patch which is submitted here: http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-October/044173.html

I'm just patch submitter and little tweaker. Original patch was developed by David Terei for x86 and ported/retargeted for ARM by Stephen Blackheath. We (at least Stephen and me) are using it happily with GHC HEAD to compile Haskell code for ARM (and also on ARM).

I've submitted the patch last possible day for inclusion in LLVM 3.0 release in a hope it's simple enough for code review and inclusion. Unfortunately the patch raised some question by Duncan which I've not been able to explain fully. Later David Terei stepped in and explained everything as I understand the email conversation.

As this patch is simple enough and should not affect LLVM code generation when not used in cooperation with GHC also it changes just one LLVM architecture (ARM), I still hope it might be merged for 3.0 release.

If it adds to motivation: GHC HEAD already contains all needed support for it so if the patch is merged it'll easy life of GHC 7.4.x (future) release users on ARM hardware.

Thanks again for consideration!
Karel