public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fwd: [AVR] Fix PR49881
       [not found] <4E36F758.90602@redhat.com>
@ 2011-08-01 18:59 ` Richard Henderson
  2011-08-02  7:53   ` Georg-Johann Lay
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2011-08-01 18:59 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

Dang, forgot to add gcc-patches...

-------- Original Message --------
Subject: [AVR] Fix PR49881
Date: Mon, 01 Aug 2011 11:58:32 -0700
From: Richard Henderson <rth@redhat.com>
To: Denis Chertykov <chertykov@gmail.com>, eric.weddington@atmel.com
CC: avr@gjlay.de

The following iteration fixes the two regressions reported
in comment 7 of the PR.

These were ICEs due to emit_push_insn_single being "helpful"
with pushing complex numbers.  Instead of recursing for the
components of a complex number, it simply generated raw
pre_dec patterns.  This is arguably a middle-end bug, but 
it's easier to fix in the backend by listing complex modes
in the macro for the push expander.

Ok?


r~


[-- Attachment #2: d-49881-3 --]
[-- Type: text/plain, Size: 2500 bytes --]

	* config/avr/avr.h (PUSH_ROUNDING): New.
	* config/avr/avr.md (pushqi1): Rename from *pushqi.
	(*pushhi, *pushsi, *pushsf): Remove.
	(MPUSH): New mode iterator.
	(push<MPUSH>1): New expander.


diff --git a/gcc/config/avr/avr.h b/gcc/config/avr/avr.h
index ddd30d6..6a80693 100644
--- a/gcc/config/avr/avr.h
+++ b/gcc/config/avr/avr.h
@@ -665,3 +665,7 @@ struct GTY(()) machine_function
   /* 'true' if a callee might be tail called */
   int sibcall_fails;
 };
+
+/* AVR does not round pushes, but the existance of this macro is
+   required in order for pushes to be generated.  */
+#define PUSH_ROUNDING(X)	(X)
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index 55a883e..f60f9f0 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -202,8 +202,7 @@
   DONE;
 })
 
