This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] rs6000-tdep.c: Minor skip_prologue() cleanup


I've just committed the patch below.  It eliminates the following
clause from the skip_prologue() loop:

      else if (((op & 0xffff0000) == 0x801e0000 ||   /* lwz 0,NUM(r30), used
						        in V.4 -mrelocatable */
		op == 0x7fc0f214) &&	/* add r30,r0,r30, used
					   in V.4 -mrelocatable */
	       lr_reg == 0x901e0000)
	{
	  continue;

	}

I've concluded that there's a typo in the constant in this part of
the condition:

    [C]	  lr_reg == 0x901e0000

However, I don't know what this constant should be changed to.  Moreover,
I've concluded that this typo is causing the clause's condition to always
evaluate to 0 (false).

In skip_prologue(), lr_reg is set by only three statements:

    [1]	  int lr_reg = -1;
    [2]	  lr_reg = (op & 0x03e00000) | 0x90010000;
    [3]	  lr_reg = 0;

Clearly if lr_reg has the values given to it by either statement [1] or
[3], it won't match 0x901e0000, thus condition [C] will be false.  When set
by statement [2], the following assertion is true:

    (lr_reg & 0x00010000) != 0

However, 

    (0x901e0000 & 0x00010000) == 0,

so condition [C] can't hold for statement [2] either.  (I.e, lr_reg will
have some bits set which aren't set in 0x901e0000, so the two don't have
a chance of comparing as equal.)

Thus condition [C] can never be true when lr_reg has a value given to
it by statements [1], [2], or [3].  This causes the entire condition
for the clause in question to always evaluate as false which means that
the clause can safely be eliminated.

Of course, one could argue that the clause ought to be fixed, and it
fact, this was my first inclination.  It seems to me that attempting
to fix it could do more harm than good though.  According to my CVS
research, the clause in question was added 27-Jul-95, and I've
verified that it also always evaluated to false in the version of
skip_prologue that existed on that date.  In other words, we've gotten
by for nearly seven years without this clause.

On the other hand, if we attempt to fix it, it seems to me that it
will be difficult to verify that it actually works properly since
one or more of the machines, operating systems, or compilers in use
in 1995 will be impossible to locate.  Moreover, it seem to me that
it'd be possible for this clause to indavertently trigger when it
didn't before (due to the bug) which might possibly cause a regression.
In other words, I wouldn't be willing to commit a potential fix without
a lot of regression testing.

Lastly, I will note that even if the clause in question did work and
do something desirable, skip_prologue() would still usually produce
the same results without it due to the catchall case at the
end of skip_prologue()'s loop which did not exist in 1995.

	* rs6000-tdep.c (skip_prologue): Eliminate unused/unreachable
	clause.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.42
diff -u -p -r1.42 rs6000-tdep.c
--- rs6000-tdep.c	2002/04/01 05:58:45	1.42
+++ rs6000-tdep.c	2002/04/05 21:39:06
@@ -592,15 +592,6 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	  break;
 
 	}
-      else if (((op & 0xffff0000) == 0x801e0000 ||   /* lwz 0,NUM(r30), used
-						        in V.4 -mrelocatable */
-		op == 0x7fc0f214) &&	/* add r30,r0,r30, used
-					   in V.4 -mrelocatable */
-	       lr_reg == 0x901e0000)
-	{
-	  continue;
-
-	}
       else if ((op & 0xffff0000) == 0x3fc00000 ||  /* addis 30,0,foo@ha, used
 						      in V.4 -mminimal-toc */
 	       (op & 0xffff0000) == 0x3bde0000)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]