[PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[Jan Beulich]

Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
".arch .nosse*"') I think this makes clear that the whole .arch logic
needs an overhaul, such that the mechanism to enable features implies
the reverse operation when disabling any, without having to modify two
places. Arm64's approach may be worthwhile to consider cloning.

Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
handling") introducing the testcase which needs fixing here explicitly
says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
this would be. Furthermore it also says "Don't disable AVX512 when
disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
sensible (consistent) alternative therefore would be to avoid enabling
SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
to inconsistencies with SSE insns accessing MMX registers).

gas/
2020-02-XX  Jan Beulich  <jbeulich@suse.com>

	* gas/testsuite/gas/i386/i386.exp: Convert nosse-5 from dump to
	list test.
	* gas/testsuite/gas/i386/nosse-5.s: Adjust comment. Reset arch to
	generic32 before enabling AVX512VL. Force alignment.
	* gas/testsuite/gas/i386/nosse-5.d: Delete.
	* gas/testsuite/gas/i386/nosse-5.l: New.

opcodes/
2020-02-XX  Jan Beulich  <jbeulich@suse.com>

	* i386-gen.c (cpu_flag_init): Add CPU_ANY_AVX_FLAGS to
	CPU_ANY_SSE4_2_FLAGS entry.
	* i386-init.h: Re-generate.

