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")))]
