Re: x86-64 support for gas part I

[Jan Hubicka <jh at suse dot cz>]

> On Sat, 16 Dec 2000, Jan Hubicka wrote:
> 
> > It seems to be quite hard to break it into smaller pieces, since majority of
> > changes is concerning adding new types Imm32S, Imm64, Disp32S and Disp32 that
> > are somehow unbreakable.
> 
> This piece can't go in by itself though, can it?
> 
> > Only drawback I am aware of is that the gas loses ability to print sane error
> > message when instruction is unsupported on given CPU.  I am abusing the
> > cpu_flag to determine whether instructions is supported in the 32bit or 64bit
> > mode, in order to avoid new falg in the instruction template.  Some
> > instructions (such as mov) are just partly supported in one of the modes, so
> > the test is not so easy - you need to pass the alternatives.
> 
> You should be able to keep this functionality, even with your "abuse" of
> cpu_flag.  Just mask off the "abused" bits when testing for cpu
> compatibility.  You also might be able to reduce the number of flag bits
Hmm, ok - assuming that we keep the constraint, that there are no isntruction
supported just partly by one CPU (this means that some templates have CpuXXX
while others not).
I will bring the check back with masking.
> needed with a slight redesign.  eg. The current code uses Disp16 and Disp32
> when there is really only a need for one flag, DispWord say, where the
> flag selects a word displacement and the size of the word can be found by
> testing flag_16bit_code and i.prefix[DATA_PREFIX].  Something could be
I was thinking about this too, but I would rather implement this after the
merger to avoid size of the pending patch.
> 
> This makes assumptions about the size of int.  What if int is 64 bits?
I tought there is upper limit on sizeof(int) prohibiting this.
> Better is
>   return ((offsetT) -1 << 31) <= n && n <= 0x7fffffff;
I was using this test, but it was producing warning concerning the
signetness of 0x7fffffff constant.  OK - I will bring it back.
> 
> > ! set_code_flag (value)
> > !      int  value;
> >   {
> > !   flag_code = value;
> > !   cpu_arch_flags &= ~(Cpu64 | CpuNo64);
> > !   cpu_arch_flags |= (flag_code == CODE_64BIT ? Cpu64 : CpuNo64);
> > !   if (value == CODE_64BIT && !(cpu_arch_flags & CpuSledgehammer))
> > !     {
> > !       as_bad (_("64bit mode not supported on this CPU."));
> > !     }
> > !   if (value == CODE_32BIT && !(cpu_arch_flags & Cpu386))
> > !     {
> > !       as_bad (_("64bit mode not supported on this CPU."));
> 
> 32 bit mode ?
Duh :)
> 
> > ! 		    if ((i.types[op] & Imm32)
> > ! 			&& (i.op[op].imms->X_add_number & ~(offsetT)0xffffffffU) == 0)
> > ! 		      {
> > ! 			i.op[op].imms->X_add_number =
> > ! 			  (((i.op[op].imms->X_add_number & 0xffffffffU) ^ 0x80000000) - 0x80000000);
> 
> You can't use a `u' suffix.  This code should compile with a K&R compiler.
> I suggest `(offsetT) -2 << 31' instead of `~(offsetT)0xffffffffU',
> `(((offsetT) 2 << 31) - 1)' instead of `0xffffffffU', and
> `((offsetT) 1 << 31)' instead of `0x80000000'.
> This needs to be fixed elsewhere in your changes too.
> 
> Constants in the range 0x80000000 to 0xffffffff need to be viewed with
> suspicion when writing code that behaves correctly both with old
> compilers and with ISO C compilers as the rules have changed.  eg.
>   sixtyfour_bit_int x = 0x80000000;
> gives x == 0xffffffff80000000 on an old compiler, x == 0x0000000080000000
> on an ISO C compiler.
I know, somehow I forgot to clean up these bits. SOrry.
> 
> I'm also being a little paranoid with shift counts, preferring
> `(offsetT) -2 << 31' to `(offsetT) -1 << 32' in case offsetT is 32 bits.
Right.
> 
> > + 				i.types[op] &= ~Disp;
> > + 				i.types[op] = Disp32S;
> 
> `|=' ?
> 
> > ! 			    if (flag_code != CODE_64BIT)
> > ! 			      i.types[op] = Disp32;	/* Must be 32 bit */
> 
> Same here.
> 
> > + 			i.types[op] &= ~Disp;
> > + 			i.types[op] = Disp32S;
> 
> And here.
Probably the = Disp32S is correct - I really want exactly this type.  I will kill
the &= instead.
> 
> > + 			if (flag_code == CODE_64BIT
> > + 			    && (i.types[op] & Disp))
> > + 			  {
> > + 			    if (i.types[op] & Disp8)
> > + 			      i.types[op] = Disp8 | Disp32S;
> > + 			    else
> > + 			      i.types[op] = Disp32S;
> > + 			  }
> 
> Do you want to lose all the other i.types[op] flags here?
> 
> >   #if ((defined (OBJ_MAYBE_ELF) && defined (OBJ_MAYBE_COFF)) \
> >        || (defined (OBJ_MAYBE_ELF) && defined (OBJ_MAYBE_AOUT)) \
> > !      || (defined (OBJ_MAYBE_COFF) && defined (OBJ_MAYBE_AOUT))) \
> > !      || defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
> 
> Simplify.  ie. remove the terms on the first two lines.
> Same thing in tc-i386.h.  Would it make sense to use
> i.prefixes[REX_BYTE] instead of i.rex.byte?
I am not sure - the behaviour of REX prefix is somewhat different -
it is not just one prefix, but 16 ones and additionally it must
come last (i.e if it is followed by other prefix, it is ignored).
But looking at as i.prefixes are handled, it seems to be sane to
use it for REG_PREFIX as well - I will do the change.

Thanks for review!
Honza
> 
> Regards, Alan Modra
> -- 
> Linuxcare.  Support for the Revolution.