--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -195,7 +195,7 @@ if [expr ([istarget "i*86-*-*"] ||  [ist
     run_list_test "nosse-2" "-march=core+nosse -al"
     run_list_test "nosse-3" "-march=+nosse -al"
     run_list_test "nosse-4" "-al"
-    run_dump_test "nosse-5"
+    run_list_test "nosse-5" "-al"
     run_list_test "noavx-1" "-al"
     run_list_test "noavx-2" "-march=+noavx -al"
     run_list_test "noavx-3" "-al"
--- a/gas/testsuite/gas/i386/nosse-5.d
+++ /dev/null
@@ -1,24 +0,0 @@
-#objdump: -drw
-#name: i386 .nosse
-
-.*: +file format .*
-
-Disassembly of section .text:
-
-0+ <.text>:
-[ 	]*[a-f0-9]+:	c5 d0 58 e6          	vaddps %xmm6,%xmm5,%xmm4
-[ 	]*[a-f0-9]+:	c5 d4 58 e6          	vaddps %ymm6,%ymm5,%ymm4
-[ 	]*[a-f0-9]+:	c5 d2 58 e6          	vaddss %xmm6,%xmm5,%xmm4
-[ 	]*[a-f0-9]+:	c5 d0 58 e6          	vaddps %xmm6,%xmm5,%xmm4
-[ 	]*[a-f0-9]+:	c5 d4 58 e6          	vaddps %ymm6,%ymm5,%ymm4
-[ 	]*[a-f0-9]+:	c5 d2 58 e6          	vaddss %xmm6,%xmm5,%xmm4
-[ 	]*[a-f0-9]+:	62 f1 54 0f 58 e6    	vaddps %xmm6,%xmm5,%xmm4\{%k7\}
-[ 	]*[a-f0-9]+:	62 f1 54 2f 58 e6    	vaddps %ymm6,%ymm5,%ymm4\{%k7\}
-[ 	]*[a-f0-9]+:	62 f1 54 48 58 e6    	vaddps %zmm6,%zmm5,%zmm4
-[ 	]*[a-f0-9]+:	62 f1 54 0f 58 e6    	vaddps %xmm6,%xmm5,%xmm4\{%k7\}
-[ 	]*[a-f0-9]+:	62 f1 54 2f 58 e6    	vaddps %ymm6,%ymm5,%ymm4\{%k7\}
-[ 	]*[a-f0-9]+:	62 f1 54 48 58 e6    	vaddps %zmm6,%zmm5,%zmm4
-[ 	]*[a-f0-9]+:	c5 d0 58 e6          	vaddps %xmm6,%xmm5,%xmm4
-[ 	]*[a-f0-9]+:	c5 d4 58 e6          	vaddps %ymm6,%ymm5,%ymm4
-[ 	]*[a-f0-9]+:	c5 d2 58 e6          	vaddss %xmm6,%xmm5,%xmm4
-#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/nosse-5.l
@@ -0,0 +1,41 @@
+.*: Assembler messages:
+.*:9: Error: .*\.avx\.nosse.*
+.*:10: Error: .*\.avx\.nosse.*
+.*:11: Error: .*\.avx\.nosse.*
+.*:19: Error: .*\.avx512vl\.nosse.*
+.*:20: Error: .*\.avx512vl\.nosse.*
+.*:21: Error: .*\.avx512vl\.nosse.*
+.*:22: Error: .*\.avx512vl\.nosse.*
+.*:23: Error: .*\.avx512vl\.nosse.*
+.*:24: Error: .*\.avx512vl\.nosse.*
+GAS LISTING .*
+#...
+[ 	]*[1-9][0-9]*[ 	]+\#.*
+[ 	]*[1-9][0-9]*[ 	]+\.text
+[ 	]*[1-9][0-9]*[ 	]+\.arch generic32
+[ 	]*[1-9][0-9]*[ 	]+\.arch \.avx
+[ 	]*[1-9][0-9]* \?\?\?\? C5D058E6[ 	]+vaddps	%xmm6, %xmm5, %xmm4
+[ 	]*[1-9][0-9]* \?\?\?\? C5D458E6[ 	]+vaddps	%ymm6, %ymm5, %ymm4
+[ 	]*[1-9][0-9]* \?\?\?\? C5D258E6[ 	]+vaddss	%xmm6, %xmm5, %xmm4
+[ 	]*[1-9][0-9]*[ 	]+\.arch \.nosse
+[ 	]*[1-9][0-9]*[ 	]+vaddps	%xmm6, %xmm5, %xmm4
+[ 	]*[1-9][0-9]*[ 	]+vaddps	%ymm6, %ymm5, %ymm4
+[ 	]*[1-9][0-9]*[ 	]+vaddss	%xmm6, %xmm5, %xmm4
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+\.arch generic32
+[ 	]*[1-9][0-9]*[ 	]+\.arch \.avx512vl
+[ 	]*[1-9][0-9]* \?\?\?\? 62F1540F[ 	]+vaddps	%xmm6, %xmm5, %xmm4\{%k7\}
+[ 	]*[1-9][0-9]*[ 	]+58E6
+[ 	]*[1-9][0-9]* \?\?\?\? 62F1542F[ 	]+vaddps	%ymm6, %ymm5, %ymm4\{%k7\}
+[ 	]*[1-9][0-9]*[ 	]+58E6
+[ 	]*[1-9][0-9]* \?\?\?\? 62F15448[ 	]+vaddps	%zmm6, %zmm5, %zmm4
+[ 	]*[1-9][0-9]*[ 	]+58E6
+[ 	]*[1-9][0-9]*[ 	]+\.arch \.nosse
+[ 	]*[1-9][0-9]*[ 	]+vaddps	%xmm6, %xmm5, %xmm4\{%k7\}
+[ 	]*[1-9][0-9]*[ 	]+vaddps	%ymm6, %ymm5, %ymm4\{%k7\}
+[ 	]*[1-9][0-9]*[ 	]+vaddps	%zmm6, %zmm5, %zmm4
+[ 	]*[1-9][0-9]*[ 	]+vaddps	%xmm6, %xmm5, %xmm4
+[ 	]*[1-9][0-9]*[ 	]+vaddps	%ymm6, %ymm5, %ymm4
+[ 	]*[1-9][0-9]*[ 	]+vaddss	%xmm6, %xmm5, %xmm4
+[ 	]*[1-9][0-9]*[ 	]*
+#pass
--- a/gas/testsuite/gas/i386/nosse-5.s
+++ b/gas/testsuite/gas/i386/nosse-5.s
@@ -1,4 +1,4 @@
-# Test .arch .nosse with .noavx/.avx/.avx512vl
+# Test .arch .nosse with .avx/.avx512vl
 	.text
 	.arch generic32
 	.arch .avx
@@ -9,6 +9,8 @@
 	vaddps	%xmm6, %xmm5, %xmm4
 	vaddps	%ymm6, %ymm5, %ymm4
 	vaddss	%xmm6, %xmm5, %xmm4
+
+	.arch generic32
 	.arch .avx512vl
 	vaddps	%xmm6, %xmm5, %xmm4{%k7}
 	vaddps	%ymm6, %ymm5, %ymm4{%k7}
@@ -20,3 +22,5 @@
 	vaddps	%xmm6, %xmm5, %xmm4
 	vaddps	%ymm6, %ymm5, %ymm4
 	vaddss	%xmm6, %xmm5, %xmm4
+
+	.p2align 4
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -332,7 +332,7 @@ static initializer cpu_flag_init[] =
   { "CPU_ANY_SSE4_1_FLAGS",
     "CPU_ANY_SSE4_2_FLAGS|CpuSSE4_1" },
   { "CPU_ANY_SSE4_2_FLAGS",
-    "CpuSSE4_2" },
+    "CPU_ANY_AVX_FLAGS|CpuSSE4_2" },
   { "CPU_ANY_SSE4_FLAGS",
     "CPU_ANY_SSE4_1_FLAGS|CpuSSE4a" },
   { "CPU_ANY_AVX_FLAGS",

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[H.J. Lu]

On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
> ".arch .nosse*"') I think this makes clear that the whole .arch logic
> needs an overhaul, such that the mechanism to enable features implies
> the reverse operation when disabling any, without having to modify two
> places. Arm64's approach may be worthwhile to consider cloning.
>
> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
> handling") introducing the testcase which needs fixing here explicitly
> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
> this would be. Furthermore it also says "Don't disable AVX512 when
> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
> sensible (consistent) alternative therefore would be to avoid enabling
> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
> to inconsistencies with SSE insns accessing MMX registers).
>

The intended usages are to build an object:

1. Without MMX, but with SSE, AVX and AVX512.
2. Without SEE nor MMX, but with AVX and AVX512.
3. Without SSE, MMX, AVX, but with AVX512.

enforced by assembler.  Your patch doesn't help it.

-- 
H.J.

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[Jan Beulich]

On 13.02.2020 12:50, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
>> needs an overhaul, such that the mechanism to enable features implies
>> the reverse operation when disabling any, without having to modify two
>> places. Arm64's approach may be worthwhile to consider cloning.
>>
>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
>> handling") introducing the testcase which needs fixing here explicitly
>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
>> this would be. Furthermore it also says "Don't disable AVX512 when
>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
>> sensible (consistent) alternative therefore would be to avoid enabling
>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
>> to inconsistencies with SSE insns accessing MMX registers).
>>
> 
> The intended usages are to build an object:
> 
> 1. Without MMX, but with SSE, AVX and AVX512.
> 2. Without SEE nor MMX, but with AVX and AVX512.
> 3. Without SSE, MMX, AVX, but with AVX512.
> 
> enforced by assembler.

For one - this isn't spelled out anywhere in the docs. And then it's
also counter-intuitive. Plus your point 3 isn't true afaict, as per
commit 89199bb5a027 mentioned above.

>  Your patch doesn't help it.

My patch gets the SSE/AVX interaction into the same state as your
89199bb5a027 did for the AVX/AVX512F, so improves consistency. MMX
is different, as it acts on a different register file.

Jan

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[H.J. Lu]

On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 12:50, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
> >> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
> >> ".arch .nosse*"') I think this makes clear that the whole .arch logic
> >> needs an overhaul, such that the mechanism to enable features implies
> >> the reverse operation when disabling any, without having to modify two
> >> places. Arm64's approach may be worthwhile to consider cloning.
> >>
> >> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
> >> handling") introducing the testcase which needs fixing here explicitly
> >> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
> >> this would be. Furthermore it also says "Don't disable AVX512 when
> >> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
> >> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
> >> sensible (consistent) alternative therefore would be to avoid enabling
> >> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
> >> to inconsistencies with SSE insns accessing MMX registers).
> >>
> >
> > The intended usages are to build an object:
> >
> > 1. Without MMX, but with SSE, AVX and AVX512.
> > 2. Without SEE nor MMX, but with AVX and AVX512.
> > 3. Without SSE, MMX, AVX, but with AVX512.
> >
> > enforced by assembler.
>
> For one - this isn't spelled out anywhere in the docs. And then it's
> also counter-intuitive. Plus your point 3 isn't true afaict, as per
> commit 89199bb5a027 mentioned above.

