problem with 64-bit arch

problem with 64-bit arch

[Jean-Marc Saffroy]

Hi,

I am trying to use CGEN for an experimental 64-bit architecture (ie. 
word-bitsize is 64), and in my case the generated arch-ibld.c file is 
incorrect.

I defined the following insn field:

(df f-disp24     "disp24"              (PCREL-ADDR) 8 24 INT
     ((value pc) (sra WI (sub WI value pc) (const 2)))
     ((value pc) (add WI (sll WI value (const 2)) pc))
)

(define-operand
   (name disp24)
   (comment "24 bit signed displacement")
   (type h-iaddr)
   (mode WI)
   (index f-disp24)
)

And arch-ibld.c contains the following code (in <arch>_cgen_insert_operand):

   switch (opindex)
     {
     case <ARCH>_OPERAND_DISP24 :
       {
         long value = fields->f_disp24;
         value = (() (((value) - (pc))) >> (2));
         errmsg = insert_normal (cd, value,
0|(1<<CGEN_IFLD_SIGNED)|(1<<CGEN_IFLD_PCREL_ADDR), 0, 8, 24, 32,
total_length, buffer);
       }
       break;

Note the missing type in the cast before assigning to value. Also using 
a long type for value will cause problems when running on a 32-bit host, 
since it operates on 64-bit values.

I had a quick look at the other two archs that use word-bitsize 64, ie. 
sparc64 and ia64, and it seems they use multi-ifields for this kind of 
PC-relative field, which may be an artificial way of avoiding the problem.

Is there an obvious fix to this problem?

Cheers,
JM

Re: problem with 64-bit arch

[Doug Evans]

Jean-Marc Saffroy wrote:
> Hi,
>
> I am trying to use CGEN for an experimental 64-bit architecture (ie. 
> word-bitsize is 64), and in my case the generated arch-ibld.c file is 
> incorrect.
>
> I defined the following insn field:
>
> (df f-disp24     "disp24"              (PCREL-ADDR) 8 24 INT
>     ((value pc) (sra WI (sub WI value pc) (const 2)))
>     ((value pc) (add WI (sll WI value (const 2)) pc))
> )
>
> (define-operand
>   (name disp24)
>   (comment "24 bit signed displacement")
>   (type h-iaddr)
>   (mode WI)
>   (index f-disp24)
> )
>
> And arch-ibld.c contains the following code (in 
> <arch>_cgen_insert_operand):
>
>   switch (opindex)
>     {
>     case <ARCH>_OPERAND_DISP24 :
>       {
>         long value = fields->f_disp24;
>         value = (() (((value) - (pc))) >> (2));
>         errmsg = insert_normal (cd, value,
> 0|(1<<CGEN_IFLD_SIGNED)|(1<<CGEN_IFLD_PCREL_ADDR), 0, 8, 24, 32,
> total_length, buffer);
>       }
>       break;
>
> Note the missing type in the cast before assigning to value. Also 
> using a long type for value will cause problems when running on a 
> 32-bit host, since it operates on 64-bit values.
>
> I had a quick look at the other two archs that use word-bitsize 64, 
> ie. sparc64 and ia64, and it seems they use multi-ifields for this 
> kind of PC-relative field, which may be an artificial way of avoiding 
> the problem.
>
> Is there an obvious fix to this problem?
>

Obvious?  Possibly.

For reference sake, there are several separate notions here: data word 
bitsize, and instruction word bitsize.
sparc64 has 64/32, ia64 has 64/64 [IIUC, and I'm leaving out "bundles", 
I don't know ia64 that well]

[For completeness sake, there's also data address and instruction 
address word bitsize.  I'm guessing they're 64 here, same as data word 
bitsize, so we can ignore them.]

We were just discussing instruction word bitsizes of 64, and it's not 
clear what to do.
I don't mind going down the path of having 64 bit instructions expressed 
as a single 64 bit integer (*1), but can you confirm your architecture 
has 64-bit instructions?  From the given sample, it seems like it has 32 
bit instructions and 64-bit words (like sparc64).

Clearly the missing type in the cast is a bug.  That should be easy to 
fix.  As for the use of long to record a 64-bit value, that should also 
be (relatively) easy to fix.
I'll dig deeper tonight.

(*1): It would foreclose using such ports on hosts that don't have 
64-ints, but I don't mind, personally.
[I wonder if someday we'll want to use SI/DI/etc. to represent 
instructions.  That's *way* down the road, if ever.]

Re: problem with 64-bit arch

[Jean-Marc Saffroy]

Doug Evans wrote:
> We were just discussing instruction word bitsizes of 64, and it's not 
> clear what to do.
> I don't mind going down the path of having 64 bit instructions expressed 
> as a single 64 bit integer (*1), but can you confirm your architecture 
> has 64-bit instructions?  From the given sample, it seems like it has 32 
> bit instructions and 64-bit words (like sparc64).

You guessed right: my target arch has 32 bit instructions (which I 
expressed with base-insn-bitsize == 32), and uses 64-bit words for data 
and addresses (expressed with word-bitsize == 64).

> Clearly the missing type in the cast is a bug.  That should be easy to 
> fix.  As for the use of long to record a 64-bit value, that should also 
> be (relatively) easy to fix.
> I'll dig deeper tonight.

Good to hear, thanks!

I found the cast could be fixed by changing the "(dfm 'DI..." line in 
mode.scm, but have no idea about the possible side effects, and didn't 
find a cure for the use of long anyway.


Cheers,
JM

Re: problem with 64-bit arch

[Doug Evans]

Jean-Marc Saffroy wrote:
> Doug Evans wrote:
>> We were just discussing instruction word bitsizes of 64, and it's not 
>> clear what to do.
>> I don't mind going down the path of having 64 bit instructions 
>> expressed as a single 64 bit integer (*1), but can you confirm your 
>> architecture has 64-bit instructions?  From the given sample, it 
>> seems like it has 32 bit instructions and 64-bit words (like sparc64).
>
> You guessed right: my target arch has 32 bit instructions (which I 
> expressed with base-insn-bitsize == 32), and uses 64-bit words for 
> data and addresses (expressed with word-bitsize == 64).
>
>> Clearly the missing type in the cast is a bug.  That should be easy 
>> to fix.  As for the use of long to record a 64-bit value, that should 
>> also be (relatively) easy to fix.
>> I'll dig deeper tonight.
>
> Good to hear, thanks!
>
> I found the cast could be fixed by changing the "(dfm 'DI..." line in 
> mode.scm, but have no idea about the possible side effects, and didn't 
> find a cure for the use of long anyway.
>
>
> Cheers,
> JM

Ok, I understand what's going on.  There's two separate issues.
Using a long to store a 64-bit value is a long outstanding "todo" item 
to fix in the opcodes support.  Working on a fix but I'm busy over the 
weekend so it'll be next week before the fix is checked in.
The cast bug is related to the above "todo" item in that the fix will 
touch the same areas (the rtl->c generator for shifts doesn't properly 
handle 64-bit values).

Blech, I wish I didn't have to use bfd for bfd_vma, bfd_uint64_t -like 
things, or reinvent all the work bfd has to put into those types.