Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Kevin Buettner <kevinb@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: [PATCH] rs6000-tdep.c: Minor skip_prologue() cleanup
Date: Fri, 05 Apr 2002 14:39:00 -0000	[thread overview]
Message-ID: <1020405223912.ZM1954@localhost.localdomain> (raw)

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)


                 reply	other threads:[~2002-04-05 22:39 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1020405223912.ZM1954@localhost.localdomain \
    --to=kevinb@redhat.com \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox