[PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Sourceware - binutils list mailing list
        * configure.ac: Configure with --enable-x86-used-note by default
        for Linux/x86.
        * configure: Regenerated.
---
 gas/configure    | 7 +++++++
 gas/configure.ac | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/gas/configure b/gas/configure
index e480b1d997..ec9f088b03 100755
--- a/gas/configure
+++ b/gas/configure
@@ -12636,6 +12636,13 @@ $as_echo "#define STRICTCOFF 1" >>confdefs.h
 
  ;;
 
+      i386-*-linux-* | x86_64-*-linux-*)
+ if test ${this_target} = $target \
+   && test ${ac_default_generate_x86_used_note} = unset; then
+  ac_default_generate_x86_used_note=1
+ fi
+ ;;
+
       i386-*-solaris2 \
       | x86_64-*-solaris2 \
       | i386-*-solaris2.[0-9] \
diff --git a/gas/configure.ac b/gas/configure.ac
index b65108fecb..8d968defb6 100644
--- a/gas/configure.ac
+++ b/gas/configure.ac
@@ -242,6 +242,13 @@ for this_target in $target $canon_targets ; do
  AC_DEFINE(STRICTCOFF, 1, [Using strict COFF?])
  ;;
 
+      i386-*-linux-* | x86_64-*-linux-*)
+ if test ${this_target} = $target \
+   && test ${ac_default_generate_x86_used_note} = unset; then
+  ac_default_generate_x86_used_note=1
+ fi
+ ;;
+
       i386-*-solaris2 \
       | x86_64-*-solaris2 \
       | i386-*-solaris2.[[0-9]] \
--
2.26.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Jan Beulich-2
On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
> * configure.ac: Configure with --enable-x86-used-note by default
> for Linux/x86.

I'm quite unconvinced this is a good idea as long as the contents of
these notes are neither reliable nor properly settled on what they
actually mean. This should be considered an experimental feature for
now, and hence would better not be enabled by default to avoid
future backwards compatibility issues.

Jan

> * configure: Regenerated.
> ---
>  gas/configure    | 7 +++++++
>  gas/configure.ac | 7 +++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/gas/configure b/gas/configure
> index e480b1d997..ec9f088b03 100755
> --- a/gas/configure
> +++ b/gas/configure
> @@ -12636,6 +12636,13 @@ $as_echo "#define STRICTCOFF 1" >>confdefs.h
>  
>   ;;
>  
> +      i386-*-linux-* | x86_64-*-linux-*)
> + if test ${this_target} = $target \
> +   && test ${ac_default_generate_x86_used_note} = unset; then
> +  ac_default_generate_x86_used_note=1
> + fi
> + ;;
> +
>        i386-*-solaris2 \
>        | x86_64-*-solaris2 \
>        | i386-*-solaris2.[0-9] \
> diff --git a/gas/configure.ac b/gas/configure.ac
> index b65108fecb..8d968defb6 100644
> --- a/gas/configure.ac
> +++ b/gas/configure.ac
> @@ -242,6 +242,13 @@ for this_target in $target $canon_targets ; do
>   AC_DEFINE(STRICTCOFF, 1, [Using strict COFF?])
>   ;;
>  
> +      i386-*-linux-* | x86_64-*-linux-*)
> + if test ${this_target} = $target \
> +   && test ${ac_default_generate_x86_used_note} = unset; then
> +  ac_default_generate_x86_used_note=1
> + fi
> + ;;
> +
>        i386-*-solaris2 \
>        | x86_64-*-solaris2 \
>        | i386-*-solaris2.[[0-9]] \
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Sourceware - binutils list mailing list
On Thu, Jul 9, 2020 at 8:21 AM Jan Beulich <[hidden email]> wrote:

>
> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
> >       * configure.ac: Configure with --enable-x86-used-note by default
> >       for Linux/x86.
>
> I'm quite unconvinced this is a good idea as long as the contents of
> these notes are neither reliable nor properly settled on what they
> actually mean. This should be considered an experimental feature for
> now, and hence would better not be enabled by default to avoid
> future backwards compatibility issues.
>

It has been an experimental feature for 2 years.  Let's see what fallouts
will be.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Jan Beulich-2
On 09.07.2020 17:22, H.J. Lu wrote:

> On Thu, Jul 9, 2020 at 8:21 AM Jan Beulich <[hidden email]> wrote:
>>
>> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
>>>       * configure.ac: Configure with --enable-x86-used-note by default
>>>       for Linux/x86.
>>
>> I'm quite unconvinced this is a good idea as long as the contents of
>> these notes are neither reliable nor properly settled on what they
>> actually mean. This should be considered an experimental feature for
>> now, and hence would better not be enabled by default to avoid
>> future backwards compatibility issues.
>>
>
> It has been an experimental feature for 2 years.

And when I asked questions about the underlying spec I did get at
best fuzzy answers from you. I still have on my todo list to get
this in shape with a pretty recent reply of yours, which was
supporting my view on how things should be (and why things are
broken right now), and hence somewhat contrary to earlier replies
of yours.

To just name the most unclear (to me) aspect of what's there
currently: What's the distinction between xmm, ymm, and zmm, when
zmm is a superset of ymm (and ymm one of xmm), yet at the same
time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
upper parts) as well. From my observations e.g. an AVX512 insn
accessing just xmm registers will record just an xmm dependency,
which then is simply wrong - the resulting binary in fact depends
on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
back then to tie what an insn records to the XCR0 bits it
requires to be set in order for it to not fault.

>  Let's see what fallouts will be.

Fallout may become noticeable only when the behavior of the notes
changes: People may start noticing that new (correct) dependencies
prevent things from working (assuming there's any consumer of
these notes).

Jan
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Sourceware - binutils list mailing list
On Thu, Jul 9, 2020 at 8:36 AM Jan Beulich <[hidden email]> wrote:

>
> On 09.07.2020 17:22, H.J. Lu wrote:
> > On Thu, Jul 9, 2020 at 8:21 AM Jan Beulich <[hidden email]> wrote:
> >>
> >> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
> >>>       * configure.ac: Configure with --enable-x86-used-note by default
> >>>       for Linux/x86.
> >>
> >> I'm quite unconvinced this is a good idea as long as the contents of
> >> these notes are neither reliable nor properly settled on what they
> >> actually mean. This should be considered an experimental feature for
> >> now, and hence would better not be enabled by default to avoid
> >> future backwards compatibility issues.
> >>
> >
> > It has been an experimental feature for 2 years.
>
> And when I asked questions about the underlying spec I did get at
> best fuzzy answers from you. I still have on my todo list to get
> this in shape with a pretty recent reply of yours, which was
> supporting my view on how things should be (and why things are
> broken right now), and hence somewhat contrary to earlier replies
> of yours.
>
> To just name the most unclear (to me) aspect of what's there
> currently: What's the distinction between xmm, ymm, and zmm, when
> zmm is a superset of ymm (and ymm one of xmm), yet at the same
> time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
> upper parts) as well. From my observations e.g. an AVX512 insn
> accessing just xmm registers will record just an xmm dependency,
> which then is simply wrong - the resulting binary in fact depends
> on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
> back then to tie what an insn records to the XCR0 bits it
> requires to be set in order for it to not fault.

Should we add a field to i386 opcode table for this?

> >  Let's see what fallouts will be.
>
> Fallout may become noticeable only when the behavior of the notes
> changes: People may start noticing that new (correct) dependencies
> prevent things from working (assuming there's any consumer of
> these notes).
>

We have a couple months to address this before 2.36 is released.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Jan Beulich-2
On 09.07.2020 17:44, H.J. Lu wrote:

> On Thu, Jul 9, 2020 at 8:36 AM Jan Beulich <[hidden email]> wrote:
>>
>> On 09.07.2020 17:22, H.J. Lu wrote:
>>> On Thu, Jul 9, 2020 at 8:21 AM Jan Beulich <[hidden email]> wrote:
>>>>
>>>> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
>>>>>       * configure.ac: Configure with --enable-x86-used-note by default
>>>>>       for Linux/x86.
>>>>
>>>> I'm quite unconvinced this is a good idea as long as the contents of
>>>> these notes are neither reliable nor properly settled on what they
>>>> actually mean. This should be considered an experimental feature for
>>>> now, and hence would better not be enabled by default to avoid
>>>> future backwards compatibility issues.
>>>>
>>>
>>> It has been an experimental feature for 2 years.
>>
>> And when I asked questions about the underlying spec I did get at
>> best fuzzy answers from you. I still have on my todo list to get
>> this in shape with a pretty recent reply of yours, which was
>> supporting my view on how things should be (and why things are
>> broken right now), and hence somewhat contrary to earlier replies
>> of yours.
>>
>> To just name the most unclear (to me) aspect of what's there
>> currently: What's the distinction between xmm, ymm, and zmm, when
>> zmm is a superset of ymm (and ymm one of xmm), yet at the same
>> time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
>> upper parts) as well. From my observations e.g. an AVX512 insn
>> accessing just xmm registers will record just an xmm dependency,
>> which then is simply wrong - the resulting binary in fact depends
>> on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
>> back then to tie what an insn records to the XCR0 bits it
>> requires to be set in order for it to not fault.
>
> Should we add a field to i386 opcode table for this?

I was wanting to try without, to see how much logic would be needed.
If it gets too much, doing so may be the better choice.