We should revisit it.

> >  Your patch doesn't help it.
>
> My patch gets the SSE/AVX interaction into the same state as your
> 89199bb5a027 did for the AVX/AVX512F, so improves consistency. MMX
> is different, as it acts on a different register file.


-- 
H.J.

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[Jan Beulich]

On 13.02.2020 15:17, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 13.02.2020 12:50, H.J. Lu wrote:
>>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
>>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
>>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
>>>> needs an overhaul, such that the mechanism to enable features implies
>>>> the reverse operation when disabling any, without having to modify two
>>>> places. Arm64's approach may be worthwhile to consider cloning.
>>>>
>>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
>>>> handling") introducing the testcase which needs fixing here explicitly
>>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
>>>> this would be. Furthermore it also says "Don't disable AVX512 when
>>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
>>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
>>>> sensible (consistent) alternative therefore would be to avoid enabling
>>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
>>>> to inconsistencies with SSE insns accessing MMX registers).
>>>>
>>>
>>> The intended usages are to build an object:
>>>
>>> 1. Without MMX, but with SSE, AVX and AVX512.
>>> 2. Without SEE nor MMX, but with AVX and AVX512.
>>> 3. Without SSE, MMX, AVX, but with AVX512.
>>>
>>> enforced by assembler.
>>
>> For one - this isn't spelled out anywhere in the docs. And then it's
>> also counter-intuitive. Plus your point 3 isn't true afaict, as per
>> commit 89199bb5a027 mentioned above.
> 
> We should revisit it.

