This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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]

Re: [PATCH/RFC] Fix PR optimization/15100


> Ugh.  I've checked it in already on mainline according to the approval
> in http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00647.html.
> I must confess that now the problem is beyond me.

I've done some digging in gdb, and now understand the issue better.
The case that was already handled was when one or more previous combinations
had reduced the libcall sequence to a single instruction.
Before distribute_notes is called, the affected instruction already
has its REG_NOTES field cleared, and its code is set to NOTE_INSN_DELETED.
Hence find_reg_note returns 0.
If it doesn't return 0 (i.e. tem is not 0), it follows that there is an
undeleted instruction left in the libcall sequence, and hence place being 0
would indicate that the instruction chain is corrupted.

The case that was not handled properly, and which your patch fixes, is
when try_combine combines the first and the last instruction of a libcall
sequence into a following instruction which uses the result of the libcall.
try_combine marks both affected libcall instructions as deleted before
calling distrubute_notes.
Again, because the other insn is already marked as NOTE_INSN_DELETED, we
can't find any REG_NOTE there.

So, your patch works, but it's not immediately apparent from the code
why it works.  The && place is a red herring.

I think something like this would be clearer:

	      /* If the insn carrying the REG_LIBCALL (*) note has been marked
		 as NOTE_INSN_DELETED, find_reg_note returns NULL_RTX.  */
	      if (! tem)
		/* We're deleting either the last remaining instruction of a
		   libcall sequence, or we are deleting both the first and
		   the last instruction of a libcall sequence.  In the latter
		   case, none of the remaining insns (if any) of the libcall
		   computes a libcall value, so they do not belong in a libcall
		   sequence.  Thus, drop the notes.  */
		place = 0;
		/* Since the other side of the libcall still exists, there must
		   be at least on insn left in that direction.  */
	     else if (! place)
	       abort ()
	     else XEXP (tem, 0) = place;
	     
(*): Use REG_RETVAL in the REG_LIBCALL case.


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