* [PATCH] i386: Remove "m" constraint for "register_operand"
@ 2007-08-10 9:40 Rask Ingemann Lambertsen
2007-08-10 9:53 ` Paolo Bonzini
2007-08-14 13:08 ` Jan Hubicka
0 siblings, 2 replies; 8+ messages in thread
From: Rask Ingemann Lambertsen @ 2007-08-10 9:40 UTC (permalink / raw)
To: gcc-patches
There is a mismatch between the constraints permitting memory operands
and the predicate accepting only register operands. This patch fixes that.
Bootstrapped and tested on x86_64-unknown-linux-gnu with no regressions. Ok
for trunk?
2007-08-10 Rask Ingemann Lambertsen <rask@sygehus.dk>
* config/i386/i386.md (subsi3_carry_zext): Remove "m" constraint for
"register_operand".
(*iorsi_1_zext): Likewise.
(*iorsi_1_zext_imm): Likewise.
* config/i386/sse.md: (*sse4_1_extractps): Likewise.
(sse2_vmsqrtv2df2): Likewise.
Index: gcc/config/i386/i386.md
===================================================================
@@ -6645,11 +6751,11 @@
(set_attr "mode" "SI")])
(define_insn "subsi3_carry_zext"
- [(set (match_operand:DI 0 "register_operand" "=rm,r")
+ [(set (match_operand:DI 0 "register_operand" "=r")
(zero_extend:DI
- (minus:SI (match_operand:SI 1 "register_operand" "0,0")
+ (minus:SI (match_operand:SI 1 "register_operand" "0")
(plus:SI (match_operand:SI 3 "ix86_carry_flag_operator" "")
- (match_operand:SI 2 "general_operand" "ri,rm")))))
+ (match_operand:SI 2 "general_operand" "g")))))
(clobber (reg:CC FLAGS_REG))]
"TARGET_64BIT && ix86_binary_operator_ok (MINUS, SImode, operands)"
"sbb{l}\t{%2, %k0|%k0, %2}"
@@ -8661,7 +8767,7 @@
;; See comment for addsi_1_zext why we do use nonimmediate_operand
(define_insn "*iorsi_1_zext"
- [(set (match_operand:DI 0 "register_operand" "=rm")
+ [(set (match_operand:DI 0 "register_operand" "=r")
(zero_extend:DI
(ior:SI (match_operand:SI 1 "nonimmediate_operand" "%0")
(match_operand:SI 2 "general_operand" "rim"))))
@@ -8672,7 +8778,7 @@
(set_attr "mode" "SI")])
(define_insn "*iorsi_1_zext_imm"
- [(set (match_operand:DI 0 "register_operand" "=rm")
+ [(set (match_operand:DI 0 "register_operand" "=r")
(ior:DI (zero_extend:DI (match_operand:SI 1 "register_operand" "%0"))
(match_operand:DI 2 "x86_64_zext_immediate_operand" "Z")))
(clobber (reg:CC FLAGS_REG))]
Index: gcc/config/i386/sse.md
===================================================================
--- gcc/config/i386/sse.md (revision 127179)
+++ gcc/config/i386/sse.md (working copy)
@@ -1532,7 +1532,7 @@
})
(define_insn "*sse4_1_extractps"
- [(set (match_operand:SF 0 "register_operand" "=rm")
+ [(set (match_operand:SF 0 "register_operand" "=r")
(vec_select:SF
(match_operand:V4SF 1 "register_operand" "x")
(parallel [(match_operand:SI 2 "const_0_to_3_operand" "n")])))]
@@ -1694,7 +1694,7 @@
(define_insn "sse2_vmsqrtv2df2"
[(set (match_operand:V2DF 0 "register_operand" "=x")
(vec_merge:V2DF
- (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "xm"))
+ (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "x"))
(match_operand:V2DF 2 "register_operand" "0")
(const_int 1)))]
"TARGET_SSE2"
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] i386: Remove "m" constraint for "register_operand"
2007-08-10 9:40 [PATCH] i386: Remove "m" constraint for "register_operand" Rask Ingemann Lambertsen
@ 2007-08-10 9:53 ` Paolo Bonzini
2007-08-10 10:23 ` Hans-Peter Nilsson
2007-08-10 11:04 ` Rask Ingemann Lambertsen
2007-08-14 13:08 ` Jan Hubicka
1 sibling, 2 replies; 8+ messages in thread
From: Paolo Bonzini @ 2007-08-10 9:53 UTC (permalink / raw)
To: Rask Ingemann Lambertsen; +Cc: gcc-patches
Rask Ingemann Lambertsen wrote:
> There is a mismatch between the constraints permitting memory operands
> and the predicate accepting only register operands. This patch fixes that.
> Bootstrapped and tested on x86_64-unknown-linux-gnu with no regressions. Ok
> for trunk?
I don't think so. The register_operand could be replaced with a
general_operand following the same logic (possibly with further tweaks
to the predicates); also, given the code that is present now in the
trunk, reload might still use the memory constraint in case a value is
spilled.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i386: Remove "m" constraint for "register_operand"
2007-08-10 9:53 ` Paolo Bonzini
@ 2007-08-10 10:23 ` Hans-Peter Nilsson
2007-08-10 11:04 ` Rask Ingemann Lambertsen
1 sibling, 0 replies; 8+ messages in thread
From: Hans-Peter Nilsson @ 2007-08-10 10:23 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Rask Ingemann Lambertsen, gcc-patches
On Fri, 10 Aug 2007, Paolo Bonzini wrote:
> Rask Ingemann Lambertsen wrote:
> > There is a mismatch between the constraints permitting memory operands
> > and the predicate accepting only register operands. This patch fixes that.
> > Bootstrapped and tested on x86_64-unknown-linux-gnu with no regressions. Ok
> > for trunk?
>
> I don't think so. The register_operand could be replaced with a
> general_operand following the same logic (possibly with further tweaks
> to the predicates); also, given the code that is present now in the
> trunk, reload might still use the memory constraint in case a value is
> spilled.
It's a bug to have looser constraints than predicate, but I'd
agree the correct change is to have general_operand or rather
nonimmediate_operand there.
brgds, H-P
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i386: Remove "m" constraint for "register_operand"
2007-08-10 9:53 ` Paolo Bonzini
2007-08-10 10:23 ` Hans-Peter Nilsson
@ 2007-08-10 11:04 ` Rask Ingemann Lambertsen
1 sibling, 0 replies; 8+ messages in thread
From: Rask Ingemann Lambertsen @ 2007-08-10 11:04 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: gcc-patches
On Fri, Aug 10, 2007 at 11:53:23AM +0200, Paolo Bonzini wrote:
> Rask Ingemann Lambertsen wrote:
> > There is a mismatch between the constraints permitting memory operands
> >and the predicate accepting only register operands. This patch fixes that.
> >Bootstrapped and tested on x86_64-unknown-linux-gnu with no regressions. Ok
> >for trunk?
>
> I don't think so. The register_operand could be replaced with a
> general_operand following the same logic (possibly with further tweaks
> to the predicates);
1) The destination must not be general_operand (try it if you don't believe
me), but you could argue for nonimmediate_operand instead. See 2).
2) The *zext* patterns really do only take a register destination.
> also, given the code that is present now in the
> trunk, reload might still use the memory constraint in case a value is
> spilled.
Wouldn't you get "unrecognizable insn" ICEs?
--
Rask Ingemann Lambertsen
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i386: Remove "m" constraint for "register_operand"
2007-08-10 9:40 [PATCH] i386: Remove "m" constraint for "register_operand" Rask Ingemann Lambertsen
2007-08-10 9:53 ` Paolo Bonzini
@ 2007-08-14 13:08 ` Jan Hubicka
2007-08-14 13:24 ` Lu, Hongjiu
2007-08-15 10:41 ` Rask Ingemann Lambertsen
1 sibling, 2 replies; 8+ messages in thread
From: Jan Hubicka @ 2007-08-14 13:08 UTC (permalink / raw)
To: Rask Ingemann Lambertsen, hongjiu.lu; +Cc: gcc-patches
> There is a mismatch between the constraints permitting memory operands
> and the predicate accepting only register operands. This patch fixes that.
> Bootstrapped and tested on x86_64-unknown-linux-gnu with no regressions. Ok
> for trunk?
The i386.md part is OK (zext variants really needs to destinate)
with following change:
> - (match_operand:SI 2 "general_operand" "ri,rm")))))
> + (match_operand:SI 2 "general_operand" "g")))))
"rim" here for consistency. I guess we should rename them all to "g" in
followup step.
For sse.md part:
> (define_insn "*sse4_1_extractps"
> - [(set (match_operand:SF 0 "register_operand" "=rm")
> + [(set (match_operand:SF 0 "register_operand" "=r")
H.J., can you please double check that memory is not allowed here?
It seems to me that changing register_operand to nonimmediate_operand
would be correct fix.
This instruction is not yet in my ISA reference.
> (vec_select:SF
> (match_operand:V4SF 1 "register_operand" "x")
> (parallel [(match_operand:SI 2 "const_0_to_3_operand" "n")])))]
> @@ -1694,7 +1694,7 @@
> (define_insn "sse2_vmsqrtv2df2"
> [(set (match_operand:V2DF 0 "register_operand" "=x")
> (vec_merge:V2DF
> - (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "xm"))
> + (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "x"))
nonimmediate_operand here instead of removing 'm'
Honza
> (match_operand:V2DF 2 "register_operand" "0")
> (const_int 1)))]
> "TARGET_SSE2"
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] i386: Remove "m" constraint for "register_operand"
2007-08-14 13:08 ` Jan Hubicka
@ 2007-08-14 13:24 ` Lu, Hongjiu
2007-08-15 10:41 ` Rask Ingemann Lambertsen
1 sibling, 0 replies; 8+ messages in thread
From: Lu, Hongjiu @ 2007-08-14 13:24 UTC (permalink / raw)
To: Jan Hubicka, Rask Ingemann Lambertsen; +Cc: gcc-patches
From binutils:
[hjl@gnu-6 i386]$ grep extractps *.s
sse4_1.s: extractps $0,%xmm0,%ecx
sse4_1.s: extractps $0,%xmm0,(%ecx)
...
Memory is allowed as destination. It was an oversight in my SSE4 patch.
Thanks for catching it.
H.J.
hongjiu.lu@intel.com
>-----Original Message-----
>From: Jan Hubicka [mailto:hubicka@ucw.cz]
>Sent: Tuesday, August 14, 2007 6:08 AM
>To: Rask Ingemann Lambertsen; Lu, Hongjiu
>Cc: gcc-patches@gcc.gnu.org
>Subject: Re: [PATCH] i386: Remove "m" constraint for "register_operand"
>
>> There is a mismatch between the constraints permitting
>memory operands
>> and the predicate accepting only register operands. This
>patch fixes that.
>> Bootstrapped and tested on x86_64-unknown-linux-gnu with no
>regressions. Ok
>> for trunk?
>
>The i386.md part is OK (zext variants really needs to destinate)
>with following change:
>> - (match_operand:SI 2 "general_operand" "ri,rm")))))
>> + (match_operand:SI 2 "general_operand" "g")))))
>
>"rim" here for consistency. I guess we should rename them all
>to "g" in
>followup step.
>
>For sse.md part:
>> (define_insn "*sse4_1_extractps"
>> - [(set (match_operand:SF 0 "register_operand" "=rm")
>> + [(set (match_operand:SF 0 "register_operand" "=r")
>
>H.J., can you please double check that memory is not allowed here?
>It seems to me that changing register_operand to nonimmediate_operand
>would be correct fix.
>This instruction is not yet in my ISA reference.
>> (vec_select:SF
>> (match_operand:V4SF 1 "register_operand" "x")
>> (parallel [(match_operand:SI 2 "const_0_to_3_operand"
>"n")])))]
>> @@ -1694,7 +1694,7 @@
>> (define_insn "sse2_vmsqrtv2df2"
>> [(set (match_operand:V2DF 0 "register_operand" "=x")
>> (vec_merge:V2DF
>> - (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "xm"))
>> + (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "x"))
>nonimmediate_operand here instead of removing 'm'
>
>Honza
>> (match_operand:V2DF 2 "register_operand" "0")
>> (const_int 1)))]
>> "TARGET_SSE2"
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] i386: Remove "m" constraint for "register_operand"
2007-08-14 13:08 ` Jan Hubicka
2007-08-14 13:24 ` Lu, Hongjiu
@ 2007-08-15 10:41 ` Rask Ingemann Lambertsen
2007-08-15 10:45 ` Jan Hubicka
1 sibling, 1 reply; 8+ messages in thread
From: Rask Ingemann Lambertsen @ 2007-08-15 10:41 UTC (permalink / raw)
To: Jan Hubicka; +Cc: hongjiu.lu, gcc-patches
On Tue, Aug 14, 2007 at 03:07:44PM +0200, Jan Hubicka wrote:
> The i386.md part is OK (zext variants really needs to destinate)
> with following change:
> > - (match_operand:SI 2 "general_operand" "ri,rm")))))
> > + (match_operand:SI 2 "general_operand" "g")))))
>
> "rim" here for consistency. I guess we should rename them all to "g" in
> followup step.
>
> For sse.md part:
> > (define_insn "*sse4_1_extractps"
> > - [(set (match_operand:SF 0 "register_operand" "=rm")
> > + [(set (match_operand:SF 0 "register_operand" "=r")
>
> H.J., can you please double check that memory is not allowed here?
> It seems to me that changing register_operand to nonimmediate_operand
> would be correct fix.
> This instruction is not yet in my ISA reference.
> > @@ -1694,7 +1694,7 @@
> > (define_insn "sse2_vmsqrtv2df2"
> > [(set (match_operand:V2DF 0 "register_operand" "=x")
> > (vec_merge:V2DF
> > - (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "xm"))
> > + (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "x"))
> nonimmediate_operand here instead of removing 'm'
>
> Honza
Updated, bootstrapped and tested on x86_64-unknown-linux-gnu with no
regressions.
2007-08-15 Rask Ingemann Lambertsen <rask@sygehus.dk>
* config/i386/i386.md (subsi3_carry_zext): Remove "m" constraint for
"register_operand".
(*iorsi_1_zext): Likewise.
(*iorsi_1_zext_imm): Likewise.
* config/i386/sse.md: (*sse4_1_extractps): Use "nonimmediate_operand"
with "rm"/"xm" constraint.
(sse2_vmsqrtv2df2): Likewise.
Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md (revision 127481)
+++ gcc/config/i386/i386.md (working copy)
@@ -6722,11 +6722,11 @@
(set_attr "mode" "SI")])
(define_insn "subsi3_carry_zext"
- [(set (match_operand:DI 0 "register_operand" "=rm,r")
+ [(set (match_operand:DI 0 "register_operand" "=r")
(zero_extend:DI
- (minus:SI (match_operand:SI 1 "register_operand" "0,0")
+ (minus:SI (match_operand:SI 1 "register_operand" "0")
(plus:SI (match_operand:SI 3 "ix86_carry_flag_operator" "")
- (match_operand:SI 2 "general_operand" "ri,rm")))))
+ (match_operand:SI 2 "general_operand" "rim")))))
(clobber (reg:CC FLAGS_REG))]
"TARGET_64BIT && ix86_binary_operator_ok (MINUS, SImode, operands)"
"sbb{l}\t{%2, %k0|%k0, %2}"
@@ -8738,7 +8738,7 @@
;; See comment for addsi_1_zext why we do use nonimmediate_operand
(define_insn "*iorsi_1_zext"
- [(set (match_operand:DI 0 "register_operand" "=rm")
+ [(set (match_operand:DI 0 "register_operand" "=r")
(zero_extend:DI
(ior:SI (match_operand:SI 1 "nonimmediate_operand" "%0")
(match_operand:SI 2 "general_operand" "rim"))))
@@ -8749,7 +8749,7 @@
(set_attr "mode" "SI")])
(define_insn "*iorsi_1_zext_imm"
- [(set (match_operand:DI 0 "register_operand" "=rm")
+ [(set (match_operand:DI 0 "register_operand" "=r")
(ior:DI (zero_extend:DI (match_operand:SI 1 "register_operand" "%0"))
(match_operand:DI 2 "x86_64_zext_immediate_operand" "Z")))
(clobber (reg:CC FLAGS_REG))]
Index: gcc/config/i386/sse.md
===================================================================
--- gcc/config/i386/sse.md (revision 127480)
+++ gcc/config/i386/sse.md (working copy)
@@ -1532,7 +1532,7 @@
})
(define_insn "*sse4_1_extractps"
- [(set (match_operand:SF 0 "register_operand" "=rm")
+ [(set (match_operand:SF 0 "nonimmediate_operand" "=rm")
(vec_select:SF
(match_operand:V4SF 1 "register_operand" "x")
(parallel [(match_operand:SI 2 "const_0_to_3_operand" "n")])))]
@@ -1694,7 +1694,7 @@
(define_insn "sse2_vmsqrtv2df2"
[(set (match_operand:V2DF 0 "register_operand" "=x")
(vec_merge:V2DF
- (sqrt:V2DF (match_operand:V2DF 1 "register_operand" "xm"))
+ (sqrt:V2DF (match_operand:V2DF 1 "nonimmediate_operand" "xm"))
(match_operand:V2DF 2 "register_operand" "0")
(const_int 1)))]
"TARGET_SSE2"
--
Rask Ingemann Lambertsen
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] i386: Remove "m" constraint for "register_operand"
2007-08-15 10:41 ` Rask Ingemann Lambertsen
@ 2007-08-15 10:45 ` Jan Hubicka
0 siblings, 0 replies; 8+ messages in thread
From: Jan Hubicka @ 2007-08-15 10:45 UTC (permalink / raw)
To: Rask Ingemann Lambertsen; +Cc: Jan Hubicka, hongjiu.lu, gcc-patches
>
> Updated, bootstrapped and tested on x86_64-unknown-linux-gnu with no
> regressions.
>
> 2007-08-15 Rask Ingemann Lambertsen <rask@sygehus.dk>
>
> * config/i386/i386.md (subsi3_carry_zext): Remove "m" constraint for
> "register_operand".
> (*iorsi_1_zext): Likewise.
> (*iorsi_1_zext_imm): Likewise.
> * config/i386/sse.md: (*sse4_1_extractps): Use "nonimmediate_operand"
> with "rm"/"xm" constraint.
> (sse2_vmsqrtv2df2): Likewise.
OK,
thanks!
Honza
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2007-08-15 10:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-10 9:40 [PATCH] i386: Remove "m" constraint for "register_operand" Rask Ingemann Lambertsen
2007-08-10 9:53 ` Paolo Bonzini
2007-08-10 10:23 ` Hans-Peter Nilsson
2007-08-10 11:04 ` Rask Ingemann Lambertsen
2007-08-14 13:08 ` Jan Hubicka
2007-08-14 13:24 ` Lu, Hongjiu
2007-08-15 10:41 ` Rask Ingemann Lambertsen
2007-08-15 10:45 ` Jan Hubicka
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).