What is the point of a mode like what you outline in 3 above
anyway? Such a mode is even contrary to -O converting EVEX to
VEX where possible.

Jan

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[H.J. Lu]

On Thu, Feb 13, 2020 at 6:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 15:17, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 13.02.2020 12:50, H.J. Lu wrote:
> >>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
> >>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
> >>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
> >>>> needs an overhaul, such that the mechanism to enable features implies
> >>>> the reverse operation when disabling any, without having to modify two
> >>>> places. Arm64's approach may be worthwhile to consider cloning.
> >>>>
> >>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
> >>>> handling") introducing the testcase which needs fixing here explicitly
> >>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
> >>>> this would be. Furthermore it also says "Don't disable AVX512 when
> >>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
> >>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
> >>>> sensible (consistent) alternative therefore would be to avoid enabling
> >>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
> >>>> to inconsistencies with SSE insns accessing MMX registers).
> >>>>
> >>>
> >>> The intended usages are to build an object:
> >>>
> >>> 1. Without MMX, but with SSE, AVX and AVX512.
> >>> 2. Without SEE nor MMX, but with AVX and AVX512.
> >>> 3. Without SSE, MMX, AVX, but with AVX512.
> >>>
> >>> enforced by assembler.
> >>
> >> For one - this isn't spelled out anywhere in the docs. And then it's
> >> also counter-intuitive. Plus your point 3 isn't true afaict, as per
> >> commit 89199bb5a027 mentioned above.
> >
> > We should revisit it.
>
> What is the point of a mode like what you outline in 3 above

We can use it to build a pure AVX512 object.

> anyway? Such a mode is even contrary to -O converting EVEX to
> VEX where possible.

Well, we should check if AVX is enabled when doing it.

-- 
H.J.

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[Jan Beulich]

