Expanding a load instruction

Expanding a load instruction

[fearyourself]

Dear all,

In the instruction set of my architecture, the offsets of a half-load
(HImode) have to be multiples of 2. However, if I set up a structure
in a certain way, the compiler will generate:

(mem/s/j:HI (plus:DI (reg:DI 134 [ ivtmp.23 ])
        (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16])

As the memory operand for the load.

Now, one solution I am going to try to fix this is to use
define_expand and add a move into another register before this load
and then load from that register (thus removing the offset of 1).

My question is: Is that how it should be done or is there another solution?

Thanks again for your help,
Jc

Re: Expanding a load instruction

[Dave Korn]

fearyourself wrote:

> In the instruction set of my architecture, the offsets of a half-load
> (HImode) have to be multiples of 2. However, if I set up a structure
> in a certain way, the compiler will generate:
> 
> (mem/s/j:HI (plus:DI (reg:DI 134 [ ivtmp.23 ])
>         (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16])
> 
> As the memory operand for the load.
> 
> Now, one solution I am going to try to fix this is to use
> define_expand and add a move into another register before this load
> and then load from that register (thus removing the offset of 1).
> 
> My question is: Is that how it should be done or is there another solution?

  This looks like what you need:

 -- Macro: STRICT_ALIGNMENT
     Define this macro to be the value 1 if instructions will fail to
     work if given data not on the nominal alignment.  If instructions
     will merely go slower in that case, define this macro as 0.

    cheers,
      DaveK

Re: Expanding a load instruction

[Jean Christophe Beyler]

Dear all,

I've moved forward on this issue. Again, the problem is not that the
data is not aligned but that the compiler tries to generate this
instruction:

(set (reg:HI 141) (mem/s/j:HI (plus:DI (reg:DI 134 [ ivtmp.23 ])
         (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16]))

And, in my target architecture, this is not authorized. I want to
transform this into:

(set (reg:DI 135) (plus:DI (reg:DI 134 [ ivtmp.23 ])
         (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16]))
(set (reg:HI 141) (mem/s/j:HI (reg:DI 135))


I've tried playing with define_expand, I can detect this problem, I
can emit my first move and then the second but for some reason, the
compiler reassembles them back together.

Any ideas why I am getting that? Am I doing anything wrong? After I
expand this, I use the DONE macro...
Thanks,
Jc

On Fri, Jun 5, 2009 at 1:17 PM, Jean Christophe
Beyler<jean.christophe.beyler@gmail.com> wrote:
> It's already at 1 actually. The problem isn't really that the data
> itself is unaligned but that the offset of the load must be a power of
> 2 (multiple of 2 for a HI mode, multiple of 4 for a SI, ...).
>
> Therefore, that macro does not seem to apply to me (and it's already at 1!).
>
> Thanks, I'm continuing to look into this issue,
> Jc
>
> On Fri, Jun 5, 2009 at 11:48 AM, Dave Korn
> <dave.korn.cygwin@googlemail.com> wrote:
>> fearyourself wrote:
>>
>>> In the instruction set of my architecture, the offsets of a half-load
>>> (HImode) have to be multiples of 2. However, if I set up a structure
>>> in a certain way, the compiler will generate:
>>>
>>> (mem/s/j:HI (plus:DI (reg:DI 134 [ ivtmp.23 ])
>>>         (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16])
>>>
>>> As the memory operand for the load.
>>>
>>> Now, one solution I am going to try to fix this is to use
>>> define_expand and add a move into another register before this load
>>> and then load from that register (thus removing the offset of 1).
>>>
>>> My question is: Is that how it should be done or is there another solution?
>>
>>  This looks like what you need:
>>
>>  -- Macro: STRICT_ALIGNMENT
>>     Define this macro to be the value 1 if instructions will fail to
>>     work if given data not on the nominal alignment.  If instructions
>>     will merely go slower in that case, define this macro as 0.
>>
>>    cheers,
>>      DaveK
>>
>>
>

Re: Expanding a load instruction

[Dave Korn]

Jean Christophe Beyler wrote:
> Dear all,
> 
> I've moved forward on this issue. Again, the problem is not that the
> data is not aligned but that the compiler tries to generate this
> instruction:
> 
> (set (reg:HI 141) (mem/s/j:HI (plus:DI (reg:DI 134 [ ivtmp.23 ])
>          (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16]))
> 
> And, in my target architecture, this is not authorized. I want to
> transform this into:
> 
> (set (reg:DI 135) (plus:DI (reg:DI 134 [ ivtmp.23 ])
>          (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16]))
> (set (reg:HI 141) (mem/s/j:HI (reg:DI 135))
> 
> 
> I've tried playing with define_expand, I can detect this problem, I
> can emit my first move and then the second but for some reason, the
> compiler reassembles them back together.
> 
> Any ideas why I am getting that? Am I doing anything wrong? After I
> expand this, I use the DONE macro...


  How about forbidding the form you don't allow in GO_IF_LEGITIMATE_ADDRESS?

    cheers,
      DaveK

Re: Expanding a load instruction

[Jean Christophe Beyler]

I'll be looking into this but I thought that GO_IF_LEGITIMATE_ADDRESS
is for branches ?

This is not my case. I've simplified my test case into:

struct test {
    const char  *name;          /* full name */
    char        a;          /* symbol */
    signed char     b;
    unsigned short  c;          /* creation/c mask value */
};

extern struct test tab[];       /* the master list of tabter types */

void bar(unsigned short);

int main(void)
{
    int i;
    for (i=0;i<128;i++)
        if(tab[i].b)
            bar(tab[i].c);
}

The compiler loads tab[i].b and, to do so, loads up the address of &tab[i].b.

Then, when it wants to pass the value of tab[i].c it tries to do a
ldhu r5, 1(r10)

which is wrong in our case. Because it is loading a HI, the offset
must be a multiple of 2. It so happens that currently the problem is
arrising when considering this RTX

(insn:HI 77 76 124 struct2.c:17 (set (reg:DI 8 r8)
        (sign_extend:DI (mem:HI (plus:DI (reg:DI 48 r48 [orig:134
ivtmp.19 ] [134])
                    (const_int 1 [0x1])) [0 S2 A16]))) 61
{extendhidi2_cyc64} (nil))

Am I wrong to think that, in this case, GO_IF_LEGITIMATE_ADDRESS would
not help? On my architecture, we actually have a scratch register we
can use.

I'm thinking of using a define_peephole2 to correct this as a later
pass, taking the address and moving it into a register thus getting a
0 offset.

I just feel that is so ugly, there should be a way to tell GCC that a
mem cannot be defined with:
(mem:HI (plus:DI (reg:DI 48 r48 [orig:134 ivtmp.19 ] [134])
                    (const_int 1 [0x1])) [0 S2 A16]))) 61
{extendhidi2_cyc64} (nil))

in the case of a HI because of the constant (and then same for SI/DI of course).

Thanks again for your help, and I will continue looking into this,
Jc

On Tue, Jun 9, 2009 at 6:36 PM, Dave
Korn<dave.korn.cygwin@googlemail.com> wrote:
> Jean Christophe Beyler wrote:
>> Dear all,
>>
>> I've moved forward on this issue. Again, the problem is not that the
>> data is not aligned but that the compiler tries to generate this
>> instruction:
>>
>> (set (reg:HI 141) (mem/s/j:HI (plus:DI (reg:DI 134 [ ivtmp.23 ])
>>          (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16]))
>>
>> And, in my target architecture, this is not authorized. I want to
>> transform this into:
>>
>> (set (reg:DI 135) (plus:DI (reg:DI 134 [ ivtmp.23 ])
>>          (const_int 1 [0x1])) [0 <variable>.geno+0 S2 A16]))
>> (set (reg:HI 141) (mem/s/j:HI (reg:DI 135))
>>
>>
>> I've tried playing with define_expand, I can detect this problem, I
>> can emit my first move and then the second but for some reason, the
>> compiler reassembles them back together.
>>
>> Any ideas why I am getting that? Am I doing anything wrong? After I
>> expand this, I use the DONE macro...
>
>
>  How about forbidding the form you don't allow in GO_IF_LEGITIMATE_ADDRESS?
>
>    cheers,
>      DaveK
>

Re: Expanding a load instruction

[Dave Korn]

Jean Christophe Beyler wrote:
> I'll be looking into this but I thought that GO_IF_LEGITIMATE_ADDRESS
> is for branches ?

  No, absolutely not.  GILA is a general filter that has overall control over
which forms of addressing modes used to address memory may be generated in RTL.

http://gcc.gnu.org/onlinedocs/gccint/Addressing-Modes.html

  Hmm, I must have been looking at a slightly outdated build of the docs;
according to that page it's been deprecated on HEAD in favour of the hook
TARGET_LEGITIMATE_ADDRESS_P.  You'll need to check the particular sources
you're using, that must be a fairly recent change(*).


    cheers,
      DaveK
-- 
(*) - Yes: it was changed in r.147534, around four weeks ago.
http://gcc.gnu.org/viewcvs?view=rev&revision=147534

Re: Expanding a load instruction

[Jean Christophe Beyler]

Ok, I wrongly read what this macro did. Sorry about that. I was
looking at the i386 port and use of this variable and this code came
up:


#ifdef REG_OK_STRICT
#define GO_IF_LEGITIMATE_ADDRESS(MODE, X, ADDR)             \
do {                                    \
  if (legitimate_address_p ((MODE), (X), 1))                \
    goto ADDR;                              \
} while (0)

#else
#define GO_IF_LEGITIMATE_ADDRESS(MODE, X, ADDR)             \
do {                                    \
  if (legitimate_address_p ((MODE), (X), 0))                \
    goto ADDR;                              \
} while (0)

#endif



and :

#define LEGITIMIZE_ADDRESS(X, OLDX, MODE, WIN)              \
do {                                    \
  (X) = legitimize_address ((X), (OLDX), (MODE));           \
  if (memory_address_p ((MODE), (X)))                   \
    goto WIN;                               \
} while (0)


It seems that I should do the same as them no for my solution. First
implement the legitimate_address function and then probably define it
in both macros.

As for the target hook, we are using GCC 4.3.2 for the moment and,
sadly, have not yet any plans to move forward to the latest version of
GCC.

Thanks again,
Jc

On Wed, Jun 10, 2009 at 10:29 AM, Dave
Korn<dave.korn.cygwin@googlemail.com> wrote:
> Jean Christophe Beyler wrote:
>> I'll be looking into this but I thought that GO_IF_LEGITIMATE_ADDRESS
>> is for branches ?
>
>  No, absolutely not.  GILA is a general filter that has overall control over
> which forms of addressing modes used to address memory may be generated in RTL.
>
> http://gcc.gnu.org/onlinedocs/gccint/Addressing-Modes.html
>
>  Hmm, I must have been looking at a slightly outdated build of the docs;
> according to that page it's been deprecated on HEAD in favour of the hook
> TARGET_LEGITIMATE_ADDRESS_P.  You'll need to check the particular sources
> you're using, that must be a fairly recent change(*).
>
>
>    cheers,
>      DaveK
> --
> (*) - Yes: it was changed in r.147534, around four weeks ago.
> http://gcc.gnu.org/viewcvs?view=rev&revision=147534
>
>

Re: Expanding a load instruction

[Dave Korn]

Jean Christophe Beyler wrote:

> It seems that I should do the same as them no for my solution. First
> implement the legitimate_address function and then probably define it
> in both macros.

  Sounds about right.

> As for the target hook, we are using GCC 4.3.2 for the moment and,
> sadly, have not yet any plans to move forward to the latest version of
> GCC.

  Don't need to worry about it until you go to GCC 4.5.0 or above, then.

    cheers,
      DaveK

Re: Expanding a load instruction

[Jean Christophe Beyler]

Thanks, I've pretty much finished this part of the implementation and
it seems to be working well.

Thank you all very much for your help,
Jc

On Wed, Jun 10, 2009 at 11:12 AM, Dave
Korn<dave.korn.cygwin@googlemail.com> wrote:
> Jean Christophe Beyler wrote:
>
>> It seems that I should do the same as them no for my solution. First
>> implement the legitimate_address function and then probably define it
>> in both macros.
>
>  Sounds about right.
>
>> As for the target hook, we are using GCC 4.3.2 for the moment and,
>> sadly, have not yet any plans to move forward to the latest version of
>> GCC.
>
>  Don't need to worry about it until you go to GCC 4.5.0 or above, then.
>
>    cheers,
>      DaveK
>
>