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