On 13.02.2020 16:56, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 6:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 13.02.2020 15:17, H.J. Lu wrote:
>>> On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 13.02.2020 12:50, H.J. Lu wrote:
>>>>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
>>>>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
>>>>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
>>>>>> needs an overhaul, such that the mechanism to enable features implies
>>>>>> the reverse operation when disabling any, without having to modify two
>>>>>> places. Arm64's approach may be worthwhile to consider cloning.
>>>>>>
>>>>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
>>>>>> handling") introducing the testcase which needs fixing here explicitly
>>>>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
>>>>>> this would be. Furthermore it also says "Don't disable AVX512 when
>>>>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
>>>>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
>>>>>> sensible (consistent) alternative therefore would be to avoid enabling
>>>>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
>>>>>> to inconsistencies with SSE insns accessing MMX registers).
>>>>>>
>>>>>
>>>>> The intended usages are to build an object:
>>>>>
>>>>> 1. Without MMX, but with SSE, AVX and AVX512.
>>>>> 2. Without SEE nor MMX, but with AVX and AVX512.
>>>>> 3. Without SSE, MMX, AVX, but with AVX512.
>>>>>
>>>>> enforced by assembler.
>>>>
>>>> For one - this isn't spelled out anywhere in the docs. And then it's
>>>> also counter-intuitive. Plus your point 3 isn't true afaict, as per
>>>> commit 89199bb5a027 mentioned above.
>>>
>>> We should revisit it.
>>
>> What is the point of a mode like what you outline in 3 above
> 
> We can use it to build a pure AVX512 object.

But that's not something to be achieved by .arch directives. I
can see the (at least theoretical) usefulness of such a mode,
but it should be controlled by something other than .arch, much
like SSE2AVX mode or SSE insn checking modes also can't be
controlled this way.

Jan

>> anyway? Such a mode is even contrary to -O converting EVEX to
>> VEX where possible.
> 
> Well, we should check if AVX is enabled when doing it.
> 

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[H.J. Lu]

On Thu, Feb 13, 2020 at 8:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 16:56, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 6:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 13.02.2020 15:17, H.J. Lu wrote:
> >>> On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 13.02.2020 12:50, H.J. Lu wrote:
> >>>>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
> >>>>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
> >>>>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
> >>>>>> needs an overhaul, such that the mechanism to enable features implies
> >>>>>> the reverse operation when disabling any, without having to modify two
> >>>>>> places. Arm64's approach may be worthwhile to consider cloning.
> >>>>>>
> >>>>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
> >>>>>> handling") introducing the testcase which needs fixing here explicitly
> >>>>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
> >>>>>> this would be. Furthermore it also says "Don't disable AVX512 when
> >>>>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
> >>>>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
> >>>>>> sensible (consistent) alternative therefore would be to avoid enabling
> >>>>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
> >>>>>> to inconsistencies with SSE insns accessing MMX registers).
> >>>>>>
> >>>>>
> >>>>> The intended usages are to build an object:
> >>>>>
> >>>>> 1. Without MMX, but with SSE, AVX and AVX512.
> >>>>> 2. Without SEE nor MMX, but with AVX and AVX512.
> >>>>> 3. Without SSE, MMX, AVX, but with AVX512.
> >>>>>
> >>>>> enforced by assembler.
> >>>>
> >>>> For one - this isn't spelled out anywhere in the docs. And then it's
> >>>> also counter-intuitive. Plus your point 3 isn't true afaict, as per
> >>>> commit 89199bb5a027 mentioned above.
> >>>
> >>> We should revisit it.
> >>
> >> What is the point of a mode like what you outline in 3 above
> >
> > We can use it to build a pure AVX512 object.
>
> But that's not something to be achieved by .arch directives. I
> can see the (at least theoretical) usefulness of such a mode,
> but it should be controlled by something other than .arch, much
> like SSE2AVX mode or SSE insn checking modes also can't be
> controlled this way.

I think it is doable.

> Jan
>
> >> anyway? Such a mode is even contrary to -O converting EVEX to
> >> VEX where possible.
> >
> > Well, we should check if AVX is enabled when doing it.
> >
>