>>>  Let's see what fallouts will be.
>>
>> Fallout may become noticeable only when the behavior of the notes
>> changes: People may start noticing that new (correct) dependencies
>> prevent things from working (assuming there's any consumer of
>> these notes).
>
> We have a couple months to address this before 2.36 is released.

Under the assumption that people test non-released code ...

Jan
Reply | Threaded
Open this post in threaded view
|

[PATCH] x86: Properly set YMM/ZMM features

Sourceware - binutils list mailing list
On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <[hidden email]> wrote:

>
> On 09.07.2020 17:44, H.J. Lu wrote:
> > On Thu, Jul 9, 2020 at 8:36 AM Jan Beulich <[hidden email]> wrote:
> >>
> >> On 09.07.2020 17:22, H.J. Lu wrote:
> >>> On Thu, Jul 9, 2020 at 8:21 AM Jan Beulich <[hidden email]> wrote:
> >>>>
> >>>> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
> >>>>>       * configure.ac: Configure with --enable-x86-used-note by default
> >>>>>       for Linux/x86.
> >>>>
> >>>> I'm quite unconvinced this is a good idea as long as the contents of
> >>>> these notes are neither reliable nor properly settled on what they
> >>>> actually mean. This should be considered an experimental feature for
> >>>> now, and hence would better not be enabled by default to avoid
> >>>> future backwards compatibility issues.
> >>>>
> >>>
> >>> It has been an experimental feature for 2 years.
> >>
> >> And when I asked questions about the underlying spec I did get at
> >> best fuzzy answers from you. I still have on my todo list to get
> >> this in shape with a pretty recent reply of yours, which was
> >> supporting my view on how things should be (and why things are
> >> broken right now), and hence somewhat contrary to earlier replies
> >> of yours.
> >>
> >> To just name the most unclear (to me) aspect of what's there
> >> currently: What's the distinction between xmm, ymm, and zmm, when
> >> zmm is a superset of ymm (and ymm one of xmm), yet at the same
> >> time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
> >> upper parts) as well. From my observations e.g. an AVX512 insn
> >> accessing just xmm registers will record just an xmm dependency,
> >> which then is simply wrong - the resulting binary in fact depends
> >> on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
> >> back then to tie what an insn records to the XCR0 bits it
> >> requires to be set in order for it to not fault.
> >
> > Should we add a field to i386 opcode table for this?
>
> I was wanting to try without, to see how much logic would be needed.
> If it gets too much, doing so may be the better choice.
I am checking in this patch.

> >>>  Let's see what fallouts will be.
> >>
> >> Fallout may become noticeable only when the behavior of the notes
> >> changes: People may start noticing that new (correct) dependencies
> >> prevent things from working (assuming there's any consumer of
> >> these notes).
> >
> > We have a couple months to address this before 2.36 is released.
>
> Under the assumption that people test non-released code ...
>
We will see.


--
H.J.

0001-x86-Properly-set-YMM-ZMM-features.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Properly set YMM/ZMM features

Sourceware - binutils list mailing list
On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <[hidden email]> wrote:

>
> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <[hidden email]> wrote:
> >
> > On 09.07.2020 17:44, H.J. Lu wrote:
> > > On Thu, Jul 9, 2020 at 8:36 AM Jan Beulich <[hidden email]> wrote:
> > >>
> > >> On 09.07.2020 17:22, H.J. Lu wrote:
> > >>> On Thu, Jul 9, 2020 at 8:21 AM Jan Beulich <[hidden email]> wrote:
> > >>>>
> > >>>> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
> > >>>>>       * configure.ac: Configure with --enable-x86-used-note by default
> > >>>>>       for Linux/x86.
> > >>>>
> > >>>> I'm quite unconvinced this is a good idea as long as the contents of
> > >>>> these notes are neither reliable nor properly settled on what they
> > >>>> actually mean. This should be considered an experimental feature for
> > >>>> now, and hence would better not be enabled by default to avoid
> > >>>> future backwards compatibility issues.
> > >>>>
> > >>>
> > >>> It has been an experimental feature for 2 years.
> > >>
> > >> And when I asked questions about the underlying spec I did get at
> > >> best fuzzy answers from you. I still have on my todo list to get
> > >> this in shape with a pretty recent reply of yours, which was
> > >> supporting my view on how things should be (and why things are
> > >> broken right now), and hence somewhat contrary to earlier replies
> > >> of yours.
> > >>
> > >> To just name the most unclear (to me) aspect of what's there
> > >> currently: What's the distinction between xmm, ymm, and zmm, when
> > >> zmm is a superset of ymm (and ymm one of xmm), yet at the same
> > >> time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
> > >> upper parts) as well. From my observations e.g. an AVX512 insn
> > >> accessing just xmm registers will record just an xmm dependency,
> > >> which then is simply wrong - the resulting binary in fact depends
> > >> on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
> > >> back then to tie what an insn records to the XCR0 bits it
> > >> requires to be set in order for it to not fault.
> > >
> > > Should we add a field to i386 opcode table for this?
> >
> > I was wanting to try without, to see how much logic would be needed.
> > If it gets too much, doing so may be the better choice.
>
> I am checking in this patch.
Here is the right patch.

