[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler

olegendo at gcc dot gnu.org gcc-bugzilla@gcc.gnu.org
Wed Jun 24 13:41:00 GMT 2015


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979

--- Comment #37 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #36)
> It seems the tstsi peephole is still wrong.  While working on AMS the
> following example:
> 
> int test (char* x, char* y, int z)
> {
>   return ((x[2] & x[3]) == 0) + z;
> }
> 
> silently produced wrong code by omitting one of the two mem loads.  After an
> update to the current trunk it results in
> 
> sh_tmp.cpp: In function 'int test(char*, char*, int)':
> sh_tmp.cpp:6:1: error: insn does not satisfy its constraints:
>  }
>  ^
> (insn 88 69 89 2 (set (reg:SI 1 r1)
>         (sign_extend:SI (mem:QI (plus:SI (reg/v/f:SI 4 r4 [orig:169 x ]
> [169])
>                     (const_int 3 [0x3])) [0 MEM[(char *)x_1(D) + 3B]+0 S1
> A8]))) sh_tmp.cpp:5 232 {*extendqisi2_compact_mem_disp}
>      (nil))
> 
> which I think is the same as PR 66611.  The tstsi related peephole at line
> 14709 is causing the trouble.

The peephole in question can't be used for code sequences that include QImode
or HImode mem loads of the operands.  The following fixes the issue by matching
the resulting insn pattern and the constraints while emitting the new peephole
insns.  I think it's good to do those full checks as opposed to checking for
the currently known problematic mem loads.  This way we an be sure that
whatever the "emit_insn (gen_rtx_SET ..." produces is actually valid code.

The problem exists on the 5 branch and on trunk.

Kaz, could you please add this to your test runs?  For me it's a bit difficult
to do proper testing at the moment.

Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md (revision 224565)
+++ gcc/config/sh/sh.md (working copy)
@@ -14727,8 +14727,19 @@
   if (REGNO (operands[1]) == REGNO (operands[2]))
       operands[2] = gen_rtx_REG (SImode, REGNO (operands[0]));

-  sh_check_add_incdec_notes (emit_insn (gen_rtx_SET (operands[2],
-                                                    operands[3])));
+  // We don't know what the new set insn will be in detail.  Just make sure
+  // that it still can be recognized and the constraints are satisfied.
+  rtx_insn* i = emit_insn (gen_rtx_SET (operands[2], operands[3]));
+                                                    
+  recog_data_d prev_recog_data = recog_data;
+  bool i_invalid = insn_invalid_p (i, false); 
+  recog_data = prev_recog_data;
+  
+  if (i_invalid)
+    FAIL;
+    
+  sh_check_add_incdec_notes (i);
+
   emit_insn (gen_tstsi_t (operands[2],
                          gen_rtx_REG (SImode, (REGNO (operands[1])))));
 })
@@ -14755,8 +14766,19 @@
        || REGNO (operands[2]) == REGNO (operands[5]))"
   [(const_int 0)]
 {
-  sh_check_add_incdec_notes (emit_insn (gen_rtx_SET (operands[2],
-                                                    operands[3])));
+  // We don't know what the new set insn will be in detail.  Just make sure
+  // that it still can be recognized and the constraints are satisfied.
+  rtx_insn* i = emit_insn (gen_rtx_SET (operands[2], operands[3]));
+
+  recog_data_d prev_recog_data = recog_data;
+  bool i_invalid = insn_invalid_p (i, false); 
+  recog_data = prev_recog_data;
+  
+  if (i_invalid)
+    FAIL;
+    
+  sh_check_add_incdec_notes (i);
+  
   emit_insn (gen_tstsi_t (operands[2],
                          gen_rtx_REG (SImode, (REGNO (operands[1])))));
 })



More information about the Gcc-bugs mailing list