-- 
H.J.

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[Jan Beulich]

On 13.02.2020 17:37, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 8:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 13.02.2020 16:56, H.J. Lu wrote:
>>> On Thu, Feb 13, 2020 at 6:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 13.02.2020 15:17, H.J. Lu wrote:
>>>>> On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 13.02.2020 12:50, H.J. Lu wrote:
>>>>>>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>
>>>>>>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
>>>>>>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
>>>>>>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
>>>>>>>> needs an overhaul, such that the mechanism to enable features implies
>>>>>>>> the reverse operation when disabling any, without having to modify two
>>>>>>>> places. Arm64's approach may be worthwhile to consider cloning.
>>>>>>>>
>>>>>>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
>>>>>>>> handling") introducing the testcase which needs fixing here explicitly
>>>>>>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
>>>>>>>> this would be. Furthermore it also says "Don't disable AVX512 when
>>>>>>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
>>>>>>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
>>>>>>>> sensible (consistent) alternative therefore would be to avoid enabling
>>>>>>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
>>>>>>>> to inconsistencies with SSE insns accessing MMX registers).
>>>>>>>>
>>>>>>>
>>>>>>> The intended usages are to build an object:
>>>>>>>
>>>>>>> 1. Without MMX, but with SSE, AVX and AVX512.
>>>>>>> 2. Without SEE nor MMX, but with AVX and AVX512.
>>>>>>> 3. Without SSE, MMX, AVX, but with AVX512.
>>>>>>>
>>>>>>> enforced by assembler.
>>>>>>
>>>>>> For one - this isn't spelled out anywhere in the docs. And then it's
>>>>>> also counter-intuitive. Plus your point 3 isn't true afaict, as per
>>>>>> commit 89199bb5a027 mentioned above.
>>>>>
>>>>> We should revisit it.
>>>>
>>>> What is the point of a mode like what you outline in 3 above
>>>
>>> We can use it to build a pure AVX512 object.
>>
>> But that's not something to be achieved by .arch directives. I
>> can see the (at least theoretical) usefulness of such a mode,
>> but it should be controlled by something other than .arch, much
>> like SSE2AVX mode or SSE insn checking modes also can't be
>> controlled this way.
> 
> I think it is doable.

Meaning this patch is okay and can go in, while you work on your
intended model?

Jan

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[H.J. Lu]

On Thu, Feb 13, 2020 at 8:46 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 17:37, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 8:03 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 13.02.2020 16:56, H.J. Lu wrote:
> >>> On Thu, Feb 13, 2020 at 6:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 13.02.2020 15:17, H.J. Lu wrote:
> >>>>> On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 13.02.2020 12:50, H.J. Lu wrote:
> >>>>>>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>
> >>>>>>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
> >>>>>>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
> >>>>>>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
> >>>>>>>> needs an overhaul, such that the mechanism to enable features implies
> >>>>>>>> the reverse operation when disabling any, without having to modify two
> >>>>>>>> places. Arm64's approach may be worthwhile to consider cloning.
> >>>>>>>>
> >>>>>>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
> >>>>>>>> handling") introducing the testcase which needs fixing here explicitly
> >>>>>>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
> >>>>>>>> this would be. Furthermore it also says "Don't disable AVX512 when
> >>>>>>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
> >>>>>>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
> >>>>>>>> sensible (consistent) alternative therefore would be to avoid enabling
> >>>>>>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
> >>>>>>>> to inconsistencies with SSE insns accessing MMX registers).
> >>>>>>>>
> >>>>>>>
> >>>>>>> The intended usages are to build an object:
> >>>>>>>
> >>>>>>> 1. Without MMX, but with SSE, AVX and AVX512.
> >>>>>>> 2. Without SEE nor MMX, but with AVX and AVX512.
> >>>>>>> 3. Without SSE, MMX, AVX, but with AVX512.
> >>>>>>>
> >>>>>>> enforced by assembler.
> >>>>>>
> >>>>>> For one - this isn't spelled out anywhere in the docs. And then it's
> >>>>>> also counter-intuitive. Plus your point 3 isn't true afaict, as per
> >>>>>> commit 89199bb5a027 mentioned above.
> >>>>>
> >>>>> We should revisit it.
> >>>>
> >>>> What is the point of a mode like what you outline in 3 above
> >>>
> >>> We can use it to build a pure AVX512 object.
> >>
> >> But that's not something to be achieved by .arch directives. I
> >> can see the (at least theoretical) usefulness of such a mode,
> >> but it should be controlled by something other than .arch, much
> >> like SSE2AVX mode or SSE insn checking modes also can't be
> >> controlled this way.
> >
> > I think it is doable.