> > >>>  Let's see what fallouts will be.
> > >>
> > >> Fallout may become noticeable only when the behavior of the notes
> > >> changes: People may start noticing that new (correct) dependencies
> > >> prevent things from working (assuming there's any consumer of
> > >> these notes).
> > >
> > > We have a couple months to address this before 2.36 is released.
> >
> > Under the assumption that people test non-released code ...
> >
>
> We will see.
>
>
> --
> H.J.


--
H.J.

0001-x86-Properly-set-YMM-ZMM-features.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Properly set YMM/ZMM features

Jan Beulich-2
On 09.07.2020 18:22, H.J. Lu wrote:

> On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <[hidden email]> wrote:
>> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <[hidden email]> wrote:
>>> On 09.07.2020 17:44, H.J. Lu wrote:
>>>> Should we add a field to i386 opcode table for this?
>>>
>>> I was wanting to try without, to see how much logic would be needed.
>>> If it gets too much, doing so may be the better choice.
>>
>> I am checking in this patch.
>
> Here is the right patch.

Well, this is certainly an improvement. But it doesn't come close to
what's needed. Again - imo the concept of i.has_reg* isn't helpful
at all. It's not the registers an insn _actually_ accesses, but the
ones the template says it might access. Think of vfpclass* with a
memory operand, for example. Or see some of the extra checks of
certain opcodes when setting the MMX bit, which could be avoided by
using such an alternative model.

I'm leaving aside here special cases where the templates don't carry
enough information. I'm also leaving aside the need to e.g. record
mask register use in some form (maybe translating to ZMM use if you
want to avoid introducing a separate tracking bit), as well as
various other issues I had spotted without looking very closely yet.

I believe a fundamental issue here is the very scarce testing: While
it may be avoidable to add an attribute to the insn templates, there
should be full testing coverage of _all_ insns with _all_ possible
operand type combinations, if the resulting notes are to be
dependable. If such (presumably table based) testing was there (and
of course if the table reflected the real needs rather than what
gas currently happens to produce), you'd easily notice the issues
I've spotted (and likely more).

Jan
Reply | Threaded
Open this post in threaded view
|

[PATCH] x86: Extract extended states from instruction template

Sourceware - binutils list mailing list
On Fri, Jul 10, 2020 at 12:02 AM Jan Beulich <[hidden email]> wrote:

>
> On 09.07.2020 18:22, H.J. Lu wrote:
> > On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <[hidden email]> wrote:
> >> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <[hidden email]> wrote:
> >>> On 09.07.2020 17:44, H.J. Lu wrote:
> >>>> Should we add a field to i386 opcode table for this?
> >>>
> >>> I was wanting to try without, to see how much logic would be needed.
> >>> If it gets too much, doing so may be the better choice.
> >>
> >> I am checking in this patch.
> >
> > Here is the right patch.
>
> Well, this is certainly an improvement. But it doesn't come close to
> what's needed. Again - imo the concept of i.has_reg* isn't helpful
> at all. It's not the registers an insn _actually_ accesses, but the
> ones the template says it might access. Think of vfpclass* with a
> memory operand, for example. Or see some of the extra checks of
> certain opcodes when setting the MMX bit, which could be avoided by
> using such an alternative model.
>
> I'm leaving aside here special cases where the templates don't carry
> enough information. I'm also leaving aside the need to e.g. record
> mask register use in some form (maybe translating to ZMM use if you
> want to avoid introducing a separate tracking bit), as well as
> various other issues I had spotted without looking very closely yet.
Here is a patch to extract extended states from operand types in
instruction template and set xstate_zmm for master register move.

> I believe a fundamental issue here is the very scarce testing: While
> it may be avoidable to add an attribute to the insn templates, there
> should be full testing coverage of _all_ insns with _all_ possible
> operand type combinations, if the resulting notes are to be
> dependable. If such (presumably table based) testing was there (and
> of course if the table reflected the real needs rather than what
> gas currently happens to produce), you'd easily notice the issues
> I've spotted (and likely more).

This is just a starting point.  I will fix any issues discovered.

Thanks.

--
H.J.

