llvm dev - Aug 2007 - [LLVMdev] How to access llvm Types from the codegen?

On 27/07/07, Evan Cheng <evan.cheng at apple.com>
wrote:
> Code generator shouldn't introspect the LLVM type at all, except to
> get the size of the type.  Anything needed should be encoded by the
> front-end.  In the short term, please focus on getting parity with
> what we already have.  This means x86-64 will be wrong, but it
> already is.  As a second step we can then worry about x86-64-specific
> param attributes that should be added to handle this stuff correctly.
>
> Evan
Sorry for the delay. I am back from a vacation and have been busy
catching up....

That is why I need the types (just for the size). This is necessary to
implement both the current behavior and the correct ABI.

consider the llvm code
---------------------------------------
%struct.s = type { i64 }

define i64 @f(%struct.s* byval %a) {
entry:
        %tmp2 = getelementptr %struct.s* %a, i32 0, i32 0
 ; <i64*> [#uses=1]
        %tmp3 = load i64* %tmp2         ; <i64> [#uses=1]
        ret i64 %tmp3
---------------------------------------

When the DAG is constructed it will contain a load from formal_args.
It looks like a lot of work to modify the DAG so that instead it uses
a copy. Even if we decide to do this, we would need to pass the size
of the structure to the DAG, which currently is not available.

I propose that structures that are passed on registers should be slip
into many DAG level arguments (for now this would be all structs) and
the DAG should contain copies instead of loads. The nice thing about
this proposal is that for structures that are passed on the stack, the
DAG doesn't need to know the size. All that we need to add to add to
the DAG is a flag so that the code generators know that the struct
pointer should be computed based on the stack pointer.

Thoughts?

Thanks,
--
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047

> I propose that structures that are passed on registers should be slip
> into many DAG level arguments (for now this would be all structs) and
> the DAG should contain copies instead of loads. The nice thing about
> this proposal is that for structures that are passed on the stack, the
> DAG doesn't need to know the size. All that we need to add to add to
> the DAG is a flag so that the code generators know that the struct
> pointer should be computed based on the stack pointer.
Just noticed that we still need the size and the alignment of the
structure so that we determine the stack location of subsequent
arguments. So, any ideas on how to add that information to the DAG?

Cheers,
-- 
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047

> Just noticed that we still need the size and the alignment of the
> structure so that we determine the stack location of subsequent
> arguments. So, any ideas on how to add that information to the DAG?
I am trying to use the remaining bits of the flag nodes. Should be
sufficient to implement a first version.

Cheers,
-- 
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047

On Fri, 3 Aug 2007, Rafael Espindola wrote:
> Sorry for the delay. I am back from a vacation and have been busy
> catching up....
Welcome back :)
> That is why I need the types (just for the size). This is necessary to
> implement both the current behavior and the correct ABI.
Right.
> consider the llvm code
> ---------------------------------------
> %struct.s = type { i64 }
>
> define i64 @f(%struct.s* byval %a) {
> entry:
>        %tmp2 = getelementptr %struct.s* %a, i32 0, i32 0
> ; <i64*> [#uses=1]
>        %tmp3 = load i64* %tmp2         ; <i64> [#uses=1]
>        ret i64 %tmp3
> ---------------------------------------
>
> When the DAG is constructed it will contain a load from formal_args.
It shouldn't contain a load: the "a" argument (which represents
the
address by-val arg) will be one of the results of the formal_arguments 
node.  Ah, you probably mean the explicit load will turn into a load 
SDNode, which is right :)
> It looks like a lot of work to modify the DAG so that instead it uses
> a copy. Even if we decide to do this, we would need to pass the size
> of the structure to the DAG, which currently is not available.
Okay, you're saying that in this example, the "byval" argument is
supposed
to be passed in a register, which does not have an address.  Further, the 
explicit load ideally becomes dead.

There are multiple steps to get optimal code for x86-64.  In the short 
term, I think it makes the most sense for the code generator to lower this 
into a store onto a stack, and use the address of the stack slot as the 
"result" value of the formal_argument.  Thus you lower this
"dag":

   a,chain = formal_arguments (...)

into a 64-bit stack slot (e.g. frame index #1) and:

   chain = store %rax -> (frameindex #1)
   a = frameindex #1

This will leave the loads in the code (ugly codegen) but will work.

Going forward, we can optimize out this stuff by having the front-end 
generate a simple i64 argument instead of using byval in this case. 
However, it should always be *safe* to use byval, so the codegen needs to 
learn how to handle this.
> I propose that structures that are passed on registers should be slip
> into many DAG level arguments (for now this would be all structs) and
> the DAG should contain copies instead of loads. The nice thing about
> this proposal is that for structures that are passed on the stack, the
> DAG doesn't need to know the size. All that we need to add to add to
> the DAG is a flag so that the code generators know that the struct
> pointer should be computed based on the stack pointer.
I don't follow, can you explain more what you mean?

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/

> > ---------------------------------------
> > %struct.s = type { i64 }
> >
> > define i64 @f(%struct.s* byval %a) {
> > entry:
> >        %tmp2 = getelementptr %struct.s* %a, i32 0, i32 0
> > ; <i64*> [#uses=1]
> >        %tmp3 = load i64* %tmp2         ; <i64> [#uses=1]
> >        ret i64 %tmp3
> > ---------------------------------------
> >
> > When the DAG is constructed it will contain a load from formal_args.
>
> It shouldn't contain a load: the "a" argument (which
represents the
> address by-val arg) will be one of the results of the formal_arguments
> node.  Ah, you probably mean the explicit load will turn into a load
> SDNode, which is right :)
Yes. That is it :-)
> > It looks like a lot of work to modify the DAG so that instead it uses
> > a copy. Even if we decide to do this, we would need to pass the size
> > of the structure to the DAG, which currently is not available.
>
> Okay, you're saying that in this example, the "byval"
argument is supposed
> to be passed in a register, which does not have an address.  Further, the
> explicit load ideally becomes dead.
>
> There are multiple steps to get optimal code for x86-64.  In the short
> term, I think it makes the most sense for the code generator to lower this
> into a store onto a stack, and use the address of the stack slot as the
> "result" value of the formal_argument.  Thus you lower this
"dag":
>
>    a,chain = formal_arguments (...)
>
> into a 64-bit stack slot (e.g. frame index #1) and:
>
>    chain = store %rax -> (frameindex #1)
>    a = frameindex #1
>
> This will leave the loads in the code (ugly codegen) but will work.
Looks like a nice first implementation :-)

> Going forward, we can optimize out this stuff by having the front-end
> generate a simple i64 argument instead of using byval in this case.
> However, it should always be *safe* to use byval, so the codegen needs to
> learn how to handle this.
..... (some badly worded proposal)
> I don't follow, can you explain more what you mean?
The idea would be to define byval as being always on the stack. This
would make it wrong for the front end (on linux-x86_64) to lower

struct s {
long a;
};
void f(struct s a) {....}

into a f function with a byval argument. It would have to use a i64.
This would make the code gen easier. Given your suggestion of just
storing the incoming register, not much so. I don't have any strong
opinions on which semantics we give to the byval attribute, but I like
the always on the stack a little bit better.

> -Chris
Cheers,
-- 
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047