Home Original page

PATCH: Avoid signed/unsigned warnings in tc-arm.c

Nick Clifton nickc@redhat.com
Mon Jul 4 14:23:00 GMT 2005 
Hi Khem,

> Thanks for reviewing it. Here is renewed patch and Zack's suggestion on 
> changelog entry verbatim.

Sorry - but I still do not think that this patch is correct.  The 
problem is the choice of which things you chose to convert into signed 
values:

 >  static void
 >  do_shift (void)
 >  {
 > -  unsigned int Rm = (inst.operands[1].present
 > +  int Rm = (inst.operands[1].present
 >  		     ? inst.operands[1].reg
 >  		     : inst.operands[0].reg);

Rm here is a register number.  There are no negative register values. 
Hence it makes sense that it is an unsigned quantity.  I realise that 
this change is because you have made the "reg" field of the operands 
sub-structure a signed integer, but I feel that this too is wrong for 
the same reason.

Looking at the error messages generated by gcc 4.0 I realise that the 
problem is the comparison between the "imm" field and the "reg" field at 
the end of do_ldrd().  This happens because the "imm" field is being 
overloaded and used as a secondary "reg" field.  Ideally we would use a 
union of an signed and an unsigned integer to get around this, but that 
is getting needlessly complicated.

A cleaner approach in my opinion then is to allow the "imm" field to 
become a signed field as needed to pacify other warnings, but to make it 
explicitly signed not just "int" but "signed int".   Keep the "reg" 
field as unsigned and when the two fields are compared use casts to 
convert the overloaded "imm" field to an unsigned value.

ie - the attached patch, which is the version that I am going to check in.

Cheers
   Nick
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: tc-arm.c.patch
URL: <https://sourceware.org/pipermail/binutils/attachments/20050704/f569906b/attachment.ksh>
More information about the Binutils mailing list