0001-x86-Extract-extended-states-from-instruction-templat.patch (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Extract extended states from instruction template

Jan Beulich-2
On 10.07.2020 15:41, H.J. Lu wrote:

> On Fri, Jul 10, 2020 at 12:02 AM Jan Beulich <[hidden email]> wrote:
>>
>> On 09.07.2020 18:22, H.J. Lu wrote:
>>> On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <[hidden email]> wrote:
>>>> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <[hidden email]> wrote:
>>>>> On 09.07.2020 17:44, H.J. Lu wrote:
>>>>>> Should we add a field to i386 opcode table for this?
>>>>>
>>>>> I was wanting to try without, to see how much logic would be needed.
>>>>> If it gets too much, doing so may be the better choice.
>>>>
>>>> I am checking in this patch.
>>>
>>> Here is the right patch.
>>
>> Well, this is certainly an improvement. But it doesn't come close to
>> what's needed. Again - imo the concept of i.has_reg* isn't helpful
>> at all. It's not the registers an insn _actually_ accesses, but the
>> ones the template says it might access. Think of vfpclass* with a
>> memory operand, for example. Or see some of the extra checks of
>> certain opcodes when setting the MMX bit, which could be avoided by
>> using such an alternative model.
>>
>> I'm leaving aside here special cases where the templates don't carry
>> enough information. I'm also leaving aside the need to e.g. record
>> mask register use in some form (maybe translating to ZMM use if you
>> want to avoid introducing a separate tracking bit), as well as
>> various other issues I had spotted without looking very closely yet.
>
> Here is a patch to extract extended states from operand types in
> instruction template and set xstate_zmm for master register move.

Ah yes, I appreciate this move.

>> I believe a fundamental issue here is the very scarce testing: While
>> it may be avoidable to add an attribute to the insn templates, there
>> should be full testing coverage of _all_ insns with _all_ possible
>> operand type combinations, if the resulting notes are to be
>> dependable. If such (presumably table based) testing was there (and
>> of course if the table reflected the real needs rather than what
>> gas currently happens to produce), you'd easily notice the issues
>> I've spotted (and likely more).
>
> This is just a starting point.  I will fix any issues discovered.

I'll see to get to running this over the various little examples I
have, once it got committed (and I got around to re-basing).

I don't think altering existing test cases is a good idea though -
accumulating the flags from multiple instructions will leave it
unclear which insn caused which flag(s) to get set. As said before,
I am of the pretty strong opinion that testsuite-wise the only
helpful thing here is to individually test insns (and in various
cases even the same insn with varying operands).

Jan
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Extract extended states from instruction template

Sourceware - binutils list mailing list
On Fri, Jul 10, 2020 at 8:17 AM Jan Beulich <[hidden email]> wrote:

>
> On 10.07.2020 15:41, H.J. Lu wrote:
> > On Fri, Jul 10, 2020 at 12:02 AM Jan Beulich <[hidden email]> wrote:
> >>
> >> On 09.07.2020 18:22, H.J. Lu wrote:
> >>> On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <[hidden email]> wrote:
> >>>> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <[hidden email]> wrote:
> >>>>> On 09.07.2020 17:44, H.J. Lu wrote:
> >>>>>> Should we add a field to i386 opcode table for this?
> >>>>>
> >>>>> I was wanting to try without, to see how much logic would be needed.
> >>>>> If it gets too much, doing so may be the better choice.
> >>>>
> >>>> I am checking in this patch.
> >>>
> >>> Here is the right patch.
> >>
> >> Well, this is certainly an improvement. But it doesn't come close to
> >> what's needed. Again - imo the concept of i.has_reg* isn't helpful
> >> at all. It's not the registers an insn _actually_ accesses, but the
> >> ones the template says it might access. Think of vfpclass* with a
> >> memory operand, for example. Or see some of the extra checks of
> >> certain opcodes when setting the MMX bit, which could be avoided by
> >> using such an alternative model.
> >>
> >> I'm leaving aside here special cases where the templates don't carry
> >> enough information. I'm also leaving aside the need to e.g. record
> >> mask register use in some form (maybe translating to ZMM use if you
> >> want to avoid introducing a separate tracking bit), as well as
> >> various other issues I had spotted without looking very closely yet.
> >
> > Here is a patch to extract extended states from operand types in
> > instruction template and set xstate_zmm for master register move.
>
> Ah yes, I appreciate this move.
>
> >> I believe a fundamental issue here is the very scarce testing: While
> >> it may be avoidable to add an attribute to the insn templates, there
> >> should be full testing coverage of _all_ insns with _all_ possible
> >> operand type combinations, if the resulting notes are to be
> >> dependable. If such (presumably table based) testing was there (and
> >> of course if the table reflected the real needs rather than what
> >> gas currently happens to produce), you'd easily notice the issues
> >> I've spotted (and likely more).
> >
> > This is just a starting point.  I will fix any issues discovered.
>
> I'll see to get to running this over the various little examples I
> have, once it got committed (and I got around to re-basing).
>
> I don't think altering existing test cases is a good idea though -
> accumulating the flags from multiple instructions will leave it
> unclear which insn caused which flag(s) to get set. As said before,
> I am of the pretty strong opinion that testsuite-wise the only
> helpful thing here is to individually test insns (and in various
> cases even the same insn with varying operands).
Fixed.

This is the updated patch I am checking in.

Thanks.

--
H.J.

0001-x86-Extract-extended-states-from-instruction-templat.patch (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Michael Matz
In reply to this post by Sourceware - binutils list mailing list
Hello,

On Thu, 9 Jul 2020, H.J. Lu via Binutils wrote:

> > >> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
> > >>>       * configure.ac: Configure with --enable-x86-used-note by default
> > >>>       for Linux/x86.
> > >>
> > >> I'm quite unconvinced this is a good idea as long as the contents of
> > >> these notes are neither reliable nor properly settled on what they
> > >> actually mean. This should be considered an experimental feature for
> > >> now, and hence would better not be enabled by default to avoid
> > >> future backwards compatibility issues.
> > >>
> > >
> > > It has been an experimental feature for 2 years.
> >
> > And when I asked questions about the underlying spec I did get at
> > best fuzzy answers from you. I still have on my todo list to get
> > this in shape with a pretty recent reply of yours, which was
> > supporting my view on how things should be (and why things are
> > broken right now), and hence somewhat contrary to earlier replies
> > of yours.
> >
> > To just name the most unclear (to me) aspect of what's there
> > currently: What's the distinction between xmm, ymm, and zmm, when
> > zmm is a superset of ymm (and ymm one of xmm), yet at the same
> > time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
> > upper parts) as well. From my observations e.g. an AVX512 insn
> > accessing just xmm registers will record just an xmm dependency,
> > which then is simply wrong - the resulting binary in fact depends
> > on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
> > back then to tie what an insn records to the XCR0 bits it
> > requires to be set in order for it to not fault.
>
> Should we add a field to i386 opcode table for this?
>
> > >  Let's see what fallouts will be.
> >
> > Fallout may become noticeable only when the behavior of the notes
> > changes: People may start noticing that new (correct) dependencies
> > prevent things from working (assuming there's any consumer of
> > these notes).
> >
>
> We have a couple months to address this before 2.36 is released.

FWIW, as data point, if this option becomes default for 2.35 I'll patch it
out again for our distro binutils.  I really really want to avoid
spreading binaries with these notes into the wild in the current state.  
Whenever I look at them my guts grumble and I think they are ill-defined.  

The past attempts to clarify them (or to create better defined
alternatives) never lead to anything like a agreement amongst affected
parties, so this change in default seems to be work-around for that by
creating unchangable facts.


Ciao,
Michael.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Sourceware - binutils list mailing list
On Mon, Jul 13, 2020 at 7:09 AM Michael Matz <[hidden email]> wrote:

>
> Hello,
>
> On Thu, 9 Jul 2020, H.J. Lu via Binutils wrote:
>
> > > >> On 09.07.2020 17:02, H.J. Lu via Binutils wrote:
> > > >>>       * configure.ac: Configure with --enable-x86-used-note by default
> > > >>>       for Linux/x86.
> > > >>
> > > >> I'm quite unconvinced this is a good idea as long as the contents of
> > > >> these notes are neither reliable nor properly settled on what they
> > > >> actually mean. This should be considered an experimental feature for
> > > >> now, and hence would better not be enabled by default to avoid
> > > >> future backwards compatibility issues.
> > > >>
> > > >
> > > > It has been an experimental feature for 2 years.
> > >
> > > And when I asked questions about the underlying spec I did get at
> > > best fuzzy answers from you. I still have on my todo list to get
> > > this in shape with a pretty recent reply of yours, which was
> > > supporting my view on how things should be (and why things are
> > > broken right now), and hence somewhat contrary to earlier replies
> > > of yours.
> > >
> > > To just name the most unclear (to me) aspect of what's there
> > > currently: What's the distinction between xmm, ymm, and zmm, when
> > > zmm is a superset of ymm (and ymm one of xmm), yet at the same
> > > time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
> > > upper parts) as well. From my observations e.g. an AVX512 insn
> > > accessing just xmm registers will record just an xmm dependency,
> > > which then is simply wrong - the resulting binary in fact depends
> > > on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
> > > back then to tie what an insn records to the XCR0 bits it
> > > requires to be set in order for it to not fault.
> >
> > Should we add a field to i386 opcode table for this?
> >
> > > >  Let's see what fallouts will be.
> > >
> > > Fallout may become noticeable only when the behavior of the notes
> > > changes: People may start noticing that new (correct) dependencies
> > > prevent things from working (assuming there's any consumer of
> > > these notes).
> > >
> >
> > We have a couple months to address this before 2.36 is released.
>
> FWIW, as data point, if this option becomes default for 2.35 I'll patch it
> out again for our distro binutils.  I really really want to avoid
> spreading binaries with these notes into the wild in the current state.
> Whenever I look at them my guts grumble and I think they are ill-defined.
>
> The past attempts to clarify them (or to create better defined
> alternatives) never lead to anything like a agreement amongst affected
> parties, so this change in default seems to be work-around for that by
> creating unchangable facts.
>

