This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: A patch for gcc.c-torture/execute/980505-1.c
- To: hjl at lucon dot org (H.J. Lu)
- Subject: Re: A patch for gcc.c-torture/execute/980505-1.c
- From: Jeffrey A Law <law at cygnus dot com>
- Date: Thu, 18 Jun 1998 23:29:32 -0600
- Cc: egcs-patches at cygnus dot com
- Reply-To: law at cygnus dot com
In message <m0ymjZV-00026AC@ocean.lucon.org>you write:
> * cse.c (delete_trivially_dead_insns): Fix the insn with the
> REG_RETVAL note when deleting an insn.
First, style issue, we do not used mixed case for variable names. Please
use all lower case.
I would recommend "retval_insn" and "libcall_note".
> if (! live_insn)
> {
> + rtx set = NULL_RTX;
> + rtx dest;
> + rtx src = NULL_RTX;
> + rtx next;
> +
> + /* Check if the whole LIBCALL block is deleted. */
> + if (insn == insn_RETVAL)
> + insn_RETVAL = NULL_RTX;
> + else
> + set = single_set (insn);
I do not believe it is possible for an insn with a RETVAL note
to ever be considered dead. Thus the condition "insn == insn_RETVAL"
should never be true. I believe this test is unnecessary.
> + if (set)
Change this to (set && in_libcall) since we do not need to make any
of these transformations unless we are in a libcall block.
> + else if (insn_RETVAL)
This should check && in_libcall too.
While checking "insn_RETVAL" may be equivalent, "in_libcall" is clearer
IMHO.
> + if (src)
Add (&& in_libcall)
> + if (insn_RETVAL && REG_NOTES (insn_RETVAL))
> + /* Fix the insn with the REG_RETVAL note within the
> + LIBCALL block. */
> + fix = insn_RETVAL;
> + else if (note_LIBCALL
> + && REG_NOTES (XEXP (note_LIBCALL, 0)))
> + /* Fix the insn with the REG_RETVAL note in the next
> + LIBCALL block. */
> + fix = XEXP (note_LIBCALL, 0);
Why it is necessary to fix insn with the REG_RETVAL note in the next
libcall? It seems to me we should only fix the REG_RETVAL for the
current libcall block.
If we have to fix other libcall blocks, then this scheme is totally
broken and we'll have to start over with a different scheme because
if cse-skip-blocks, cse-follow-jumps and friends.
> + /* We start from here and go forward until DEST is
> + set or we see FIX. */
> + for (; next != fix; next = NEXT_INSN (next))
Hmmm, I would think that we wouldn't need to search if we only fix
the REG_RETVAL note for this libcall block. Unless DEST could have
originally been set twice in the libcall block. I guess that's
technically possible, so I guess we do need to search.
> + {
> + set = single_set (next);
> + if (set && rtx_equal_p (SET_DEST (set), dest))
> + break;
> + }
It also seems to me that we might want to abort if set is null, or try
to handle that case better (ie by using note_stores to determine what
things are set by an insn).
jeff