diff mbox

PR60822 (m68k, missing earlyclobber in extendplussidi)

Message ID 50f4b5de5e870723c49d1bca41bffadc77eb849d.1397668443.git.segher@kernel.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool April 16, 2014, 7:18 p.m. UTC 
operand[0] has a subreg taken (as operand[3]), which is modified
before operand[1] is used.

Built succesfully but I'm not set up to run the testsuite, sorry.
It fixes the testcase of course.


gcc/ChangeLog:

2014-04-16  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/m68k/m68k.md (extendplussidi): Add earlyclobber.

---
 gcc/config/m68k/m68k.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jeff Law April 16, 2014, 8:45 p.m. UTC | #1 
On 04/16/14 13:18, Segher Boessenkool wrote:
> operand[0] has a subreg taken (as operand[3]), which is modified
> before operand[1] is used.
>
> Built succesfully but I'm not set up to run the testsuite, sorry.
> It fixes the testcase of course.
>
>
> gcc/ChangeLog:
>
> 2014-04-16  Segher Boessenkool  <segher@kernel.crashing.org>
>
> 	* config/m68k/m68k.md (extendplussidi): Add earlyclobber.
But in the case where writing operand3 would overwrite operand1, 
shouldn't we have have used the true arm of this statement:

  if (GET_CODE (operands[1]) == REG
       && REGNO (operands[1]) == REGNO (operands[3]))
     output_asm_insn ("add%.l %2,%3", operands);
   else
     output_asm_insn ("move%.l %2,%3\;add%.l %1,%3", operands);

Looking at the .reload dump I see:


(insn 11 33 14 2 (set (reg:DI 0 %d0 [orig:47 D.1394 ] [47])
         (sign_extend:DI (plus:SI (mem:SI (plus:SI (reg/v/f:SI 8 %a0 
[orig:40 p ] [40])
                         (reg:SI 1 %d1 [44])) [3 p_4(D)->a+0 S4 A16])
                 (mem:SI (plus:SI (reg/v/f:SI 8 %a0 [orig:40 p ] [40])
                         (reg:SI 0 %d0 [45])) [3 p_4(D)->b+0 S4 A16])))) 
j.c:12 78 {extendplussidi}


Isn't the problem that operands 1 is a MEM which use the same register 
as operands 3 in the memory address?

ISTM either removing the memory constraint entirely, or splitting it off 
into a separate alternative and only earlyclobbering that alternative 
would be better.

Or am I missing something?

jeff
diff mbox

Patch

diff --git a/gcc/config/m68k/m68k.md b/gcc/config/m68k/m68k.md
index e61048b..9e7f3e2 100644
--- a/gcc/config/m68k/m68k.md
+++ b/gcc/config/m68k/m68k.md
@@ -1869,7 +1869,7 @@  (define_insn "extendsidi2"
 ;; result of the SI tree to be in the lower register of the DI target
 
 (define_insn "extendplussidi"
-  [(set (match_operand:DI 0 "register_operand" "=d")
+  [(set (match_operand:DI 0 "register_operand" "=&d")
     (sign_extend:DI (plus:SI (match_operand:SI 1 "general_operand" "%rmn")
             (match_operand:SI 2 "general_operand" "rmn"))))]
   ""