Re: [PATCH] PR60822 (m68k, missing earlyclobber in extendplussidi)
On Mon, 12 May 2014, Hans-Peter Nilsson wrote: On Thu, 24 Apr 2014, Jeff Law wrote: On 04/16/14 18:20, seg...@kernel.crashing.org wrote: PR target/60822 2014-04-16 Segher Boessenkool seg...@kernel.crashing.org * config/m68k/m68k.md (extendplussidi): Don't allow memory for operand 1. Thanks. I tweaked the comment and added the testcase to the regression suite and installed the fix on the trunk. In the commit message: Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.c trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.x I hope it's not too officious to remind people that dg- markup can be used *even* in the c-torture test-suite these days; gating in .x files is not necessary (since a few years, IIRC). I believe bug 20567 is still current: gcc.c-torture/execute ignores dg- directives and still needs .x files, until someone converts it to the dg harness. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] PR60822 (m68k, missing earlyclobber in extendplussidi)
On Tue, 13 May 2014, Joseph S. Myers wrote:
On Mon, 12 May 2014, Hans-Peter Nilsson wrote:
On Thu, 24 Apr 2014, Jeff Law wrote:
On 04/16/14 18:20, seg...@kernel.crashing.org wrote:
PR target/60822
2014-04-16 Segher Boessenkool seg...@kernel.crashing.org
* config/m68k/m68k.md (extendplussidi): Don't allow memory for
operand 1.
Thanks. I tweaked the comment and added the testcase to the regression
suite
and installed the fix on the trunk.
In the commit message:
Added:
trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.c
trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.x
I hope it's not too officious to remind people that dg- markup
can be used *even* in the c-torture test-suite these days;
gating in .x files is not necessary (since a few years, IIRC).
I believe bug 20567 is still current: gcc.c-torture/execute ignores dg-
directives and still needs .x files, until someone converts it to the dg
harness.
I didn't believe that to be correct, so I checked; it is.
To wit, removing the .x file and adding a top line
/* { dg-require-effective-target int16 } */
didn't stop the test from being executed for an int32plus
target. Also, no sign of dg-anything in c-torture.exp.
Sorry for being wrong.
Let me instead suggest sticking to gcc.dg/torture for new tests.
There I'd be right. :)
brgds, H-P
Re: [PATCH] PR60822 (m68k, missing earlyclobber in extendplussidi)
On Thu, 24 Apr 2014, Jeff Law wrote: On 04/16/14 18:20, seg...@kernel.crashing.org wrote: PR target/60822 2014-04-16 Segher Boessenkool seg...@kernel.crashing.org * config/m68k/m68k.md (extendplussidi): Don't allow memory for operand 1. Thanks. I tweaked the comment and added the testcase to the regression suite and installed the fix on the trunk. In the commit message: Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.c trunk/gcc/testsuite/gcc.c-torture/execute/pr60822.x I hope it's not too officious to remind people that dg- markup can be used *even* in the c-torture test-suite these days; gating in .x files is not necessary (since a few years, IIRC). brgds, H-P
Re: [PATCH] PR60822 (m68k, missing earlyclobber in extendplussidi)
On 04/16/14 18:20, seg...@kernel.crashing.org wrote: On Wed, Apr 16, 2014 at 02:45:28PM -0600, Jeff Law wrote: Isn't the problem that operands 1 is a MEM which use the same register as operands 3 in the memory address? Yes, exactly. 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? No, that does seem better :-) I tried both your suggestions; the first results in better code. Here's a new patch. As before, it builds and fixes the testcase, but I didn't run the testsuite (I have no emulator set up). Thanks, Segher gcc/ PR target/60822 2014-04-16 Segher Boessenkool seg...@kernel.crashing.org * config/m68k/m68k.md (extendplussidi): Don't allow memory for operand 1. Thanks. I tweaked the comment and added the testcase to the regression suite and installed the fix on the trunk. Jeff
Re: [PATCH] PR60822 (m68k, missing earlyclobber in extendplussidi)
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 seg...@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
Re: [PATCH] PR60822 (m68k, missing earlyclobber in extendplussidi)
On Wed, Apr 16, 2014 at 02:45:28PM -0600, Jeff Law wrote:
Isn't the problem that operands 1 is a MEM which use the same register
as operands 3 in the memory address?
Yes, exactly.
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?
No, that does seem better :-)
I tried both your suggestions; the first results in better code. Here's
a new patch. As before, it builds and fixes the testcase, but I didn't
run the testsuite (I have no emulator set up).
Thanks,
Segher
gcc/
PR target/60822
2014-04-16 Segher Boessenkool seg...@kernel.crashing.org
* config/m68k/m68k.md (extendplussidi): Don't allow memory for
operand 1.
---
gcc/config/m68k/m68k.md | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gcc/config/m68k/m68k.md b/gcc/config/m68k/m68k.md
index e61048b..57ba1a1 100644
--- a/gcc/config/m68k/m68k.md
+++ b/gcc/config/m68k/m68k.md
@@ -1868,9 +1868,12 @@ (define_insn extendsidi2
;; Maybe there is a way to make that the general case, by forcing the
;; result of the SI tree to be in the lower register of the DI target
+;; Don't allow two memory operands: it needs an earlyclobber and will
+;; result in worse code.
+
(define_insn extendplussidi
[(set (match_operand:DI 0 register_operand =d)
-(sign_extend:DI (plus:SI (match_operand:SI 1 general_operand %rmn)
+(sign_extend:DI (plus:SI (match_operand:SI 1 nonmemory_operand %rn)
(match_operand:SI 2 general_operand rmn]
{
--
1.8.1.4
