* [Patch, AVR]: Fix PR49487 (ICE for wrong rotate scratch)
@ 2011-07-13 13:17 Georg-Johann Lay
2011-07-14 7:54 ` Georg-Johann Lay
0 siblings, 1 reply; 3+ messages in thread
From: Georg-Johann Lay @ 2011-07-13 13:17 UTC (permalink / raw)
To: gcc-patches; +Cc: Anatoly Sokolov, Denis Chertykov, Eric Weddington
This is a patch to fix PR49487.
As Denis will be off-line for some time, it'd be great if
a global reviewer would review it. It appears that he is
the only AVR maintainer who approves patches.
The reason for the ICE is as explained in the PR:
Rotate pattern use "X" as constraint for an operand which is
used as scratch. However, the operand is a match_operand
and not a match_scratch.
Because the scratch is not needed in all situations, I choose
to use match_scratch instead of match_operand and not to fix
the constraints. Fixing constraints would lead to superfluous
allocation of register if no scratch was needed.
Tested with 2 FAILs less: gcc.c-torture/compile/pr46883.c
ICEs without this patch and passes with it.
The test case in the PR passes, too. That test case
passes also the current unpatched 4.7, but it's obvious that
the constraint/operand combination is a bug.
Ok to commit and back-port to 4.6?
Johann
PR target/49487
* config/avr/avr.md (rotl<mode>3): Generate SCRATCH instead
of REG.
(*rotw<mode>): Use const_int_operand for operands2.
Use match_scatch for operands3.
(*rotb<mode>): Ditto
* config/avr/avr.c (avr_rotate_bytes): Treat SCRATCH.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch, AVR]: Fix PR49487 (ICE for wrong rotate scratch)
2011-07-13 13:17 [Patch, AVR]: Fix PR49487 (ICE for wrong rotate scratch) Georg-Johann Lay
@ 2011-07-14 7:54 ` Georg-Johann Lay
2011-07-14 14:52 ` Richard Henderson
0 siblings, 1 reply; 3+ messages in thread
From: Georg-Johann Lay @ 2011-07-14 7:54 UTC (permalink / raw)
To: gcc-patches
Cc: Anatoly Sokolov, Denis Chertykov, Eric Weddington, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]
http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01036.html
Georg-Johann Lay wrote:
> This is a patch to fix PR49487.
Forgot to attach it. Here it is.
> As Denis will be off-line for some time, it'd be great if
> a global reviewer would review it. It appears that he is
> the only AVR maintainer who approves patches.
>
> The reason for the ICE is as explained in the PR:
>
> Rotate pattern use "X" as constraint for an operand which is
> used as scratch. However, the operand is a match_operand
> and not a match_scratch.
>
> Because the scratch is not needed in all situations, I choose
> to use match_scratch instead of match_operand and not to fix
> the constraints. Fixing constraints would lead to superfluous
> allocation of register if no scratch was needed.
>
> Tested with 2 FAILs less: gcc.c-torture/compile/pr46883.c
> ICEs without this patch and passes with it.
>
> The test case in the PR passes, too. That test case
> passes also the current unpatched 4.7, but it's obvious that
> the constraint/operand combination is a bug.
>
> Ok to commit and back-port to 4.6?
>
> Johann
>
> PR target/49487
> * config/avr/avr.md (rotl<mode>3): Generate SCRATCH instead
> of REG.
> (*rotw<mode>): Use const_int_operand for operands2.
> Use match_scatch for operands3.
> (*rotb<mode>): Ditto
> * config/avr/avr.c (avr_rotate_bytes): Treat SCRATCH.
[-- Attachment #2: pr49487.diff --]
[-- Type: text/x-patch, Size: 3988 bytes --]
Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md (revision 176136)
+++ config/avr/avr.md (working copy)
@@ -1597,18 +1597,18 @@ (define_expand "rotl<mode>3"
(match_operand:VOID 2 "const_int_operand" "")))
(clobber (match_dup 3))])]
""
- "
-{
- if (CONST_INT_P (operands[2]) && 0 == (INTVAL (operands[2]) % 8))
{
- if (AVR_HAVE_MOVW && 0 == INTVAL (operands[2]) % 16)
- operands[3] = gen_reg_rtx (<rotsmode>mode);
- else
- operands[3] = gen_reg_rtx (QImode);
- }
- else
- FAIL;
-}")
+ if (CONST_INT_P (operands[2])
+ && 0 == INTVAL (operands[2]) % 8)
+ {
+ if (AVR_HAVE_MOVW && 0 == INTVAL (operands[2]) % 16)
+ operands[3] = gen_rtx_SCRATCH (<rotsmode>mode);
+ else
+ operands[3] = gen_rtx_SCRATCH (QImode);
+ }
+ else
+ FAIL;
+ })
;; Overlapping non-HImode registers often (but not always) need a scratch.
@@ -1620,34 +1620,38 @@ (define_expand "rotl<mode>3"
; Split word aligned rotates using scratch that is mode dependent.
(define_insn_and_split "*rotw<mode>"
[(set (match_operand:HIDI 0 "register_operand" "=r,r,#&r")
- (rotate:HIDI (match_operand:HIDI 1 "register_operand" "0,r,r")
- (match_operand 2 "immediate_operand" "n,n,n")))
- (clobber (match_operand:<rotsmode> 3 "register_operand" "=<rotx>" ))]
- "(CONST_INT_P (operands[2]) &&
- (0 == (INTVAL (operands[2]) % 16) && AVR_HAVE_MOVW))"
+ (rotate:HIDI (match_operand:HIDI 1 "register_operand" "0,r,r")
+ (match_operand 2 "const_int_operand" "n,n,n")))
+ (clobber (match_scratch:<rotsmode> 3 "=<rotx>"))]
+ "AVR_HAVE_MOVW
+ && CONST_INT_P (operands[2])
+ && 0 == INTVAL (operands[2]) % 16"
"#"
"&& (reload_completed || <MODE>mode == DImode)"
[(const_int 0)]
- "avr_rotate_bytes (operands);
- DONE;"
-)
+ {
+ avr_rotate_bytes (operands);
+ DONE;
+ })
; Split byte aligned rotates using scratch that is always QI mode.
(define_insn_and_split "*rotb<mode>"
[(set (match_operand:HIDI 0 "register_operand" "=r,r,#&r")
- (rotate:HIDI (match_operand:HIDI 1 "register_operand" "0,r,r")
- (match_operand 2 "immediate_operand" "n,n,n")))
- (clobber (match_operand:QI 3 "register_operand" "=<rotx>" ))]
- "(CONST_INT_P (operands[2]) &&
- (8 == (INTVAL (operands[2]) % 16)
- || (!AVR_HAVE_MOVW && 0 == (INTVAL (operands[2]) % 16))))"
+ (rotate:HIDI (match_operand:HIDI 1 "register_operand" "0,r,r")
+ (match_operand 2 "const_int_operand" "n,n,n")))
+ (clobber (match_scratch:QI 3 "=<rotx>"))]
+ "CONST_INT_P (operands[2])
+ && (8 == INTVAL (operands[2]) % 16
+ || (!AVR_HAVE_MOVW
+ && 0 == INTVAL (operands[2]) % 16))"
"#"
"&& (reload_completed || <MODE>mode == DImode)"
[(const_int 0)]
- "avr_rotate_bytes (operands);
- DONE;"
-)
+ {
+ avr_rotate_bytes (operands);
+ DONE;
+ })
;;<< << << << << << << << << << << << << << << << << << << << << << << << << <<
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c (revision 176141)
+++ config/avr/avr.c (working copy)
@@ -4438,7 +4438,9 @@ avr_rotate_bytes (rtx operands[])
if (mode == DImode)
move_mode = QImode;
/* Make scratch smaller if needed. */
- if (GET_MODE (scratch) == HImode && move_mode == QImode)
+ if (SCRATCH != GET_CODE (scratch)
+ && HImode == GET_MODE (scratch)
+ && QImode == move_mode)
scratch = simplify_gen_subreg (move_mode, scratch, HImode, 0);
move_size = GET_MODE_SIZE (move_mode);
@@ -4534,6 +4536,8 @@ avr_rotate_bytes (rtx operands[])
When this move occurs, it will break chain deadlock.
The scratch register is substituted for real move. */
+ gcc_assert (SCRATCH != GET_CODE (scratch));
+
move[size].src = move[blocked].dst;
move[size].dst = scratch;
/* Scratch move is never blocked. */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch, AVR]: Fix PR49487 (ICE for wrong rotate scratch)
2011-07-14 7:54 ` Georg-Johann Lay
@ 2011-07-14 14:52 ` Richard Henderson
0 siblings, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2011-07-14 14:52 UTC (permalink / raw)
To: Georg-Johann Lay
Cc: gcc-patches, Anatoly Sokolov, Denis Chertykov, Eric Weddington
On 07/14/2011 12:41 AM, Georg-Johann Lay wrote:
>> Ok to commit and back-port to 4.6?
>>
>> Johann
>>
>> PR target/49487
>> * config/avr/avr.md (rotl<mode>3): Generate SCRATCH instead
>> of REG.
>> (*rotw<mode>): Use const_int_operand for operands2.
>> Use match_scatch for operands3.
>> (*rotb<mode>): Ditto
>> * config/avr/avr.c (avr_rotate_bytes): Treat SCRATCH.
Ok.
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-07-14 14:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-13 13:17 [Patch, AVR]: Fix PR49487 (ICE for wrong rotate scratch) Georg-Johann Lay
2011-07-14 7:54 ` Georg-Johann Lay
2011-07-14 14:52 ` 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).