public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix up Yr constraint
@ 2016-05-24 18:40 Jakub Jelinek
  2016-05-24 20:47 ` Uros Bizjak
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2016-05-24 18:40 UTC (permalink / raw)
  To: Uros Bizjak, Kirill Yukhin, Ilya Enkovich; +Cc: gcc-patches

Hi!

The Yr constraint contrary to what has been said when it has been submitted
actually is always NO_REX_SSE_REGS or NO_REGS, never ALL_SSE_REGS, so
the RA restriction to only the first 8 regs is done no matter what we tune
for.

This is because we test X86_TUNE_AVOID_4BYTE_PREFIXES, which is an enum
value (59), rather than actually checking if the tune flag.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-05-24  Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.h (TARGET_AVOID_4BYTE_PREFIXES): Define.
	* config/i386/constraints.md (Yr): Test TARGET_AVOID_4BYTE_PREFIXES
	rather than X86_TUNE_AVOID_4BYTE_PREFIXES.

--- gcc/config/i386/i386.h.jj	2016-05-24 10:56:02.000000000 +0200
+++ gcc/config/i386/i386.h	2016-05-24 15:13:05.715906018 +0200
@@ -465,6 +465,8 @@ extern unsigned char ix86_tune_features[
 	ix86_tune_features[X86_TUNE_SLOW_PSHUFB]
 #define TARGET_VECTOR_PARALLEL_EXECUTION \
 	ix86_tune_features[X86_TUNE_VECTOR_PARALLEL_EXECUTION]
+#define TARGET_AVOID_4BYTE_PREFIXES \
+	ix86_tune_features[X86_TUNE_AVOID_4BYTE_PREFIXES]
 #define TARGET_FUSE_CMP_AND_BRANCH_32 \
 	ix86_tune_features[X86_TUNE_FUSE_CMP_AND_BRANCH_32]
 #define TARGET_FUSE_CMP_AND_BRANCH_64 \
--- gcc/config/i386/constraints.md.jj	2016-05-12 10:29:41.000000000 +0200
+++ gcc/config/i386/constraints.md	2016-05-24 15:14:21.647914550 +0200
@@ -142,7 +142,7 @@ (define_register_constraint "Yf"
  "@internal Any x87 register when 80387 FP arithmetic is enabled.")
 
 (define_register_constraint "Yr"
- "TARGET_SSE ? (X86_TUNE_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : ALL_SSE_REGS) : NO_REGS"
+ "TARGET_SSE ? (TARGET_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : ALL_SSE_REGS) : NO_REGS"
  "@internal Lower SSE register when avoiding REX prefix and all SSE registers otherwise.")
 
 (define_register_constraint "Yv"

	Jakub

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

* Re: [PATCH] Fix up Yr constraint
  2016-05-24 18:40 [PATCH] Fix up Yr constraint Jakub Jelinek
@ 2016-05-24 20:47 ` Uros Bizjak
  2016-05-24 21:36   ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Uros Bizjak @ 2016-05-24 20:47 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Kirill Yukhin, Ilya Enkovich, gcc-patches

On Tue, May 24, 2016 at 6:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The Yr constraint contrary to what has been said when it has been submitted
> actually is always NO_REX_SSE_REGS or NO_REGS, never ALL_SSE_REGS, so
> the RA restriction to only the first 8 regs is done no matter what we tune
> for.
>
> This is because we test X86_TUNE_AVOID_4BYTE_PREFIXES, which is an enum
> value (59), rather than actually checking if the tune flag.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-05-24  Jakub Jelinek  <jakub@redhat.com>
>
>         * config/i386/i386.h (TARGET_AVOID_4BYTE_PREFIXES): Define.
>         * config/i386/constraints.md (Yr): Test TARGET_AVOID_4BYTE_PREFIXES
>         rather than X86_TUNE_AVOID_4BYTE_PREFIXES.

Uh, another brown-paper bag bug...

OK everywhere.

Thanks,
Uros.

