[Bug target/68277] [5/6 Regression] [SH]: error: insn does not satisfy its constraints when compiling erlang

olegendo at gcc dot gnu.org gcc-bugzilla@gcc.gnu.org
Sun Nov 15 07:17:00 GMT 2015


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

--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #7)
> (In reply to Kazumoto Kojima from comment #6)
> I've changed the predicate of the 2nd operand to arith_operand instead
> of const_int_operand in your patch and run testsuite.

Yes, of course, sorry.

> There is one new failure:
> 
> FAIL: gfortran.dg/pr65450.f90   -O3 -g  execution test
> 
> which is
> 
> Program received signal SIGSEGV: Segmentation fault - invalid memory
> reference.
> 
> Weird.  I'm afraid that even my first patch makes wrong codes silently,
> though it doesn't fail for the above test.

With that test and the patch I see one difference:

before:
        dt      r11
        add     r7,r12
        mov.w   .L65,r7    <<<
        add     r1,r7

after:
        dt      r11
        add     r7,r12
        mov     r1,r7      <<<
        add     r1,r7


The insn in .reload is:

(note 133 132 747 9 [bb 9] NOTE_INSN_BASIC_BLOCK)
(insn 747 133 134 9 (set (reg:SI 7 r7)
        (const_int 4000 [0xfa0])) 256 {movsi_ie}
     (nil))

(insn 134 747 135 9 (parallel [
            (set (reg:SI 12 r12 [orig:295 ivtmp.165 ] [295])
                (plus:SI (reg:SI 12 r12 [orig:295 ivtmp.165 ] [295])
                    (reg:SI 7 r7)))
            (clobber (scratch:SI))
        ]) 65 {addsi3_scr}
     (nil))

(insn 135 134 748 9 (parallel [
            (set (reg:SI 7 r7 [orig:298 ivtmp.172 ] [298])
                (plus:SI (reg:SI 1 r1 [orig:280 ivtmp.135 ] [280])
                    (const_int 4000 [0xfa0])))
            (clobber (reg:SI 0 r0))
        ]) 65 {addsi3_scr}
     (nil))


addsi_scr will convert this to something like

   mov.w   #4000, r7
   add     r1,r7

but then .postreload sees the previous same constant load in insn 747, removes
it and changes the add insn to:

(insn 135 134 748 9 (set (reg:SI 7 r7 [orig:298 ivtmp.172 ] [298])
        (plus:SI (reg:SI 1 r1 [orig:280 ivtmp.135 ] [280])
            (reg:SI 7 r7))) 66 {*addsi3}
     (nil))

so we get overlapping regs for operands[0] and operands[2] which is not
checked.  In fact, the very same bug is hidden in the addsi3_scr pattern which
we have added recently, but it seems it hasn't been triggered (yet).

The patch below is a bit of a hammer, but hopefully it works as intended. 
CSiBE code size change is +- 0 everywhere, but there are some differences in
the generated code.  Postreload is able to remove some redundant constant
loads.

I've checked that gfortran.dg/pr65450.f90 and attachment 36686 look OK with
this patch.  Could you please give it another test run?


Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md (revision 230158)
+++ gcc/config/sh/sh.md (working copy)
@@ -2232,11 +2232,51 @@
        }
     }
   else if (!reg_overlap_mentioned_p (operands[0], operands[1]))
-    emit_move_insn (operands[0], operands[1]);
+    {
+      if (!reg_overlap_mentioned_p (operands[0], operands[2]))
+       emit_move_insn (operands[0], operands[1]);
+      else
+       operands[2] = operands[1];
+    }
 }
   [(set_attr "type" "arith")])

+;; Old reload might generate add insns directly (not through the expander) for
+;; the memory address of complex insns like atomic insns when reloading.
 (define_insn_and_split "*addsi3"
+  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+       (plus:SI (match_operand:SI 1 "arith_reg_operand" "r")
+                (match_operand:SI 2 "arith_or_int_operand" "rn")))]
+  "TARGET_SH1 && !sh_lra_p ()
+   && reload_completed
+   && !reg_overlap_mentioned_p (operands[0], operands[1])"
+  "#"
+  "&& 1"
+  [(set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))]
+{
+  if (operands[2] == const0_rtx)
+    {
+      emit_move_insn (operands[0], operands[1]);
+      DONE;
+    }
+
+  if (CONST_INT_P (operands[2]))
+    {
+      if (satisfies_constraint_I08 (operands[2]))
+       emit_move_insn (operands[0], operands[1]);
+      else
+       {
+         emit_move_insn (operands[0], operands[2]);
+         operands[2] = operands[1];
+       }
+    }
+  else if (!reg_overlap_mentioned_p (operands[0], operands[2]))
+    emit_move_insn (operands[0], operands[1]);
+  else
+    operands[2] = operands[1];
+})
+
+(define_insn_and_split "*addsi3"
   [(set (match_operand:SI 0 "arith_reg_dest" "=r,r")
        (plus:SI (match_operand:SI 1 "arith_reg_operand" "%0,r")
                 (match_operand:SI 2 "arith_operand" "rI08,Z")))]


More information about the Gcc-bugs mailing list