Please open bugs for issues you find.

Thanks.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Michael Matz
Hello,

On Mon, 13 Jul 2020, H.J. Lu wrote:

> > > > >> I'm quite unconvinced this is a good idea as long as the contents of
> > > > >> these notes are neither reliable nor properly settled on what they
> > > > >> actually mean. This should be considered an experimental feature for
> > > > >> now, and hence would better not be enabled by default to avoid
> > > > >> future backwards compatibility issues.
> > > > >>
> > > > >
> > > > > It has been an experimental feature for 2 years.
> > > >
> > > > And when I asked questions about the underlying spec I did get at
> > > > best fuzzy answers from you. I still have on my todo list to get
> > > > this in shape with a pretty recent reply of yours, which was
> > > > supporting my view on how things should be (and why things are
> > > > broken right now), and hence somewhat contrary to earlier replies
> > > > of yours.
> > > >
> > > > To just name the most unclear (to me) aspect of what's there
> > > > currently: What's the distinction between xmm, ymm, and zmm, when
> > > > zmm is a superset of ymm (and ymm one of xmm), yet at the same
> > > > time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
> > > > upper parts) as well. From my observations e.g. an AVX512 insn
> > > > accessing just xmm registers will record just an xmm dependency,
> > > > which then is simply wrong - the resulting binary in fact depends
> > > > on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
> > > > back then to tie what an insn records to the XCR0 bits it
> > > > requires to be set in order for it to not fault.
> > >
> > > Should we add a field to i386 opcode table for this?
> > >
> > > > >  Let's see what fallouts will be.
> > > >
> > > > Fallout may become noticeable only when the behavior of the notes
> > > > changes: People may start noticing that new (correct) dependencies
> > > > prevent things from working (assuming there's any consumer of
> > > > these notes).
> > > >
> > >
> > > We have a couple months to address this before 2.36 is released.
> >
> > FWIW, as data point, if this option becomes default for 2.35 I'll patch it
> > out again for our distro binutils.  I really really want to avoid
> > spreading binaries with these notes into the wild in the current state.
> > Whenever I look at them my guts grumble and I think they are ill-defined.
> >
> > The past attempts to clarify them (or to create better defined
> > alternatives) never lead to anything like a agreement amongst affected
> > parties, so this change in default seems to be work-around for that by
> > creating unchangable facts.
> >
>
> Please open bugs for issues you find.
>
> Thanks.