> --- gcc/config/i386/i386.h.jj   2016-05-24 10:56:02.000000000 +0200
> +++ gcc/config/i386/i386.h      2016-05-24 15:13:05.715906018 +0200
> @@ -465,6 +465,8 @@ extern unsigned char ix86_tune_features[
>         ix86_tune_features[X86_TUNE_SLOW_PSHUFB]
>  #define TARGET_VECTOR_PARALLEL_EXECUTION \
>         ix86_tune_features[X86_TUNE_VECTOR_PARALLEL_EXECUTION]
> +#define TARGET_AVOID_4BYTE_PREFIXES \
> +       ix86_tune_features[X86_TUNE_AVOID_4BYTE_PREFIXES]
>  #define TARGET_FUSE_CMP_AND_BRANCH_32 \
>         ix86_tune_features[X86_TUNE_FUSE_CMP_AND_BRANCH_32]
>  #define TARGET_FUSE_CMP_AND_BRANCH_64 \
> --- gcc/config/i386/constraints.md.jj   2016-05-12 10:29:41.000000000 +0200
> +++ gcc/config/i386/constraints.md      2016-05-24 15:14:21.647914550 +0200
> @@ -142,7 +142,7 @@ (define_register_constraint "Yf"
>   "@internal Any x87 register when 80387 FP arithmetic is enabled.")
>
>  (define_register_constraint "Yr"
> - "TARGET_SSE ? (X86_TUNE_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : ALL_SSE_REGS) : NO_REGS"
> + "TARGET_SSE ? (TARGET_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : ALL_SSE_REGS) : NO_REGS"
>   "@internal Lower SSE register when avoiding REX prefix and all SSE registers otherwise.")
>
>  (define_register_constraint "Yv"
>
>         Jakub

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

* Re: [PATCH] Fix up Yr constraint
  2016-05-24 20:47 ` Uros Bizjak
@ 2016-05-24 21:36   ` Jakub Jelinek
  2016-05-25  9:00     ` Uros Bizjak
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2016-05-24 21:36 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Kirill Yukhin, Ilya Enkovich, gcc-patches

On Tue, May 24, 2016 at 08:35:12PM +0200, Uros Bizjak wrote:
> On Tue, May 24, 2016 at 6:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > Hi!
> >
> > The Yr constraint contrary to what has been said when it has been submitted
> > actually is always NO_REX_SSE_REGS or NO_REGS, never ALL_SSE_REGS, so
> > the RA restriction to only the first 8 regs is done no matter what we tune
> > for.
> >
> > This is because we test X86_TUNE_AVOID_4BYTE_PREFIXES, which is an enum
> > value (59), rather than actually checking if the tune flag.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > 2016-05-24  Jakub Jelinek  <jakub@redhat.com>
> >
> >         * config/i386/i386.h (TARGET_AVOID_4BYTE_PREFIXES): Define.
> >         * config/i386/constraints.md (Yr): Test TARGET_AVOID_4BYTE_PREFIXES
> >         rather than X86_TUNE_AVOID_4BYTE_PREFIXES.
> 
> Uh, another brown-paper bag bug...
> 
> OK everywhere.

I fear it might be too dangerous for -mavx512* for the branches; I went
through all the Yr uses on the trunk, but not on the branches.
Would you be ok with using 
"TARGET_SSE ? (TARGET_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : SSE_REGS) : NO_REGS"
on the branches instead?
Or I guess we could use it on the trunk too, it should make no difference there
(because on the trunk it is only used when !TARGET_AVX).
Or maybe even
"TARGET_SSE ? ((TARGET_AVOID_4BYTE_PREFIXES && !TARGET_AVX) ? NO_REX_SSE_REGS : SSE_REGS) : NO_REGS"
(again, should make zero difference on the trunk, but might be better for
the branches).

> > --- gcc/config/i386/i386.h.jj   2016-05-24 10:56:02.000000000 +0200
> > +++ gcc/config/i386/i386.h      2016-05-24 15:13:05.715906018 +0200
> > @@ -465,6 +465,8 @@ extern unsigned char ix86_tune_features[
> >         ix86_tune_features[X86_TUNE_SLOW_PSHUFB]
> >  #define TARGET_VECTOR_PARALLEL_EXECUTION \
> >         ix86_tune_features[X86_TUNE_VECTOR_PARALLEL_EXECUTION]
> > +#define TARGET_AVOID_4BYTE_PREFIXES \
> > +       ix86_tune_features[X86_TUNE_AVOID_4BYTE_PREFIXES]
> >  #define TARGET_FUSE_CMP_AND_BRANCH_32 \
> >         ix86_tune_features[X86_TUNE_FUSE_CMP_AND_BRANCH_32]
> >  #define TARGET_FUSE_CMP_AND_BRANCH_64 \
> > --- gcc/config/i386/constraints.md.jj   2016-05-12 10:29:41.000000000 +0200
> > +++ gcc/config/i386/constraints.md      2016-05-24 15:14:21.647914550 +0200
> > @@ -142,7 +142,7 @@ (define_register_constraint "Yf"
> >   "@internal Any x87 register when 80387 FP arithmetic is enabled.")
> >
> >  (define_register_constraint "Yr"
> > - "TARGET_SSE ? (X86_TUNE_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : ALL_SSE_REGS) : NO_REGS"
> > + "TARGET_SSE ? (TARGET_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : ALL_SSE_REGS) : NO_REGS"
> >   "@internal Lower SSE register when avoiding REX prefix and all SSE registers otherwise.")
> >
> >  (define_register_constraint "Yv"

	Jakub

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

* Re: [PATCH] Fix up Yr constraint
  2016-05-24 21:36   ` Jakub Jelinek
@ 2016-05-25  9:00     ` Uros Bizjak
  0 siblings, 0 replies; 4+ messages in thread
From: Uros Bizjak @ 2016-05-25  9:00 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Kirill Yukhin, Ilya Enkovich, gcc-patches

On Tue, May 24, 2016 at 9:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, May 24, 2016 at 08:35:12PM +0200, Uros Bizjak wrote:
>> On Tue, May 24, 2016 at 6:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > Hi!
>> >
>> > The Yr constraint contrary to what has been said when it has been submitted
>> > actually is always NO_REX_SSE_REGS or NO_REGS, never ALL_SSE_REGS, so
>> > the RA restriction to only the first 8 regs is done no matter what we tune
>> > for.
>> >
>> > This is because we test X86_TUNE_AVOID_4BYTE_PREFIXES, which is an enum
>> > value (59), rather than actually checking if the tune flag.
>> >
>> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>> >
>> > 2016-05-24  Jakub Jelinek  <jakub@redhat.com>
>> >
>> >         * config/i386/i386.h (TARGET_AVOID_4BYTE_PREFIXES): Define.
>> >         * config/i386/constraints.md (Yr): Test TARGET_AVOID_4BYTE_PREFIXES
>> >         rather than X86_TUNE_AVOID_4BYTE_PREFIXES.
>>
>> Uh, another brown-paper bag bug...
>>
>> OK everywhere.
>
> I fear it might be too dangerous for -mavx512* for the branches; I went
> through all the Yr uses on the trunk, but not on the branches.
> Would you be ok with using
> "TARGET_SSE ? (TARGET_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS : SSE_REGS) : NO_REGS"
> on the branches instead?
> Or I guess we could use it on the trunk too, it should make no difference there
> (because on the trunk it is only used when !TARGET_AVX).
> Or maybe even
> "TARGET_SSE ? ((TARGET_AVOID_4BYTE_PREFIXES && !TARGET_AVX) ? NO_REX_SSE_REGS : SSE_REGS) : NO_REGS"
> (again, should make zero difference on the trunk, but might be better for
> the branches).

Indeed, let's play safe and go with the later version on branches.
Please also add a small comment, to avoid head-scratching in the
future.

Uros.

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

end of thread, other threads:[~2016-05-25  6:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-24 18:40 [PATCH] Fix up Yr constraint Jakub Jelinek
2016-05-24 20:47 ` Uros Bizjak
2016-05-24 21:36   ` Jakub Jelinek
2016-05-25  9:00     ` Uros Bizjak

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).