* [PATCH] Fix pr67963 @ 2015-10-14 15:08 Yulia Koval 2015-10-14 15:15 ` H.J. Lu 2015-10-15 11:45 ` Uros Bizjak 0 siblings, 2 replies; 14+ messages in thread From: Yulia Koval @ 2015-10-14 15:08 UTC (permalink / raw) To: GCC Patches, H.J. Lu, Uros Bizjak [-- Attachment #1: Type: text/plain, Size: 733 bytes --] Hi, This patch fixes the issue: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 gcc/config/i386/i386.c (ix86_option_override_internal) Disable 80387 mask if lakemont target is set. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 4c25c9e..db722aa 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, break; } + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) + { + opts->x_target_flags &= ~MASK_80387; + opts_set->x_target_flags |= MASK_80387; + } + if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) error ("Intel MPX does not support x32"); Ok for trunk? Yulia [-- Attachment #2: patch --] [-- Type: application/octet-stream, Size: 521 bytes --] diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 4c25c9e..db722aa 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, break; } + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) + { + opts->x_target_flags &= ~MASK_80387; + opts_set->x_target_flags |= MASK_80387; + } + if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) error ("Intel MPX does not support x32"); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-14 15:08 [PATCH] Fix pr67963 Yulia Koval @ 2015-10-14 15:15 ` H.J. Lu 2015-10-14 15:17 ` H.J. Lu 2015-10-15 11:45 ` Uros Bizjak 1 sibling, 1 reply; 14+ messages in thread From: H.J. Lu @ 2015-10-14 15:15 UTC (permalink / raw) To: Yulia Koval; +Cc: GCC Patches, Uros Bizjak On Wed, Oct 14, 2015 at 8:08 AM, Yulia Koval <vaalfreja@gmail.com> wrote: > Hi, > > This patch fixes the issue: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 > > gcc/config/i386/i386.c (ix86_option_override_internal) Disable > 80387 mask if lakemont target is set. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 4c25c9e..db722aa 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, > break; > } > > + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) > + { > + opts->x_target_flags &= ~MASK_80387; > + opts_set->x_target_flags |= MASK_80387; > + } > + > if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) > error ("Intel MPX does not support x32"); > > Ok for trunk? We should add a bit to "struct pta" to indicate availability of 80387 ISA and turn it off for lakemount if 90387 ISA hasn't be turned on explicitly. We also need some testcases. -- H.J. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-14 15:15 ` H.J. Lu @ 2015-10-14 15:17 ` H.J. Lu 2015-10-15 7:53 ` Uros Bizjak 0 siblings, 1 reply; 14+ messages in thread From: H.J. Lu @ 2015-10-14 15:17 UTC (permalink / raw) To: Yulia Koval; +Cc: GCC Patches, Uros Bizjak On Wed, Oct 14, 2015 at 8:15 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Oct 14, 2015 at 8:08 AM, Yulia Koval <vaalfreja@gmail.com> wrote: >> Hi, >> >> This patch fixes the issue: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 >> >> gcc/config/i386/i386.c (ix86_option_override_internal) Disable >> 80387 mask if lakemont target is set. >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 4c25c9e..db722aa 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, >> break; >> } >> >> + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) >> + { >> + opts->x_target_flags &= ~MASK_80387; >> + opts_set->x_target_flags |= MASK_80387; >> + } >> + >> if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) >> error ("Intel MPX does not support x32"); >> >> Ok for trunk? > > We should add a bit to "struct pta" to indicate availability of > 80387 ISA and turn it off for lakemount if 90387 ISA hasn't be > turned on explicitly. Something like diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index a2314e7..1cea58e 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -4348,6 +4348,7 @@ ix86_option_override_internal (bool main_args_p, const enum processor_type processor; const enum attr_cpu schedule; const unsigned HOST_WIDE_INT flags; + const unsigned HOST_WIDE_INT mask; } const processor_alias_table[] = { > We also need some testcases. > > -- > H.J. -- H.J. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-14 15:17 ` H.J. Lu @ 2015-10-15 7:53 ` Uros Bizjak 0 siblings, 0 replies; 14+ messages in thread From: Uros Bizjak @ 2015-10-15 7:53 UTC (permalink / raw) To: H.J. Lu; +Cc: Yulia Koval, GCC Patches On Wed, Oct 14, 2015 at 5:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Oct 14, 2015 at 8:15 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Wed, Oct 14, 2015 at 8:08 AM, Yulia Koval <vaalfreja@gmail.com> wrote: >>> Hi, >>> >>> This patch fixes the issue: >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 >>> >>> gcc/config/i386/i386.c (ix86_option_override_internal) Disable >>> 80387 mask if lakemont target is set. >>> >>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >>> index 4c25c9e..db722aa 100644 >>> --- a/gcc/config/i386/i386.c >>> +++ b/gcc/config/i386/i386.c >>> @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, >>> break; >>> } >>> >>> + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) >>> + { >>> + opts->x_target_flags &= ~MASK_80387; >>> + opts_set->x_target_flags |= MASK_80387; >>> + } >>> + >>> if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) >>> error ("Intel MPX does not support x32"); >>> >>> Ok for trunk? >> >> We should add a bit to "struct pta" to indicate availability of >> 80387 ISA and turn it off for lakemount if 90387 ISA hasn't be >> turned on explicitly. > > Something like Hm, I don't think introducing another flag is a good idea. Better move -m80387 (together with -msof-float and -mhard-float) to ISA flags and add corresponding PTA_80387 define. Uros. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-14 15:08 [PATCH] Fix pr67963 Yulia Koval 2015-10-14 15:15 ` H.J. Lu @ 2015-10-15 11:45 ` Uros Bizjak 2015-10-15 16:57 ` Uros Bizjak 1 sibling, 1 reply; 14+ messages in thread From: Uros Bizjak @ 2015-10-15 11:45 UTC (permalink / raw) To: Yulia Koval; +Cc: GCC Patches, H.J. Lu On Wed, Oct 14, 2015 at 5:08 PM, Yulia Koval <vaalfreja@gmail.com> wrote: > Hi, > > This patch fixes the issue: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 > > gcc/config/i386/i386.c (ix86_option_override_internal) Disable > 80387 mask if lakemont target is set. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 4c25c9e..db722aa 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, > break; > } > > + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) > + { > + opts->x_target_flags &= ~MASK_80387; > + opts_set->x_target_flags |= MASK_80387; > + } > + > if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) > error ("Intel MPX does not support x32"); > > Ok for trunk? The problem is in TARGET_SUBTARGET{32,64}_DEFAULT, that will override set target flags, unless relevant bit of opts_set->x_target_flags is set. However, we can't just set x_target_flag, because __attribute__((target("arch=...."))) won't work. Unfortunately, my proposed patch in the PR violates this last requirement. This can probably be solved by adding local "x_target_flags_mask" and use it after TARGET_SUBTARGET processing. Uros. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-15 11:45 ` Uros Bizjak @ 2015-10-15 16:57 ` Uros Bizjak 2015-10-15 16:59 ` H.J. Lu 0 siblings, 1 reply; 14+ messages in thread From: Uros Bizjak @ 2015-10-15 16:57 UTC (permalink / raw) To: Yulia Koval; +Cc: GCC Patches, H.J. Lu [-- Attachment #1: Type: text/plain, Size: 1976 bytes --] On Thu, Oct 15, 2015 at 1:45 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Wed, Oct 14, 2015 at 5:08 PM, Yulia Koval <vaalfreja@gmail.com> wrote: >> Hi, >> >> This patch fixes the issue: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 >> >> gcc/config/i386/i386.c (ix86_option_override_internal) Disable >> 80387 mask if lakemont target is set. >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 4c25c9e..db722aa 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, >> break; >> } >> >> + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) >> + { >> + opts->x_target_flags &= ~MASK_80387; >> + opts_set->x_target_flags |= MASK_80387; >> + } >> + >> if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) >> error ("Intel MPX does not support x32"); >> >> Ok for trunk? > > The problem is in TARGET_SUBTARGET{32,64}_DEFAULT, that will override > set target flags, unless relevant bit of opts_set->x_target_flags is > set. However, we can't just set x_target_flag, because > __attribute__((target("arch=...."))) won't work. Unfortunately, my > proposed patch in the PR violates this last requirement. > > This can probably be solved by adding local "x_target_flags_mask" and > use it after TARGET_SUBTARGET processing. Attached is a final patch I plan to commit to mainline soon. 2015-10-15 Uros Bizjak <ubizjak@gmail.com> PR target/67963 * config/i386/i386.c (ix86_option_override_internal): Add PTA_NO_80387. Add PTA_NO_80387 to "lakemont". Disable MASK_80387 target flag after target flags were initialized to target defaults. testsuite/ChangeLog: 2015-10-15 Uros Bizjak <ubizjak@gmail.com> PR target/67963 * gcc.target/i386/pr67963-1.c: New test. * gcc.target/i386/pr67963-2.c: Ditto. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Uros. [-- Attachment #2: p.diff.txt --] [-- Type: text/plain, Size: 3040 bytes --] Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 228843) +++ config/i386/i386.c (working copy) @@ -4247,6 +4247,7 @@ ix86_option_override_internal (bool main_args_p, { int i; unsigned int ix86_arch_mask; + int ix86_target_flags_mask = 0; const bool ix86_tune_specified = (opts->x_ix86_tune_string != NULL); const char *prefix; const char *suffix; @@ -4311,6 +4312,7 @@ ix86_option_override_internal (bool main_args_p, #define PTA_PCOMMIT (HOST_WIDE_INT_1 << 56) #define PTA_MWAITX (HOST_WIDE_INT_1 << 57) #define PTA_CLZERO (HOST_WIDE_INT_1 << 58) +#define PTA_NO_80387 (HOST_WIDE_INT_1 << 59) #define PTA_CORE2 \ (PTA_64BIT | PTA_MMX | PTA_SSE | PTA_SSE2 | PTA_SSE3 | PTA_SSSE3 \ @@ -4355,7 +4357,7 @@ ix86_option_override_internal (bool main_args_p, {"i486", PROCESSOR_I486, CPU_NONE, 0}, {"i586", PROCESSOR_PENTIUM, CPU_PENTIUM, 0}, {"pentium", PROCESSOR_PENTIUM, CPU_PENTIUM, 0}, - {"lakemont", PROCESSOR_LAKEMONT, CPU_PENTIUM, 0}, + {"lakemont", PROCESSOR_LAKEMONT, CPU_PENTIUM, PTA_NO_80387}, {"pentium-mmx", PROCESSOR_PENTIUM, CPU_PENTIUM, PTA_MMX}, {"winchip-c6", PROCESSOR_I486, CPU_NONE, PTA_MMX}, {"winchip2", PROCESSOR_I486, CPU_NONE, PTA_MMX | PTA_3DNOW | PTA_PRFCHW}, @@ -4935,7 +4937,9 @@ ix86_option_override_internal (bool main_args_p, if (processor_alias_table[i].flags & PTA_MWAITX && !(opts->x_ix86_isa_flags_explicit & OPTION_MASK_ISA_MWAITX)) opts->x_ix86_isa_flags |= OPTION_MASK_ISA_MWAITX; - + if (processor_alias_table[i].flags & PTA_NO_80387 + && !(opts_set->x_target_flags & MASK_80387)) + ix86_target_flags_mask |= MASK_80387; break; } @@ -5126,6 +5130,9 @@ ix86_option_override_internal (bool main_args_p, opts->x_target_flags |= MASK_NO_RED_ZONE; } + /* Disable specific processor dependant features after target defaults. */ + opts->x_target_flags &= ~ix86_target_flags_mask; + /* Keep nonleaf frame pointers. */ if (opts->x_flag_omit_frame_pointer) opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER; Index: testsuite/gcc.target/i386/pr67963-1.c =================================================================== --- testsuite/gcc.target/i386/pr67963-1.c (revision 0) +++ testsuite/gcc.target/i386/pr67963-1.c (working copy) @@ -0,0 +1,9 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-O2 -march=lakemont" } */ + +float foo (void) +{ + return 0.0f; +} + +/* { dg-final { scan-assembler-not "fldz" } } */ Index: testsuite/gcc.target/i386/pr67963-2.c =================================================================== --- testsuite/gcc.target/i386/pr67963-2.c (revision 0) +++ testsuite/gcc.target/i386/pr67963-2.c (working copy) @@ -0,0 +1,11 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-O2 -march=pentium" } */ + +float +__attribute__((target("arch=lakemont"))) +foo (void) +{ + return 0.0f; +} + +/* { dg-final { scan-assembler-not "fldz" } } */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-15 16:57 ` Uros Bizjak @ 2015-10-15 16:59 ` H.J. Lu 2015-10-15 19:16 ` Uros Bizjak 0 siblings, 1 reply; 14+ messages in thread From: H.J. Lu @ 2015-10-15 16:59 UTC (permalink / raw) To: Uros Bizjak; +Cc: Yulia Koval, GCC Patches On Thu, Oct 15, 2015 at 9:57 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Thu, Oct 15, 2015 at 1:45 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Wed, Oct 14, 2015 at 5:08 PM, Yulia Koval <vaalfreja@gmail.com> wrote: >>> Hi, >>> >>> This patch fixes the issue: >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 >>> >>> gcc/config/i386/i386.c (ix86_option_override_internal) Disable >>> 80387 mask if lakemont target is set. >>> >>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >>> index 4c25c9e..db722aa 100644 >>> --- a/gcc/config/i386/i386.c >>> +++ b/gcc/config/i386/i386.c >>> @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, >>> break; >>> } >>> >>> + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) >>> + { >>> + opts->x_target_flags &= ~MASK_80387; >>> + opts_set->x_target_flags |= MASK_80387; >>> + } >>> + >>> if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) >>> error ("Intel MPX does not support x32"); >>> >>> Ok for trunk? >> >> The problem is in TARGET_SUBTARGET{32,64}_DEFAULT, that will override >> set target flags, unless relevant bit of opts_set->x_target_flags is >> set. However, we can't just set x_target_flag, because >> __attribute__((target("arch=...."))) won't work. Unfortunately, my >> proposed patch in the PR violates this last requirement. >> >> This can probably be solved by adding local "x_target_flags_mask" and >> use it after TARGET_SUBTARGET processing. > > Attached is a final patch I plan to commit to mainline soon. > > 2015-10-15 Uros Bizjak <ubizjak@gmail.com> > > PR target/67963 > * config/i386/i386.c (ix86_option_override_internal): Add PTA_NO_80387. > Add PTA_NO_80387 to "lakemont". Disable MASK_80387 target flag > after target flags were initialized to target defaults. > > testsuite/ChangeLog: > > 2015-10-15 Uros Bizjak <ubizjak@gmail.com> > > PR target/67963 > * gcc.target/i386/pr67963-1.c: New test. > * gcc.target/i386/pr67963-2.c: Ditto. > > Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. > > Uros. Do we support -O2 -march=lakemont with __attribute__((target("arch=silvermont"))) -- H.J. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-15 16:59 ` H.J. Lu @ 2015-10-15 19:16 ` Uros Bizjak 2015-10-15 19:21 ` H.J. Lu 0 siblings, 1 reply; 14+ messages in thread From: Uros Bizjak @ 2015-10-15 19:16 UTC (permalink / raw) To: H.J. Lu; +Cc: Yulia Koval, GCC Patches On Thu, Oct 15, 2015 at 6:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Oct 15, 2015 at 9:57 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Thu, Oct 15, 2015 at 1:45 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> On Wed, Oct 14, 2015 at 5:08 PM, Yulia Koval <vaalfreja@gmail.com> wrote: >>>> Hi, >>>> >>>> This patch fixes the issue: >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 >>>> >>>> gcc/config/i386/i386.c (ix86_option_override_internal) Disable >>>> 80387 mask if lakemont target is set. >>>> >>>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >>>> index 4c25c9e..db722aa 100644 >>>> --- a/gcc/config/i386/i386.c >>>> +++ b/gcc/config/i386/i386.c >>>> @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, >>>> break; >>>> } >>>> >>>> + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) >>>> + { >>>> + opts->x_target_flags &= ~MASK_80387; >>>> + opts_set->x_target_flags |= MASK_80387; >>>> + } >>>> + >>>> if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) >>>> error ("Intel MPX does not support x32"); >>>> >>>> Ok for trunk? >>> >>> The problem is in TARGET_SUBTARGET{32,64}_DEFAULT, that will override >>> set target flags, unless relevant bit of opts_set->x_target_flags is >>> set. However, we can't just set x_target_flag, because >>> __attribute__((target("arch=...."))) won't work. Unfortunately, my >>> proposed patch in the PR violates this last requirement. >>> >>> This can probably be solved by adding local "x_target_flags_mask" and >>> use it after TARGET_SUBTARGET processing. >> >> Attached is a final patch I plan to commit to mainline soon. >> >> 2015-10-15 Uros Bizjak <ubizjak@gmail.com> >> >> PR target/67963 >> * config/i386/i386.c (ix86_option_override_internal): Add PTA_NO_80387. >> Add PTA_NO_80387 to "lakemont". Disable MASK_80387 target flag >> after target flags were initialized to target defaults. >> >> testsuite/ChangeLog: >> >> 2015-10-15 Uros Bizjak <ubizjak@gmail.com> >> >> PR target/67963 >> * gcc.target/i386/pr67963-1.c: New test. >> * gcc.target/i386/pr67963-2.c: Ditto. >> >> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. >> >> Uros. > > Do we support -O2 -march=lakemont with > > __attribute__((target("arch=silvermont"))) Hm, no. Uros. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-15 19:16 ` Uros Bizjak @ 2015-10-15 19:21 ` H.J. Lu 2015-10-15 19:31 ` Uros Bizjak 0 siblings, 1 reply; 14+ messages in thread From: H.J. Lu @ 2015-10-15 19:21 UTC (permalink / raw) To: Uros Bizjak; +Cc: Yulia Koval, GCC Patches On Thu, Oct 15, 2015 at 12:16 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Thu, Oct 15, 2015 at 6:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Oct 15, 2015 at 9:57 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> On Thu, Oct 15, 2015 at 1:45 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >>>> On Wed, Oct 14, 2015 at 5:08 PM, Yulia Koval <vaalfreja@gmail.com> wrote: >>>>> Hi, >>>>> >>>>> This patch fixes the issue: >>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 >>>>> >>>>> gcc/config/i386/i386.c (ix86_option_override_internal) Disable >>>>> 80387 mask if lakemont target is set. >>>>> >>>>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >>>>> index 4c25c9e..db722aa 100644 >>>>> --- a/gcc/config/i386/i386.c >>>>> +++ b/gcc/config/i386/i386.c >>>>> @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, >>>>> break; >>>>> } >>>>> >>>>> + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) >>>>> + { >>>>> + opts->x_target_flags &= ~MASK_80387; >>>>> + opts_set->x_target_flags |= MASK_80387; >>>>> + } >>>>> + >>>>> if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) >>>>> error ("Intel MPX does not support x32"); >>>>> >>>>> Ok for trunk? >>>> >>>> The problem is in TARGET_SUBTARGET{32,64}_DEFAULT, that will override >>>> set target flags, unless relevant bit of opts_set->x_target_flags is >>>> set. However, we can't just set x_target_flag, because >>>> __attribute__((target("arch=...."))) won't work. Unfortunately, my >>>> proposed patch in the PR violates this last requirement. >>>> >>>> This can probably be solved by adding local "x_target_flags_mask" and >>>> use it after TARGET_SUBTARGET processing. >>> >>> Attached is a final patch I plan to commit to mainline soon. >>> >>> 2015-10-15 Uros Bizjak <ubizjak@gmail.com> >>> >>> PR target/67963 >>> * config/i386/i386.c (ix86_option_override_internal): Add PTA_NO_80387. >>> Add PTA_NO_80387 to "lakemont". Disable MASK_80387 target flag >>> after target flags were initialized to target defaults. >>> >>> testsuite/ChangeLog: >>> >>> 2015-10-15 Uros Bizjak <ubizjak@gmail.com> >>> >>> PR target/67963 >>> * gcc.target/i386/pr67963-1.c: New test. >>> * gcc.target/i386/pr67963-2.c: Ditto. >>> >>> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. >>> >>> Uros. >> >> Do we support -O2 -march=lakemont with >> >> __attribute__((target("arch=silvermont"))) > > Hm, no. > Do we issue an error or silently ignore __attribute__((target("arch=silvermont")))? If we don't support it, should we support -O2 -march=silvermont __attribute__((target("arch=lakemont"))) -- H.J. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-15 19:21 ` H.J. Lu @ 2015-10-15 19:31 ` Uros Bizjak 2015-10-16 6:45 ` Uros Bizjak 0 siblings, 1 reply; 14+ messages in thread From: Uros Bizjak @ 2015-10-15 19:31 UTC (permalink / raw) To: H.J. Lu; +Cc: Yulia Koval, GCC Patches [-- Attachment #1: Type: text/plain, Size: 3059 bytes --] On Thu, Oct 15, 2015 at 9:21 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Oct 15, 2015 at 12:16 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Thu, Oct 15, 2015 at 6:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Thu, Oct 15, 2015 at 9:57 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>>> On Thu, Oct 15, 2015 at 1:45 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >>>>> On Wed, Oct 14, 2015 at 5:08 PM, Yulia Koval <vaalfreja@gmail.com> wrote: >>>>>> Hi, >>>>>> >>>>>> This patch fixes the issue: >>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67963 >>>>>> >>>>>> gcc/config/i386/i386.c (ix86_option_override_internal) Disable >>>>>> 80387 mask if lakemont target is set. >>>>>> >>>>>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >>>>>> index 4c25c9e..db722aa 100644 >>>>>> --- a/gcc/config/i386/i386.c >>>>>> +++ b/gcc/config/i386/i386.c >>>>>> @@ -4943,6 +4943,12 @@ ix86_option_override_internal (bool main_args_p, >>>>>> break; >>>>>> } >>>>>> >>>>>> + if (!strcmp (opts->x_ix86_arch_string, "lakemont")) >>>>>> + { >>>>>> + opts->x_target_flags &= ~MASK_80387; >>>>>> + opts_set->x_target_flags |= MASK_80387; >>>>>> + } >>>>>> + >>>>>> if (TARGET_X32 && (opts->x_ix86_isa_flags & OPTION_MASK_ISA_MPX)) >>>>>> error ("Intel MPX does not support x32"); >>>>>> >>>>>> Ok for trunk? >>>>> >>>>> The problem is in TARGET_SUBTARGET{32,64}_DEFAULT, that will override >>>>> set target flags, unless relevant bit of opts_set->x_target_flags is >>>>> set. However, we can't just set x_target_flag, because >>>>> __attribute__((target("arch=...."))) won't work. Unfortunately, my >>>>> proposed patch in the PR violates this last requirement. >>>>> >>>>> This can probably be solved by adding local "x_target_flags_mask" and >>>>> use it after TARGET_SUBTARGET processing. >>>> >>>> Attached is a final patch I plan to commit to mainline soon. >>>> >>>> 2015-10-15 Uros Bizjak <ubizjak@gmail.com> >>>> >>>> PR target/67963 >>>> * config/i386/i386.c (ix86_option_override_internal): Add PTA_NO_80387. >>>> Add PTA_NO_80387 to "lakemont". Disable MASK_80387 target flag >>>> after target flags were initialized to target defaults. >>>> >>>> testsuite/ChangeLog: >>>> >>>> 2015-10-15 Uros Bizjak <ubizjak@gmail.com> >>>> >>>> PR target/67963 >>>> * gcc.target/i386/pr67963-1.c: New test. >>>> * gcc.target/i386/pr67963-2.c: Ditto. >>>> >>>> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. >>>> >>>> Uros. >>> >>> Do we support -O2 -march=lakemont with >>> >>> __attribute__((target("arch=silvermont"))) >> >> Hm, no. >> > > Do we issue an error or silently ignore > __attribute__((target("arch=silvermont")))? > If we don't support it, should we support > > -O2 -march=silvermont > > __attribute__((target("arch=lakemont"))) Actually, we have to re-initialize: opts->x_target_flags |= (TARGET_DEFAULT | TARGET_SUBTARGET_DEFAULT) & ~opts_set->x_target_flags; just before TARGET_SUBTARGET{32,64}_DEFAULT processing, and it will work. Uros. [-- Attachment #2: p.diff.txt --] [-- Type: text/plain, Size: 3402 bytes --] Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 228843) +++ config/i386/i386.c (working copy) @@ -4247,6 +4247,7 @@ ix86_option_override_internal (bool main_args_p, { int i; unsigned int ix86_arch_mask; + int ix86_target_flags_mask = 0; const bool ix86_tune_specified = (opts->x_ix86_tune_string != NULL); const char *prefix; const char *suffix; @@ -4311,6 +4312,7 @@ ix86_option_override_internal (bool main_args_p, #define PTA_PCOMMIT (HOST_WIDE_INT_1 << 56) #define PTA_MWAITX (HOST_WIDE_INT_1 << 57) #define PTA_CLZERO (HOST_WIDE_INT_1 << 58) +#define PTA_NO_80387 (HOST_WIDE_INT_1 << 59) #define PTA_CORE2 \ (PTA_64BIT | PTA_MMX | PTA_SSE | PTA_SSE2 | PTA_SSE3 | PTA_SSSE3 \ @@ -4355,7 +4357,7 @@ ix86_option_override_internal (bool main_args_p, {"i486", PROCESSOR_I486, CPU_NONE, 0}, {"i586", PROCESSOR_PENTIUM, CPU_PENTIUM, 0}, {"pentium", PROCESSOR_PENTIUM, CPU_PENTIUM, 0}, - {"lakemont", PROCESSOR_LAKEMONT, CPU_PENTIUM, 0}, + {"lakemont", PROCESSOR_LAKEMONT, CPU_PENTIUM, PTA_NO_80387}, {"pentium-mmx", PROCESSOR_PENTIUM, CPU_PENTIUM, PTA_MMX}, {"winchip-c6", PROCESSOR_I486, CPU_NONE, PTA_MMX}, {"winchip2", PROCESSOR_I486, CPU_NONE, PTA_MMX | PTA_3DNOW | PTA_PRFCHW}, @@ -4935,7 +4937,9 @@ ix86_option_override_internal (bool main_args_p, if (processor_alias_table[i].flags & PTA_MWAITX && !(opts->x_ix86_isa_flags_explicit & OPTION_MASK_ISA_MWAITX)) opts->x_ix86_isa_flags |= OPTION_MASK_ISA_MWAITX; - + if (processor_alias_table[i].flags & PTA_NO_80387 + && !(opts_set->x_target_flags & MASK_80387)) + ix86_target_flags_mask |= MASK_80387; break; } @@ -5094,6 +5098,9 @@ ix86_option_override_internal (bool main_args_p, if (!opts_set->x_ix86_branch_cost) opts->x_ix86_branch_cost = ix86_tune_cost->branch_cost; + opts->x_target_flags + |= (TARGET_DEFAULT | TARGET_SUBTARGET_DEFAULT) & ~opts_set->x_target_flags; + if (TARGET_64BIT_P (opts->x_ix86_isa_flags)) { opts->x_target_flags @@ -5126,6 +5133,9 @@ ix86_option_override_internal (bool main_args_p, opts->x_target_flags |= MASK_NO_RED_ZONE; } + /* Disable specific processor dependant features after target defaults. */ + opts->x_target_flags &= ~ix86_target_flags_mask; + /* Keep nonleaf frame pointers. */ if (opts->x_flag_omit_frame_pointer) opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER; Index: testsuite/gcc.target/i386/pr67963-1.c =================================================================== --- testsuite/gcc.target/i386/pr67963-1.c (revision 0) +++ testsuite/gcc.target/i386/pr67963-1.c (working copy) @@ -0,0 +1,9 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-O2 -march=lakemont" } */ + +float foo (void) +{ + return 0.0f; +} + +/* { dg-final { scan-assembler-not "fldz" } } */ Index: testsuite/gcc.target/i386/pr67963-2.c =================================================================== --- testsuite/gcc.target/i386/pr67963-2.c (revision 0) +++ testsuite/gcc.target/i386/pr67963-2.c (working copy) @@ -0,0 +1,11 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-O2 -march=pentium" } */ + +float +__attribute__((target("arch=lakemont"))) +foo (void) +{ + return 0.0f; +} + +/* { dg-final { scan-assembler-not "fldz" } } */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-15 19:31 ` Uros Bizjak @ 2015-10-16 6:45 ` Uros Bizjak 2015-10-16 9:37 ` Uros Bizjak 0 siblings, 1 reply; 14+ messages in thread From: Uros Bizjak @ 2015-10-16 6:45 UTC (permalink / raw) To: H.J. Lu; +Cc: Yulia Koval, GCC Patches On Thu, Oct 15, 2015 at 9:30 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >>>> Do we support -O2 -march=lakemont with >>>> >>>> __attribute__((target("arch=silvermont"))) >>> >>> Hm, no. >>> >> >> Do we issue an error or silently ignore >> __attribute__((target("arch=silvermont")))? >> If we don't support it, should we support >> >> -O2 -march=silvermont >> >> __attribute__((target("arch=lakemont"))) > > Actually, we have to re-initialize: > > opts->x_target_flags > |= (TARGET_DEFAULT | TARGET_SUBTARGET_DEFAULT) & ~opts_set->x_target_flags; > > just before TARGET_SUBTARGET{32,64}_DEFAULT processing, and it will work. No, this won't work. The value of MASK_NO_FANCY_MATH depend on MASK_80387setting, and once fancy math bit is set, it couldn't be cleared for march != lakemont. It looks just like we want to error out when lakemont is enabled with -m80387. Uros. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-16 6:45 ` Uros Bizjak @ 2015-10-16 9:37 ` Uros Bizjak 2015-10-16 10:33 ` H.J. Lu 0 siblings, 1 reply; 14+ messages in thread From: Uros Bizjak @ 2015-10-16 9:37 UTC (permalink / raw) To: H.J. Lu; +Cc: Yulia Koval, GCC Patches [-- Attachment #1: Type: text/plain, Size: 1067 bytes --] On Fri, Oct 16, 2015 at 8:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Thu, Oct 15, 2015 at 9:30 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > >>>>> Do we support -O2 -march=lakemont with >>>>> >>>>> __attribute__((target("arch=silvermont"))) >>>> >>>> Hm, no. >>>> >>> >>> Do we issue an error or silently ignore >>> __attribute__((target("arch=silvermont")))? >>> If we don't support it, should we support >>> >>> -O2 -march=silvermont >>> >>> __attribute__((target("arch=lakemont"))) >> >> Actually, we have to re-initialize: >> >> opts->x_target_flags >> |= (TARGET_DEFAULT | TARGET_SUBTARGET_DEFAULT) & ~opts_set->x_target_flags; >> >> just before TARGET_SUBTARGET{32,64}_DEFAULT processing, and it will work. > > No, this won't work. The value of MASK_NO_FANCY_MATH depend on > MASK_80387setting, and once fancy math bit is set, it couldn't be > cleared for march != lakemont. > > It looks just like we want to error out when lakemont is enabled with -m80387. Like in the attached patch, that also slightly improves existing error reporting. Uros. [-- Attachment #2: p.diff.txt --] [-- Type: text/plain, Size: 1778 bytes --] Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 228863) +++ config/i386/i386.c (working copy) @@ -4949,16 +4949,30 @@ ix86_option_override_internal (bool main_args_p, { /* Verify that x87/MMX/SSE/AVX is off for -miamcu. */ if (TARGET_80387_P (opts->x_target_flags)) - sorry ("X87 FPU isn%'t supported in Intel MCU psABI"); - else if ((opts->x_ix86_isa_flags & (OPTION_MASK_ISA_MMX - | OPTION_MASK_ISA_SSE - | OPTION_MASK_ISA_AVX))) - sorry ("%s isn%'t supported in Intel MCU psABI", - TARGET_MMX_P (opts->x_ix86_isa_flags) - ? "MMX" - : TARGET_SSE_P (opts->x_ix86_isa_flags) ? "SSE" : "AVX"); + sorry ("X87 FPU is not supported in Intel MCU psABI"); + if ((opts->x_ix86_isa_flags & (OPTION_MASK_ISA_MMX + | OPTION_MASK_ISA_SSE + | OPTION_MASK_ISA_AVX))) + sorry ("%s is not supported in Intel MCU psABI", + TARGET_AVX_P (opts->x_ix86_isa_flags) + ? "AVX" + : TARGET_SSE_P (opts->x_ix86_isa_flags) ? "SSE" : "MMX"); } + if (ix86_arch == PROCESSOR_LAKEMONT) + { + /* Verify that x87/MMX/SSE/AVX is off for -march=lakemont. */ + if (TARGET_80387_P (opts->x_target_flags)) + error ("X87 FPU is not supported on Lakemont CPU"); + if ((opts->x_ix86_isa_flags & (OPTION_MASK_ISA_MMX + | OPTION_MASK_ISA_SSE + | OPTION_MASK_ISA_AVX))) + error ("%s is not supported on Lakemont CPU", + TARGET_AVX_P (opts->x_ix86_isa_flags) + ? "AVX" + : TARGET_SSE_P (opts->x_ix86_isa_flags) ? "SSE" : "MMX"); + } + if (!strcmp (opts->x_ix86_arch_string, "generic")) error ("generic CPU can be used only for %stune=%s %s", prefix, suffix, sw); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-16 9:37 ` Uros Bizjak @ 2015-10-16 10:33 ` H.J. Lu 2015-10-16 10:45 ` H.J. Lu 0 siblings, 1 reply; 14+ messages in thread From: H.J. Lu @ 2015-10-16 10:33 UTC (permalink / raw) To: Uros Bizjak; +Cc: Yulia Koval, GCC Patches On Fri, Oct 16, 2015 at 2:35 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Fri, Oct 16, 2015 at 8:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Thu, Oct 15, 2015 at 9:30 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >> >>>>>> Do we support -O2 -march=lakemont with >>>>>> >>>>>> __attribute__((target("arch=silvermont"))) >>>>> >>>>> Hm, no. >>>>> >>>> >>>> Do we issue an error or silently ignore >>>> __attribute__((target("arch=silvermont")))? >>>> If we don't support it, should we support >>>> >>>> -O2 -march=silvermont >>>> >>>> __attribute__((target("arch=lakemont"))) >>> >>> Actually, we have to re-initialize: >>> >>> opts->x_target_flags >>> |= (TARGET_DEFAULT | TARGET_SUBTARGET_DEFAULT) & ~opts_set->x_target_flags; >>> >>> just before TARGET_SUBTARGET{32,64}_DEFAULT processing, and it will work. >> >> No, this won't work. The value of MASK_NO_FANCY_MATH depend on >> MASK_80387setting, and once fancy math bit is set, it couldn't be >> cleared for march != lakemont. >> >> It looks just like we want to error out when lakemont is enabled with -m80387. > > Like in the attached patch, that also slightly improves existing error > reporting. > We should use a bit instead of checking PROCESSOR_LAKEMONT so that we don't need to check another PROCESSOR_XXX for a new IA MCU processor. Thanks. -- H.J. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix pr67963 2015-10-16 10:33 ` H.J. Lu @ 2015-10-16 10:45 ` H.J. Lu 0 siblings, 0 replies; 14+ messages in thread From: H.J. Lu @ 2015-10-16 10:45 UTC (permalink / raw) To: Uros Bizjak; +Cc: Yulia Koval, GCC Patches On Fri, Oct 16, 2015 at 3:31 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, Oct 16, 2015 at 2:35 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Fri, Oct 16, 2015 at 8:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> On Thu, Oct 15, 2015 at 9:30 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> >>>>>>> Do we support -O2 -march=lakemont with >>>>>>> >>>>>>> __attribute__((target("arch=silvermont"))) >>>>>> >>>>>> Hm, no. >>>>>> >>>>> >>>>> Do we issue an error or silently ignore >>>>> __attribute__((target("arch=silvermont")))? >>>>> If we don't support it, should we support >>>>> >>>>> -O2 -march=silvermont >>>>> >>>>> __attribute__((target("arch=lakemont"))) >>>> >>>> Actually, we have to re-initialize: >>>> >>>> opts->x_target_flags >>>> |= (TARGET_DEFAULT | TARGET_SUBTARGET_DEFAULT) & ~opts_set->x_target_flags; >>>> >>>> just before TARGET_SUBTARGET{32,64}_DEFAULT processing, and it will work. >>> >>> No, this won't work. The value of MASK_NO_FANCY_MATH depend on >>> MASK_80387setting, and once fancy math bit is set, it couldn't be >>> cleared for march != lakemont. >>> >>> It looks just like we want to error out when lakemont is enabled with -m80387. >> >> Like in the attached patch, that also slightly improves existing error >> reporting. >> > > We should use a bit instead of checking PROCESSOR_LAKEMONT > so that we don't need to check another PROCESSOR_XXX for > a new IA MCU processor. > Another related bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67985 We may be able to fix both in a patch. -- H.J. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-10-16 10:45 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-14 15:08 [PATCH] Fix pr67963 Yulia Koval 2015-10-14 15:15 ` H.J. Lu 2015-10-14 15:17 ` H.J. Lu 2015-10-15 7:53 ` Uros Bizjak 2015-10-15 11:45 ` Uros Bizjak 2015-10-15 16:57 ` Uros Bizjak 2015-10-15 16:59 ` H.J. Lu 2015-10-15 19:16 ` Uros Bizjak 2015-10-15 19:21 ` H.J. Lu 2015-10-15 19:31 ` Uros Bizjak 2015-10-16 6:45 ` Uros Bizjak 2015-10-16 9:37 ` Uros Bizjak 2015-10-16 10:33 ` H.J. Lu 2015-10-16 10:45 ` H.J. Lu
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).