llvm dev - Apr 2012 - [LLVMdev] [llvm-commits] [cfe-commits] [Patch?] Fix handling of ARM homogenous aggregates

On Wednesday 04 Apr 2012 12:41:49 Patrik Hägglund H
wrote:
> Hi Tim,
> 
> > So I've come to the conclusion that the real flaw is LLVM
> > not exposing enough information to the target-dependent
> > backend code for it to do the right thing.
> 
> We also had this problem. You might find this patch useful as a starting
> point: http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-March/048266.html
Thanks. I'd considered using MachineFunction fiddling purely from a 
LowerFormalArguments perspective (I hadnn't noticed the subtlety that 
LowerCall needs this info to be passed in).

Doesn't this mean you have to replicate all the machinations of 
SelectionDAGBuilder to work out which argument you're dealing with at any 
given moment, though? I'm thinking of how it splits structs and implicitly 
adds sret parameters in particular, though there may be more I don't know
of.

How does this information get handled by the TableGen calling-conv code in 
your situation? The only way I can think of is a custom CCState which gets 
told about each argument as it passes by and allows CCCustom functions to 
access its special information (or, possibly, a CCIf with a cast).
    
    CCCustom<"TellCCStateAboutArg">,
    [...]
    CCIf<"cast<MyCCState>(State).isPointerArg()">,
CCAssignToReg<[P1, P2]>>,

Putting that information in the InputArg/OutputArg and incorporating it the 
CCAssignFn interface  allows a more straightforward implementation in the 
targets, in my view (for both our uses). It's also information that's
readily
available when InputArg/OutputArgs are being constructed. In your case:

    CCIf<"SourceTy->isPointerTy()", CCAssignToReg<[P1,
P2]>>;

I've got a patch which implements it for ARM and X86 (though not HFAs using 
the features yet, I'm still musing on the best interface to present there --
"HFA* byval" for target simplicity or "HFA" for user
simplicity), I'll see if
I can clean it up for other targets and send it for comparison.

The main issue with my approach is that split struct args are still tricky: 
they get identical types and another custom CCState is needed to handle them 
en-masse (to find out where we are in the struct). Optimal for that case might 
be an extra flag similar to isSplit(), but for structs.

Thoughts?

Tim.

> Doesn't this mean you have to replicate all the machinations of 
> SelectionDAGBuilder to work out which argument you're dealing with at
any
> given moment, though?
We have some logic for matching flattened arguments back to original arguments.
> How does this information get handled by the TableGen calling-conv code in 
> your situation?
We only use CCCustom for our target.

/Patrik Hägglund

-----Original Message-----
From: Tim Northover [mailto:Tim.Northover at arm.com] 
Sent: den 4 april 2012 14:27
To: llvmdev at cs.uiuc.edu
Cc: Patrik Hägglund H; Bob Wilson; Anton Korobeynikov; James Molloy; cfe-commits
Subject: Re: [LLVMdev] [cfe-commits] Fix handling of ARM homogenous aggregates

On Wednesday 04 Apr 2012 12:41:49 Patrik Hägglund H
wrote:
> Hi Tim,
> 
> > So I've come to the conclusion that the real flaw is LLVM
> > not exposing enough information to the target-dependent
> > backend code for it to do the right thing.
> 
> We also had this problem. You might find this patch useful as a starting
> point: http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-March/048266.html
Thanks. I'd considered using MachineFunction fiddling purely from a 
LowerFormalArguments perspective (I hadnn't noticed the subtlety that 
LowerCall needs this info to be passed in).

Doesn't this mean you have to replicate all the machinations of 
SelectionDAGBuilder to work out which argument you're dealing with at any 
given moment, though? I'm thinking of how it splits structs and implicitly 
adds sret parameters in particular, though there may be more I don't know
of.

How does this information get handled by the TableGen calling-conv code in 
your situation? The only way I can think of is a custom CCState which gets 
told about each argument as it passes by and allows CCCustom functions to 
access its special information (or, possibly, a CCIf with a cast).
    
    CCCustom<"TellCCStateAboutArg">,
    [...]
    CCIf<"cast<MyCCState>(State).isPointerArg()">,
CCAssignToReg<[P1, P2]>>,

Putting that information in the InputArg/OutputArg and incorporating it the 
CCAssignFn interface  allows a more straightforward implementation in the 
targets, in my view (for both our uses). It's also information that's
readily
available when InputArg/OutputArgs are being constructed. In your case:

    CCIf<"SourceTy->isPointerTy()", CCAssignToReg<[P1,
P2]>>;

I've got a patch which implements it for ARM and X86 (though not HFAs using 
the features yet, I'm still musing on the best interface to present there --
"HFA* byval" for target simplicity or "HFA" for user
simplicity), I'll see if
I can clean it up for other targets and send it for comparison.

The main issue with my approach is that split struct args are still tricky: 
they get identical types and another custom CCState is needed to handle them 
en-masse (to find out where we are in the struct). Optimal for that case might 
be an extra flag similar to isSplit(), but for structs.

Thoughts?

Tim.

On Wednesday 04 Apr 2012 13:27:07 Tim Northover wrote:
> Putting that information in the InputArg/OutputArg and incorporating it the
> CCAssignFn interface  allows a more straightforward implementation in the
> targets, in my view (for both our uses). It's also information
that's
> readily available when InputArg/OutputArgs are being constructed. In your
> case:
> 
>     CCIf<"SourceTy->isPointerTy()", CCAssignToReg<[P1,
P2]>>;
> 
> I've got a patch which implements it for ARM and X86 (though not HFAs
using
> the features yet, I'm still musing on the best interface to present
there
> -- "HFA* byval" for target simplicity or "HFA" for user
simplicity), I'll
> see if I can clean it up for other targets and send it for comparison.
Here's the patch I was talking about. It's probably not quite ready for 
committing yet, but I think it's a step towards supporting the HFA ABI
without
excessive work from the front-end or duplication of SelectionDAGBuilder code.

If this is the right approach, it still needs to be decided whether the extra 
complexity and inconsistency (with MIPS etc) of passing an argument via 
"{float, float}" vs "{float, float}* byval" is worth the
simplified interface
and possible extra optimisation opportunities.

Opinions, anyone? (Hint hint).

Tim.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: callingconv-sourcetype.diff
Type: text/x-patch
Size: 39781 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120405/b5329082/attachment.bin>

Tim,
> Opinions, anyone? (Hint hint).
I think here stuff should be thought of from different points. While
providing source type for argument might be beneficial, it might cause
moving the code from frontend to backend. Consider e.g. passing struct
by value including crazy padding inside. The ABI might specify that
padding should be removed and struct is passed field-by-field.

Also, note that in many cases the ABI rules are worded in terms of
source language which might now be preserved during IR generation,
so...

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University