I meant ".arch" could be used to build a pure AVX512 object.

> Meaning this patch is okay and can go in, while you work on your
> intended model?

No.

-- 
H.J.

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[Jan Beulich]

On 13.02.2020 17:58, H.J. Lu wrote:
> On Thu, Feb 13, 2020 at 8:46 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 13.02.2020 17:37, H.J. Lu wrote:
>>> On Thu, Feb 13, 2020 at 8:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 13.02.2020 16:56, H.J. Lu wrote:
>>>>> On Thu, Feb 13, 2020 at 6:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 13.02.2020 15:17, H.J. Lu wrote:
>>>>>>> On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>
>>>>>>>> On 13.02.2020 12:50, H.J. Lu wrote:
>>>>>>>>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>>
>>>>>>>>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
>>>>>>>>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
>>>>>>>>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
>>>>>>>>>> needs an overhaul, such that the mechanism to enable features implies
>>>>>>>>>> the reverse operation when disabling any, without having to modify two
>>>>>>>>>> places. Arm64's approach may be worthwhile to consider cloning.
>>>>>>>>>>
>>>>>>>>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
>>>>>>>>>> handling") introducing the testcase which needs fixing here explicitly
>>>>>>>>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
>>>>>>>>>> this would be. Furthermore it also says "Don't disable AVX512 when
>>>>>>>>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
>>>>>>>>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
>>>>>>>>>> sensible (consistent) alternative therefore would be to avoid enabling
>>>>>>>>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
>>>>>>>>>> to inconsistencies with SSE insns accessing MMX registers).
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The intended usages are to build an object:
>>>>>>>>>
>>>>>>>>> 1. Without MMX, but with SSE, AVX and AVX512.
>>>>>>>>> 2. Without SEE nor MMX, but with AVX and AVX512.
>>>>>>>>> 3. Without SSE, MMX, AVX, but with AVX512.
>>>>>>>>>
>>>>>>>>> enforced by assembler.
>>>>>>>>
>>>>>>>> For one - this isn't spelled out anywhere in the docs. And then it's
>>>>>>>> also counter-intuitive. Plus your point 3 isn't true afaict, as per
>>>>>>>> commit 89199bb5a027 mentioned above.
>>>>>>>
>>>>>>> We should revisit it.
>>>>>>
>>>>>> What is the point of a mode like what you outline in 3 above
>>>>>
>>>>> We can use it to build a pure AVX512 object.
>>>>
>>>> But that's not something to be achieved by .arch directives. I
>>>> can see the (at least theoretical) usefulness of such a mode,
>>>> but it should be controlled by something other than .arch, much
>>>> like SSE2AVX mode or SSE insn checking modes also can't be
>>>> controlled this way.
>>>
>>> I think it is doable.
> 
> I meant ".arch" could be used to build a pure AVX512 object.

Well, that's an option, albeit I think it's not ideal. What is not
an option is for ".arch .avx512f" to not also enable AVX512F's
prereqs. There could be an extension like ".arch .avx512f-only" if
you want such a mode. And of course there ought to be symmetry -
if some .arch enables prereqs, then if any of the prereqs gets
disabled, the dependent feature should get disabled, too. That's
how other arches do it, I don't see why x86 should be different.

Jan

Re: [PATCH] x86: fix AVX* dependencies of ".arch .nosse*"

[H.J. Lu]