I'm not talking about bugs in the implementation.  I'm talking about the
whole design of these notes.  Over the years I tried multiple times to
start a discussion about that, it never lead to anywhere and I can't
imagine this changing if I start again, so I'll not.  But I'll do whatever
I can to prevent spreading binaries containing those notes.


Ciao,
Michael.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Sourceware - binutils list mailing list
On Mon, Jul 13, 2020 at 8:48 AM Michael Matz <[hidden email]> wrote:

>
> Hello,
>
> On Mon, 13 Jul 2020, H.J. Lu wrote:
>
> > > > > >> I'm quite unconvinced this is a good idea as long as the contents of
> > > > > >> these notes are neither reliable nor properly settled on what they
> > > > > >> actually mean. This should be considered an experimental feature for
> > > > > >> now, and hence would better not be enabled by default to avoid
> > > > > >> future backwards compatibility issues.
> > > > > >>
> > > > > >
> > > > > > It has been an experimental feature for 2 years.
> > > > >
> > > > > And when I asked questions about the underlying spec I did get at
> > > > > best fuzzy answers from you. I still have on my todo list to get
> > > > > this in shape with a pretty recent reply of yours, which was
> > > > > supporting my view on how things should be (and why things are
> > > > > broken right now), and hence somewhat contrary to earlier replies
> > > > > of yours.
> > > > >
> > > > > To just name the most unclear (to me) aspect of what's there
> > > > > currently: What's the distinction between xmm, ymm, and zmm, when
> > > > > zmm is a superset of ymm (and ymm one of xmm), yet at the same
> > > > > time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
> > > > > upper parts) as well. From my observations e.g. an AVX512 insn
> > > > > accessing just xmm registers will record just an xmm dependency,
> > > > > which then is simply wrong - the resulting binary in fact depends
> > > > > on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
> > > > > back then to tie what an insn records to the XCR0 bits it
> > > > > requires to be set in order for it to not fault.
> > > >
> > > > Should we add a field to i386 opcode table for this?
> > > >
> > > > > >  Let's see what fallouts will be.
> > > > >
> > > > > Fallout may become noticeable only when the behavior of the notes
> > > > > changes: People may start noticing that new (correct) dependencies
> > > > > prevent things from working (assuming there's any consumer of
> > > > > these notes).
> > > > >
> > > >
> > > > We have a couple months to address this before 2.36 is released.
> > >
> > > FWIW, as data point, if this option becomes default for 2.35 I'll patch it
> > > out again for our distro binutils.  I really really want to avoid
> > > spreading binaries with these notes into the wild in the current state.
> > > Whenever I look at them my guts grumble and I think they are ill-defined.
> > >
> > > The past attempts to clarify them (or to create better defined
> > > alternatives) never lead to anything like a agreement amongst affected
> > > parties, so this change in default seems to be work-around for that by
> > > creating unchangable facts.
> > >
> >
> > Please open bugs for issues you find.
> >
> > Thanks.
>
> I'm not talking about bugs in the implementation.  I'm talking about the
> whole design of these notes.  Over the years I tried multiple times to
> start a discussion about that, it never lead to anywhere and I can't
> imagine this changing if I start again, so I'll not.  But I'll do whatever
> I can to prevent spreading binaries containing those notes.
>

