diff mbox

PATCH: fix think-o in genrecog.c

Message ID 4E2518C3.8060906@codesourcery.com
State New
Headers show

Commit Message

Sandra Loosemore July 19, 2011, 5:40 a.m. UTC 
I've been experimenting with a patch that adds a new define_peephole2 
for MIPS.  It was blowing up with a segfault in insn-recog.c while 
building libstdc++, and looking at the generated code for my new pattern 
there was clearly a bad offset being passed to peep2_next_insn.  I 
tracked it down to a regression from r174305, Richard's patch to add a 
new data structure to keep track of positions in genrecog.c.  Looks like 
a think-o in the code that accounts for filtering out match_scratch and 
match_dup in the input pattern; and for added fun, the caching of 
position chains was making it produce incorrect code for a 
define_peephole2 that doesn't even involve those things.  :-P  (In fact, 
it's the existing "Same as above, except LO is the initial target of the 
macc" pattern in mips.md that was initially being mis-counted.)

Is this patch OK to check in?  I tested it by diffing the generated 
insn-recog.c for a mips-sde-elf build against one I got using a 
pre-r174305 genrecog.c.

-Sandra

2011-07-18  Sandra Loosemore  <sandra@codesourcery.com>

	gcc/
	* genrecog.c (make_insn_sequence): Correct position numbering
	when filtering out match_scratch and match_dup.

Comments

Richard Sandiford July 20, 2011, 11:24 a.m. UTC | #1 
Sandra Loosemore <sandra@codesourcery.com> writes:
> I've been experimenting with a patch that adds a new define_peephole2 
> for MIPS.  It was blowing up with a segfault in insn-recog.c while 
> building libstdc++, and looking at the generated code for my new pattern 
> there was clearly a bad offset being passed to peep2_next_insn.  I 
> tracked it down to a regression from r174305, Richard's patch to add a 
> new data structure to keep track of positions in genrecog.c.

Sorry :-(

Patch looks OK to me, but I can't approve it.

Richard
Bernd Schmidt July 20, 2011, 11:26 a.m. UTC | #2 
On 07/20/11 13:24, Richard Sandiford wrote:
> Sandra Loosemore <sandra@codesourcery.com> writes:
>> I've been experimenting with a patch that adds a new define_peephole2 
>> for MIPS.  It was blowing up with a segfault in insn-recog.c while 
>> building libstdc++, and looking at the generated code for my new pattern 
>> there was clearly a bad offset being passed to peep2_next_insn.  I 
>> tracked it down to a regression from r174305, Richard's patch to add a 
>> new data structure to keep track of positions in genrecog.c.
> 
> Sorry :-(
> 
> Patch looks OK to me, but I can't approve it.

Looks like the SC has some more things to fix.

Ok for the patch.


Bernd
diff mbox

Patch

Index: genrecog.c
===================================================================
--- genrecog.c	(revision 176433)
+++ genrecog.c	(working copy)
@@ -2345,7 +2345,7 @@  make_insn_sequence (rtx insn, enum routi
 	  if (GET_CODE (tmp) != MATCH_SCRATCH && GET_CODE (tmp) != MATCH_DUP)
 	    {
 	      c_test_pos = next_position (pos_ptr, &root_pos,
-					  POS_PEEP2_INSN, i);
+					  POS_PEEP2_INSN, j);
 	      XVECEXP (x, 0, j) = tmp;
 	      j++;
 	      pos_ptr = &c_test_pos->next;