-
-(define_insn "*pushqi"
+(define_insn "pushqi1"
   [(set (mem:QI (post_dec:HI (reg:HI REG_SP)))
         (match_operand:QI 0 "reg_or_0_operand" "r,L"))]
   ""
@@ -212,33 +211,29 @@
 	push __zero_reg__"
   [(set_attr "length" "1,1")])
 
-(define_insn "*pushhi"
-  [(set (mem:HI (post_dec:HI (reg:HI REG_SP)))
-        (match_operand:HI 0 "reg_or_0_operand" "r,L"))]
-  ""
-  "@
-	push %B0\;push %A0
-	push __zero_reg__\;push __zero_reg__"
-  [(set_attr "length" "2,2")])
+;; All modes for a multi-byte push.  We must include complex modes here too,
+;; lest emit_single_push_insn "helpfully " create the auto-inc itself.
+(define_mode_iterator MPUSH
+  [(CQI "")
+   (HI "") (CHI "")
+   (SI "") (CSI "")
+   (DI "") (CDI "")
+   (SF "") (SC "")])
 
-(define_insn "*pushsi"
-  [(set (mem:SI (post_dec:HI (reg:HI REG_SP)))
-        (match_operand:SI 0 "reg_or_0_operand" "r,L"))]
+(define_expand "push<mode>1"
+  [(match_operand:MPUSH 0 "general_operand" "")]
   ""
-  "@
-	push %D0\;push %C0\;push %B0\;push %A0
-	push __zero_reg__\;push __zero_reg__\;push __zero_reg__\;push __zero_reg__"
-  [(set_attr "length" "4,4")])
-
-(define_insn "*pushsf"
-  [(set (mem:SF (post_dec:HI (reg:HI REG_SP)))
-        (match_operand:SF 0 "register_operand" "r"))]
-  ""
-  "push %D0
-	push %C0
-	push %B0
-	push %A0"
-  [(set_attr "length" "4")])
+{
+  int i;
+  for (i = GET_MODE_SIZE (<MODE>mode) - 1; i >= 0; --i)
+    {
+      rtx part = simplify_gen_subreg (QImode, operands[0], <MODE>mode, i);
+      if (part != const0_rtx)
+	part = force_reg (QImode, part);
+      emit_insn (gen_pushqi1 (part));
+    }
+  DONE;
+})
 
 ;;========================================================================
 ;; move byte

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [AVR] Fix PR49881
  2011-08-01 18:59 ` Fwd: [AVR] Fix PR49881 Richard Henderson
@ 2011-08-02  7:53   ` Georg-Johann Lay
  2011-08-02 17:41     ` Richard Henderson
  0 siblings, 1 reply; 6+ messages in thread
From: Georg-Johann Lay @ 2011-08-02  7:53 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches

Richard Henderson wrote:
> Dang, forgot to add gcc-patches...
> 
> -------- Original Message --------
> Subject: [AVR] Fix PR49881
> 
> The following iteration fixes the two regressions reported
> in comment 7 of the PR.
> 
> These were ICEs due to emit_push_insn_single being "helpful"
> with pushing complex numbers.  Instead of recursing for the
> components of a complex number, it simply generated raw
> pre_dec patterns.  This is arguably a middle-end bug, but 
> it's easier to fix in the backend by listing complex modes
> in the macro for the push expander.
> 
> Ok?
> 
> 
> r~

After updating to r177084 (your patch is r177071) I still get *many*
test fails or untested (programs hangs?) fails.

Just few examples:

PASS: gcc.c-torture/execute/920726-1.c compilation,  -O1
FAIL: gcc.c-torture/execute/920726-1.c execution,  -O1

PASS: gcc.c-torture/execute/930513-1.c compilation,  -Os
UNTESTED: gcc.c-torture/execute/930513-1.c execution,  -Os

FAIL: gcc.c-torture/execute/920726-1.c execution,  -O0
FAIL: gcc.c-torture/execute/920726-1.c execution,  -O1
FAIL: gcc.c-torture/execute/920726-1.c execution,  -O2
FAIL: gcc.c-torture/execute/920726-1.c execution,  -O3 -fomit-frame-pointer
FAIL: gcc.c-torture/execute/920726-1.c execution,  -O3 -fomit-frame-pointer -funroll-loops
FAIL: gcc.c-torture/execute/920726-1.c execution,  -O3 -fomit-frame-pointer -funroll-all-loops
-finline-functions
FAIL: gcc.c-torture/execute/920726-1.c execution,  -O3 -g

There are still unrecognizables:

gcc.c-torture/execute/complex-7.c:56:1: error: unrecognizable insn:
(insn 17 14 18 3 (set (mem:SF (post_dec:HI (reg/f:HI 32 __SP_L__)) [0 S4 A8])
        (reg:SF 43 [ f5.0+4 ]))
/mnt/nfs/home/georg/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/execute/complex-7.c:52 -1
     (nil))

FAIL: gcc.c-torture/execute/complex-7.c compilation,  -O0

Is it possible that you run avr regressions?

Johann

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [AVR] Fix PR49881
  2011-08-02  7:53   ` Georg-Johann Lay
@ 2011-08-02 17:41     ` Richard Henderson
  2011-08-02 19:03       ` Georg-Johann Lay
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2011-08-02 17:41 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

On 08/02/2011 12:52 AM, Georg-Johann Lay wrote:
> There are still unrecognizables:
> 
> gcc.c-torture/execute/complex-7.c:56:1: error: unrecognizable insn:
> (insn 17 14 18 3 (set (mem:SF (post_dec:HI (reg/f:HI 32 __SP_L__)) [0 S4 A8])
>         (reg:SF 43 [ f5.0+4 ]))
> /mnt/nfs/home/georg/gnu/gcc.gnu.org/trunk/gcc/testsuite/gcc.c-torture/execute/complex-7.c:52 -1
>      (nil))

I was pretty sure I ran the compile tests.  I've tried several
times to come up with an environment that would properly run
the simulator, without success.  AVR support seems to be in
too many different places, none of which properly communicate
with each other.

That said, this fixes that test case, committed as obvious.


r~

[-- Attachment #2: z --]
[-- Type: text/plain, Size: 397 bytes --]

        * config/avr/avr.md (push<MPUSH>1): Don't constrain the operand.

diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index f60f9f0..b8560df 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -221,7 +221,7 @@
    (SF "") (SC "")])
 
 (define_expand "push<mode>1"
-  [(match_operand:MPUSH 0 "general_operand" "")]
+  [(match_operand:MPUSH 0 "" "")]
   ""
 {
   int i;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [AVR] Fix PR49881
  2011-08-02 17:41     ` Richard Henderson
@ 2011-08-02 19:03       ` Georg-Johann Lay
  2011-08-02 19:11         ` Richard Henderson
  2011-08-02 19:53         ` Richard Henderson
  0 siblings, 2 replies; 6+ messages in thread
From: Georg-Johann Lay @ 2011-08-02 19:03 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches, Jörg Wunsch

In CCing Jörg.

Richard Henderson schrieb:
> On 08/02/2011 12:52 AM, Georg-Johann Lay wrote:
> 
>>There are still unrecognizables:
>>
>>gcc.c-torture/execute/complex-7.c:56:1: error: unrecognizable insn:
>>(insn 17 14 18 3 (set (mem:SF (post_dec:HI (reg/f:HI 32 __SP_L__)) [0 S4 A8])
>>        (reg:SF 43 [ f5.0+4 ]))
>>     (nil))
> 
> I was pretty sure I ran the compile tests.  I've tried several
> times to come up with an environment that would properly run
> the simulator, without success.  AVR support seems to be in
> too many different places, none of which properly communicate
> with each other.

Just ask :-)

For questions/answers you may want to read/post to the looow traffic 
avr-gcc-list:
    http://lists.gnu.org/archive/html/avr-gcc-list/

== binutils ==

configure plain vanilla: --target=avr --prefix=<same-as-avr-gcc>
build and install

== gcc ==

You've done that already. I'm using something around
configure --target=avr --enable-languages=c,c++ --disable-nls 
--prefix=<same-as-binutils> --with-dwarf2

== avr-libc ==

Needed to run because it provides startup code and C libs.
A bit tricky, easiest to use is current CVS head.

With the latest version 1.7.1 from
http://download.savannah.gnu.org/releases/avr-libc/
you will need the patches
http://svn.sv.gnu.org/viewvc?view=rev&root=avr-libc&revision=2239
http://svn.sv.gnu.org/viewvc?view=rev&root=avr-libc&revision=2241

configure with --host=avr --prefix=<same-as-avr-gcc> CC=avr-gcc
If you give CC= (e.g. if your avr-gcc is noz in PATH) note that
the name must contain "avr" i.e. build using CC=xgcc or so does
not work. In-tree build is not supported.

Building is currently blocked by PR49864 so you have a dead-lock
and may want to downgrade gcc or avr BE to r177070 to build avr-libc.

Install it.

If I overlooked something Joerg will correct me.

== avrtest ==

There is a text
http://lists.gnu.org/archive/html/avr-gcc-list/2011-06/msg00015.html
and a README:
http://winavr.cvs.sourceforge.net/viewvc/winavr/avrtest/README?view=markup

In the case there are questions: Ask.

> That said, this fixes that test case, committed as obvious.

Didn't try it yet. Is that capable of fixing the runtime FAILs?

Johann

> 
> r~
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [AVR] Fix PR49881
  2011-08-02 19:03       ` Georg-Johann Lay
@ 2011-08-02 19:11         ` Richard Henderson
  2011-08-02 19:53         ` Richard Henderson
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2011-08-02 19:11 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: GCC Patches, Jörg Wunsch

On 08/02/2011 12:01 PM, Georg-Johann Lay wrote:
> == avrtest ==
> 
> There is a text
> http://lists.gnu.org/archive/html/avr-gcc-list/2011-06/msg00015.html
> and a README:
> http://winavr.cvs.sourceforge.net/viewvc/winavr/avrtest/README?view=markup
> 
> In the case there are questions: Ask.

Ah, a totally different simulator than google found.

I don't suppose you've ever looked into fixing up what's
in binutils/sim/avr/ so that it works well enough, and 
gets automatically included into gdb as "target sim"?


r~

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [AVR] Fix PR49881
  2011-08-02 19:03       ` Georg-Johann Lay
  2011-08-02 19:11         ` Richard Henderson
@ 2011-08-02 19:53         ` Richard Henderson
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2011-08-02 19:53 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: GCC Patches, Jörg Wunsch

On 08/02/2011 12:01 PM, Georg-Johann Lay wrote:
> Didn't try it yet. Is that capable of fixing the runtime FAILs?

Apparently.  I installed your simulator and get comparable results to 

  http://gcc.gnu.org/ml/gcc-testresults/2011-07/msg03632.html


r~

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-08-02 19:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <4E36F758.90602@redhat.com>
2011-08-01 18:59 ` Fwd: [AVR] Fix PR49881 Richard Henderson
2011-08-02  7:53   ` Georg-Johann Lay
2011-08-02 17:41     ` Richard Henderson
2011-08-02 19:03       ` Georg-Johann Lay
2011-08-02 19:11         ` Richard Henderson
2011-08-02 19:53         ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).