There is a potential use case now:

https://sourceware.org/pipermail/libc-alpha/2020-July/116135.html


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Linux/x86: Configure gas with --enable-x86-used-note by default

Jan Beulich-2
On 13.07.2020 18:06, H.J. Lu wrote:

> On Mon, Jul 13, 2020 at 8:48 AM Michael Matz <[hidden email]> wrote:
>>
>> Hello,
>>
>> On Mon, 13 Jul 2020, H.J. Lu wrote:
>>
>>>>>>>> I'm quite unconvinced this is a good idea as long as the contents of
>>>>>>>> these notes are neither reliable nor properly settled on what they
>>>>>>>> actually mean. This should be considered an experimental feature for
>>>>>>>> now, and hence would better not be enabled by default to avoid
>>>>>>>> future backwards compatibility issues.
>>>>>>>>
>>>>>>>
>>>>>>> It has been an experimental feature for 2 years.
>>>>>>
>>>>>> And when I asked questions about the underlying spec I did get at
>>>>>> best fuzzy answers from you. I still have on my todo list to get
>>>>>> this in shape with a pretty recent reply of yours, which was
>>>>>> supporting my view on how things should be (and why things are
>>>>>> broken right now), and hence somewhat contrary to earlier replies
>>>>>> of yours.
>>>>>>
>>>>>> To just name the most unclear (to me) aspect of what's there
>>>>>> currently: What's the distinction between xmm, ymm, and zmm, when
>>>>>> zmm is a superset of ymm (and ymm one of xmm), yet at the same
>>>>>> time AVX512 writes to xmm mean writes to ymm and zmm (zeroing
>>>>>> upper parts) as well. From my observations e.g. an AVX512 insn
>>>>>> accessing just xmm registers will record just an xmm dependency,
>>>>>> which then is simply wrong - the resulting binary in fact depends
>>>>>> on all of xmm, ymm, zmm, and the mask registers. IIRC I proposed
>>>>>> back then to tie what an insn records to the XCR0 bits it
>>>>>> requires to be set in order for it to not fault.
>>>>>
>>>>> Should we add a field to i386 opcode table for this?
>>>>>
>>>>>>>  Let's see what fallouts will be.
>>>>>>
>>>>>> Fallout may become noticeable only when the behavior of the notes
>>>>>> changes: People may start noticing that new (correct) dependencies
>>>>>> prevent things from working (assuming there's any consumer of
>>>>>> these notes).
>>>>>>
>>>>>
>>>>> We have a couple months to address this before 2.36 is released.
>>>>
>>>> FWIW, as data point, if this option becomes default for 2.35 I'll patch it
>>>> out again for our distro binutils.  I really really want to avoid
>>>> spreading binaries with these notes into the wild in the current state.
>>>> Whenever I look at them my guts grumble and I think they are ill-defined.
>>>>
>>>> The past attempts to clarify them (or to create better defined
>>>> alternatives) never lead to anything like a agreement amongst affected
>>>> parties, so this change in default seems to be work-around for that by
>>>> creating unchangable facts.
>>>>
>>>
>>> Please open bugs for issues you find.
>>>
>>> Thanks.
>>
>> I'm not talking about bugs in the implementation.  I'm talking about the
>> whole design of these notes.  Over the years I tried multiple times to
>> start a discussion about that, it never lead to anywhere and I can't
>> imagine this changing if I start again, so I'll not.  But I'll do whatever
>> I can to prevent spreading binaries containing those notes.
>>
>
> There is a potential use case now:
>
> https://sourceware.org/pipermail/libc-alpha/2020-July/116135.html

I'm sorry for repeating myself, but imo it is a _prereq_ for such a
use case that things are both well specified and sufficiently well
implemented (see e.g. my remark regarding testsuite coverage).

Jan