diff mbox

gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

Message ID 543D98F3.2030009@gmail.com
State New, archived
Headers

Commit Message

Chen Gang Oct. 14, 2014, 9:43 p.m. UTC 
  On 10/15/2014 05:29 AM, Andreas Schwab wrote:
> Chen Gang <gang.chen.5i5j@gmail.com> writes:
> 
>>  	  && (((inst >> 6) & 0xf) == 0x8
>> -	      || (inst >> 6) & 0xf) == 0x9))
>> +	      || ((inst >> 6) & 0xf) == 0x9)))
> 
>              ((inst >> 6) & 0xe) == 8
> 
> Andreas.
> 

I guess, your fixing may like below, which will be a different logical
working flow.


If you are sure it is, please help send related patch with more details
comments for it (excuse me, I am not quite familiar the related logical
details).


Thanks.

Comments

Joel Brobecker Oct. 15, 2014, 3 p.m. UTC | #1 
> On 10/15/2014 05:29 AM, Andreas Schwab wrote:
> > Chen Gang <gang.chen.5i5j@gmail.com> writes:
> > 
> >>  	  && (((inst >> 6) & 0xf) == 0x8
> >> -	      || (inst >> 6) & 0xf) == 0x9))
> >> +	      || ((inst >> 6) & 0xf) == 0x9)))
> > 
> >              ((inst >> 6) & 0xe) == 8
> > 
> > Andreas.
> > 
> 
> I guess, your fixing may like below, which will be a different logical
> working flow.

I think Andreas is telling you that...

     ((inst >> 6) & 0xf) == 0x8
     || ((inst >> 6) & 0xf) == 0x9

... is logically equivalent to ...

     ((inst >> 6) & 0xe) == 8

In other word, if it does not matter if bit 7 is set or not
(the difference between 0x8 and 0x9) all you have to do is mask it.
That way, you test both conditions with one comparison instead of 2.
diff mbox

Patch

diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 627f31a..3112732 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -1402,8 +1402,8 @@  inst_saves_gr (unsigned long inst)
      too.  */
   if ((inst >> 26) == 0x19 || (inst >> 26) == 0x18
       || ((inst >> 26) == 0x3
-         && (((inst >> 6) & 0xf) == 0x8
-             || (inst >> 6) & 0xf) == 0x9))
+         && ((inst >> 6) & 0xf) == 0x8
+             || ((inst >> 6) & 0xf) == 0x9))
     return hppa_extract_5R_store (inst);
 
   return 0;