On Thu, Feb 13, 2020 at 9:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 17:58, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 8:46 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 13.02.2020 17:37, H.J. Lu wrote:
> >>> On Thu, Feb 13, 2020 at 8:03 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 13.02.2020 16:56, H.J. Lu wrote:
> >>>>> On Thu, Feb 13, 2020 at 6:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 13.02.2020 15:17, H.J. Lu wrote:
> >>>>>>> On Thu, Feb 13, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>
> >>>>>>>> On 13.02.2020 12:50, H.J. Lu wrote:
> >>>>>>>>> On Thu, Feb 13, 2020 at 1:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Since ".arch .avx*" enables SSE*, disabling SSE* should also disable
> >>>>>>>>>> AVX*. Together with 7deea9aad866 ('x86: fix SSE4a dependencies of
> >>>>>>>>>> ".arch .nosse*"') I think this makes clear that the whole .arch logic
> >>>>>>>>>> needs an overhaul, such that the mechanism to enable features implies
> >>>>>>>>>> the reverse operation when disabling any, without having to modify two
> >>>>>>>>>> places. Arm64's approach may be worthwhile to consider cloning.
> >>>>>>>>>>
> >>>>>>>>>> Note that while commit 1848e567343e ("Update x86 CPU_XXX_FLAGS
> >>>>>>>>>> handling") introducing the testcase which needs fixing here explicitly
> >>>>>>>>>> says "Don't disable AVX nor AVX512 when disabling SSE", I don't see why
> >>>>>>>>>> this would be. Furthermore it also says "Don't disable AVX512 when
> >>>>>>>>>> disabling AVX", which too has been undone meanwhile (commit 89199bb5a027
> >>>>>>>>>> ["ix86: Disable AVX512F when disabling AVX2"], PR gas/24359). The only
> >>>>>>>>>> sensible (consistent) alternative therefore would be to avoid enabling
> >>>>>>>>>> SSE* with ".arch .avx*", like is done for SSE* wrt MMX (in turn leading
> >>>>>>>>>> to inconsistencies with SSE insns accessing MMX registers).
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> The intended usages are to build an object:
> >>>>>>>>>
> >>>>>>>>> 1. Without MMX, but with SSE, AVX and AVX512.
> >>>>>>>>> 2. Without SEE nor MMX, but with AVX and AVX512.
> >>>>>>>>> 3. Without SSE, MMX, AVX, but with AVX512.
> >>>>>>>>>
> >>>>>>>>> enforced by assembler.
> >>>>>>>>
> >>>>>>>> For one - this isn't spelled out anywhere in the docs. And then it's
> >>>>>>>> also counter-intuitive. Plus your point 3 isn't true afaict, as per
> >>>>>>>> commit 89199bb5a027 mentioned above.
> >>>>>>>
> >>>>>>> We should revisit it.
> >>>>>>
> >>>>>> What is the point of a mode like what you outline in 3 above
> >>>>>
> >>>>> We can use it to build a pure AVX512 object.
> >>>>
> >>>> But that's not something to be achieved by .arch directives. I
> >>>> can see the (at least theoretical) usefulness of such a mode,
> >>>> but it should be controlled by something other than .arch, much
> >>>> like SSE2AVX mode or SSE insn checking modes also can't be
> >>>> controlled this way.
> >>>
> >>> I think it is doable.
> >
> > I meant ".arch" could be used to build a pure AVX512 object.
>
> Well, that's an option, albeit I think it's not ideal. What is not
> an option is for ".arch .avx512f" to not also enable AVX512F's
> prereqs. There could be an extension like ".arch .avx512f-only" if
> you want such a mode. And of course there ought to be symmetry -
> if some .arch enables prereqs, then if any of the prereqs gets
> disabled, the dependent feature should get disabled, too. That's
> how other arches do it, I don't see why x86 should be different.
>

x86 doesn't use .arch directives much.   We need to carefully
consider our options.   I opened:

https://sourceware.org/bugzilla/show_bug.cgi?id=25550

-- 
H.J.