[PATCH] x86: actually implement .noopt

[PATCH] x86: actually implement .noopt

[Jan Beulich]

For quite some time we've had support for -O command line options. With
that ignoring at least .noopt isn't really a good idea.

Re-purpose the optimize-3 test for testing this directive's effect as
well.

As to the doc addition - this uses the same text as is there for the
{nooptimize} pseudo-prefix, despite me not being convinced of the "size"
part being fully accurate there (and hence also here).
---
I haven't been able to find any applicable documentation for other
(perhaps legacy) assemblers' support of .noopt (and .optim). An option
to also handle .optim would be to
- restore previously saved settings (if optimization was enabled on the
  command line),
- enable a pre-determined level of optimization if optimization was not
  enabled on the command line.
Thoughts?

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -139,6 +139,7 @@ arch_entry;
 
 static void update_code_flag (int, int);
 static void s_insn (int);
+static void s_noopt (int);
 static void set_code_flag (int);
 static void set_16bit_gcc_code_flag (int);
 static void set_intel_syntax (int);
@@ -1231,7 +1232,7 @@ const pseudo_typeS md_pseudo_table[] =
   {"value", cons, 2},
   {"slong", signed_cons, 4},
   {"insn", s_insn, 0},
-  {"noopt", s_ignore, 0},
+  {"noopt", s_noopt, 0},
   {"optim", s_ignore, 0},
   {"code16gcc", set_16bit_gcc_code_flag, CODE_16BIT},
   {"code16", set_code_flag, CODE_16BIT},
@@ -4966,6 +4967,13 @@ optimize_encoding (void)
     }
 }
 
+static void
+s_noopt (int dummy ATTRIBUTE_UNUSED)
+{
+  optimize = 0;
+  optimize_for_space = 0;
+}
+
 /* Return non-zero for load instruction.  */
 
 static int
--- a/gas/doc/c-i386.texi
+++ b/gas/doc/c-i386.texi
@@ -763,6 +763,10 @@ be suffixed by @code{@{:d@var{n}@}} to s
 This can be combined with an embedded broadcast specifier:
 @samp{8(%eax)@{1to8:d8@}}.
 
+@cindex @code{noopt} directive
+@item .noopt
+Disable instruction size optimization.
+
 @c FIXME: Document other x86 specific directives ?  Eg: .code16gcc,
 
 @end table
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -614,6 +614,7 @@ if [gas_32_check] then {
     run_list_test "optimize-6a" "-I${srcdir}/$subdir -march=+noavx -al"
     run_dump_test "optimize-6b"
     run_list_test "optimize-7" "-I${srcdir}/$subdir -march=+noavx2 -al"
+    run_dump_test "noopt"
     run_dump_test "lea-optimize"
     run_dump_test "lea16-optimize"
     run_dump_test "lea16-optimize2"
--- /dev/null
+++ b/gas/testsuite/gas/i386/noopt.d
@@ -0,0 +1,4 @@
+#as: -Os -I${srcdir}/$subdir
+#objdump: -drw
+#name: .noopt directive
+#dump: optimize-3.d
--- /dev/null
+++ b/gas/testsuite/gas/i386/noopt.s
@@ -0,0 +1,2 @@
+	.noopt
+	.include "optimize-3.s"
--- a/gas/testsuite/gas/i386/optimize-3.d
+++ b/gas/testsuite/gas/i386/optimize-3.d
@@ -1,4 +1,4 @@
-#as: -Os
+#as: -Os --defsym USE_PREFIX=1
 #objdump: -drw
 #name: optimized encoding 3 with -Os
 
--- a/gas/testsuite/gas/i386/optimize-3.s
+++ b/gas/testsuite/gas/i386/optimize-3.s
@@ -1,24 +1,32 @@
 # Check instructions with optimized encoding
 
+	.macro noopt insn:vararg
+	.ifdef USE_PREFIX
+	{nooptimize} \insn
+	.else
+	\insn
+	.endif
+	.endm
+
 	.text
 _start:
-	{nooptimize} testl $0x7f, %eax
+	noopt testl $0x7f, %eax
 
-	{nooptimize} lock xchg	%ecx, (%edx)
-	{nooptimize} lock xchg	(%ecx), %edx
+	noopt lock xchg	%ecx, (%edx)
+	noopt lock xchg	(%ecx), %edx
 
-	{nooptimize} vmovdqa32	%ymm1, %ymm2
-	{nooptimize} vmovdqa64	%ymm1, %ymm2
-	{nooptimize} vmovdqu8	%xmm1, %xmm2
-	{nooptimize} vmovdqu16	%xmm1, %xmm2
-	{nooptimize} vmovdqu32	%xmm1, %xmm2
-	{nooptimize} vmovdqu64	%xmm1, %xmm2
+	noopt vmovdqa32	%ymm1, %ymm2
+	noopt vmovdqa64	%ymm1, %ymm2
+	noopt vmovdqu8	%xmm1, %xmm2
+	noopt vmovdqu16	%xmm1, %xmm2
+	noopt vmovdqu32	%xmm1, %xmm2
+	noopt vmovdqu64	%xmm1, %xmm2
 
-	{nooptimize} vpandd	%xmm2, %xmm3, %xmm4
-	{nooptimize} vpandq	%ymm2, %ymm3, %ymm4
-	{nooptimize} vpandnd	%ymm2, %ymm3, %ymm4
-	{nooptimize} vpandnq	%xmm2, %xmm3, %xmm4
-	{nooptimize} vpord	%xmm2, %xmm3, %xmm4
-	{nooptimize} vporq	%ymm2, %ymm3, %ymm4
-	{nooptimize} vpxord	%ymm2, %ymm3, %ymm4
-	{nooptimize} vpxorq	%xmm2, %xmm3, %xmm4
+	noopt vpandd	%xmm2, %xmm3, %xmm4
+	noopt vpandq	%ymm2, %ymm3, %ymm4
+	noopt vpandnd	%ymm2, %ymm3, %ymm4
+	noopt vpandnq	%xmm2, %xmm3, %xmm4
+	noopt vpord	%xmm2, %xmm3, %xmm4
+	noopt vporq	%ymm2, %ymm3, %ymm4
+	noopt vpxord	%ymm2, %ymm3, %ymm4
+	noopt vpxorq	%xmm2, %xmm3, %xmm4

Re: [PATCH] x86: actually implement .noopt

[Fangrui Song]

On Fri, Jan 5, 2024 at 12:31 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> For quite some time we've had support for -O command line options. With
> that ignoring at least .noopt isn't really a good idea.
>
> Re-purpose the optimize-3 test for testing this directive's effect as
> well.
>
> As to the doc addition - this uses the same text as is there for the
> {nooptimize} pseudo-prefix, despite me not being convinced of the "size"
> part being fully accurate there (and hence also here).
> ---
> I haven't been able to find any applicable documentation for other
> (perhaps legacy) assemblers' support of .noopt (and .optim). An option
> to also handle .optim would be to
> - restore previously saved settings (if optimization was enabled on the
>   command line),
> - enable a pre-determined level of optimization if optimization was not
>   enabled on the command line.
> Thoughts?

Interesting. This is from
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b6f8c7c45229a8a5405079e586bfbaad396d2cbe
("x86: Add -O[2|s] assembler command-line options").

If push/pop are to be added, it seems clear if push/pop/setting use
the same directive like .noopt push, .noopt pop, .noopt optimize,
.noopt nooptimize, but the negative directive name now causes
confusion.
In addition, "opt" option means "option" (mips/riscv have .option).
Perhaps switch to a more meaningful directive name?

> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -139,6 +139,7 @@ arch_entry;
>
>  static void update_code_flag (int, int);
>  static void s_insn (int);
> +static void s_noopt (int);
>  static void set_code_flag (int);
>  static void set_16bit_gcc_code_flag (int);
>  static void set_intel_syntax (int);
> @@ -1231,7 +1232,7 @@ const pseudo_typeS md_pseudo_table[] =
>    {"value", cons, 2},
>    {"slong", signed_cons, 4},
>    {"insn", s_insn, 0},
> -  {"noopt", s_ignore, 0},
> +  {"noopt", s_noopt, 0},
>    {"optim", s_ignore, 0},
>    {"code16gcc", set_16bit_gcc_code_flag, CODE_16BIT},
>    {"code16", set_code_flag, CODE_16BIT},
> @@ -4966,6 +4967,13 @@ optimize_encoding (void)
>      }
>  }
>
> +static void
> +s_noopt (int dummy ATTRIBUTE_UNUSED)
> +{
> +  optimize = 0;
> +  optimize_for_space = 0;
> +}
> +
>  /* Return non-zero for load instruction.  */
>
>  static int
> --- a/gas/doc/c-i386.texi
> +++ b/gas/doc/c-i386.texi
> @@ -763,6 +763,10 @@ be suffixed by @code{@{:d@var{n}@}} to s
>  This can be combined with an embedded broadcast specifier:
>  @samp{8(%eax)@{1to8:d8@}}.
>
> +@cindex @code{noopt} directive
> +@item .noopt
> +Disable instruction size optimization.
> +
>  @c FIXME: Document other x86 specific directives ?  Eg: .code16gcc,
>
>  @end table
> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -614,6 +614,7 @@ if [gas_32_check] then {
>      run_list_test "optimize-6a" "-I${srcdir}/$subdir -march=+noavx -al"
>      run_dump_test "optimize-6b"
>      run_list_test "optimize-7" "-I${srcdir}/$subdir -march=+noavx2 -al"
> +    run_dump_test "noopt"
>      run_dump_test "lea-optimize"
>      run_dump_test "lea16-optimize"
>      run_dump_test "lea16-optimize2"
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/noopt.d
> @@ -0,0 +1,4 @@
> +#as: -Os -I${srcdir}/$subdir
> +#objdump: -drw
> +#name: .noopt directive
> +#dump: optimize-3.d
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/noopt.s
> @@ -0,0 +1,2 @@
> +       .noopt
> +       .include "optimize-3.s"
> --- a/gas/testsuite/gas/i386/optimize-3.d
> +++ b/gas/testsuite/gas/i386/optimize-3.d
> @@ -1,4 +1,4 @@
> -#as: -Os
> +#as: -Os --defsym USE_PREFIX=1
>  #objdump: -drw
>  #name: optimized encoding 3 with -Os
>
> --- a/gas/testsuite/gas/i386/optimize-3.s
> +++ b/gas/testsuite/gas/i386/optimize-3.s
> @@ -1,24 +1,32 @@
>  # Check instructions with optimized encoding
>
> +       .macro noopt insn:vararg
> +       .ifdef USE_PREFIX
> +       {nooptimize} \insn
> +       .else
> +       \insn
> +       .endif
> +       .endm
> +
>         .text
>  _start:
> -       {nooptimize} testl $0x7f, %eax
> +       noopt testl $0x7f, %eax
>
> -       {nooptimize} lock xchg  %ecx, (%edx)
> -       {nooptimize} lock xchg  (%ecx), %edx
> +       noopt lock xchg %ecx, (%edx)
> +       noopt lock xchg (%ecx), %edx
>
> -       {nooptimize} vmovdqa32  %ymm1, %ymm2
> -       {nooptimize} vmovdqa64  %ymm1, %ymm2
> -       {nooptimize} vmovdqu8   %xmm1, %xmm2
> -       {nooptimize} vmovdqu16  %xmm1, %xmm2
> -       {nooptimize} vmovdqu32  %xmm1, %xmm2
> -       {nooptimize} vmovdqu64  %xmm1, %xmm2
> +       noopt vmovdqa32 %ymm1, %ymm2
> +       noopt vmovdqa64 %ymm1, %ymm2
> +       noopt vmovdqu8  %xmm1, %xmm2
> +       noopt vmovdqu16 %xmm1, %xmm2
> +       noopt vmovdqu32 %xmm1, %xmm2
> +       noopt vmovdqu64 %xmm1, %xmm2
>
> -       {nooptimize} vpandd     %xmm2, %xmm3, %xmm4
> -       {nooptimize} vpandq     %ymm2, %ymm3, %ymm4
> -       {nooptimize} vpandnd    %ymm2, %ymm3, %ymm4
> -       {nooptimize} vpandnq    %xmm2, %xmm3, %xmm4
> -       {nooptimize} vpord      %xmm2, %xmm3, %xmm4
> -       {nooptimize} vporq      %ymm2, %ymm3, %ymm4
> -       {nooptimize} vpxord     %ymm2, %ymm3, %ymm4
> -       {nooptimize} vpxorq     %xmm2, %xmm3, %xmm4
> +       noopt vpandd    %xmm2, %xmm3, %xmm4
> +       noopt vpandq    %ymm2, %ymm3, %ymm4
> +       noopt vpandnd   %ymm2, %ymm3, %ymm4
> +       noopt vpandnq   %xmm2, %xmm3, %xmm4
> +       noopt vpord     %xmm2, %xmm3, %xmm4
> +       noopt vporq     %ymm2, %ymm3, %ymm4
> +       noopt vpxord    %ymm2, %ymm3, %ymm4
> +       noopt vpxorq    %xmm2, %xmm3, %xmm4

Re: [PATCH] x86: actually implement .noopt

[Jan Beulich]

On 08.01.2024 02:18, Fangrui Song wrote:
> On Fri, Jan 5, 2024 at 12:31 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> For quite some time we've had support for -O command line options. With
>> that ignoring at least .noopt isn't really a good idea.
>>
>> Re-purpose the optimize-3 test for testing this directive's effect as
>> well.
>>
>> As to the doc addition - this uses the same text as is there for the
>> {nooptimize} pseudo-prefix, despite me not being convinced of the "size"
>> part being fully accurate there (and hence also here).
>> ---
>> I haven't been able to find any applicable documentation for other
>> (perhaps legacy) assemblers' support of .noopt (and .optim). An option
>> to also handle .optim would be to
>> - restore previously saved settings (if optimization was enabled on the
>>   command line),
>> - enable a pre-determined level of optimization if optimization was not
>>   enabled on the command line.
>> Thoughts?
> 
> Interesting. This is from
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b6f8c7c45229a8a5405079e586bfbaad396d2cbe
> ("x86: Add -O[2|s] assembler command-line options").

I'm confused by your reply: How is the above related (other than being,
as stated, the reason for wanting to no longer ignore .noopt)?

> If push/pop are to be added, it seems clear if push/pop/setting use
> the same directive like .noopt push, .noopt pop, .noopt optimize,
> .noopt nooptimize, but the negative directive name now causes
> confusion.
> In addition, "opt" option means "option" (mips/riscv have .option).
> Perhaps switch to a more meaningful directive name?

Well, did you look at, in particular, ...

>> @@ -1231,7 +1232,7 @@ const pseudo_typeS md_pseudo_table[] =
>>    {"value", cons, 2},
>>    {"slong", signed_cons, 4},
>>    {"insn", s_insn, 0},
>> -  {"noopt", s_ignore, 0},
>> +  {"noopt", s_noopt, 0},
>>    {"optim", s_ignore, 0},
>>    {"code16gcc", set_16bit_gcc_code_flag, CODE_16BIT},
>>    {"code16", set_code_flag, CODE_16BIT},

... this hunk? The respective "positive" directive is .optim. Yet
implementing that without knowing its origin seemed less desirable to
me. If we were entirely free, I certainly would agree with having
push/pop forms.

Jan

Re: [PATCH] x86: actually implement .noopt

[Fangrui Song]

On Sun, Jan 7, 2024 at 11:48 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.01.2024 02:18, Fangrui Song wrote:
> > On Fri, Jan 5, 2024 at 12:31 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> For quite some time we've had support for -O command line options. With
> >> that ignoring at least .noopt isn't really a good idea.
> >>
> >> Re-purpose the optimize-3 test for testing this directive's effect as
> >> well.
> >>
> >> As to the doc addition - this uses the same text as is there for the
> >> {nooptimize} pseudo-prefix, despite me not being convinced of the "size"
> >> part being fully accurate there (and hence also here).
> >> ---
> >> I haven't been able to find any applicable documentation for other
> >> (perhaps legacy) assemblers' support of .noopt (and .optim). An option
> >> to also handle .optim would be to
> >> - restore previously saved settings (if optimization was enabled on the
> >>   command line),
> >> - enable a pre-determined level of optimization if optimization was not
> >>   enabled on the command line.
> >> Thoughts?
> >
> > Interesting. This is from
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b6f8c7c45229a8a5405079e586bfbaad396d2cbe
> > ("x86: Add -O[2|s] assembler command-line options").
>
> I'm confused by your reply: How is the above related (other than being,
> as stated, the reason for wanting to no longer ignore .noopt)?

Sorry, I did not realize that .optim and .noopt have been available
(bug ignored) for a long time (since 1992)...

% grep 'optim and' gas/ChangeLog-9295
        (md_pseudo_table): Ignore .optim and .noopt.

Thomas G. Szymanski, "Assembling Code for Machines with Span-Dependent
Instructions", April 1978, coined the term "span-dependent
instructions".
_AIX PS/2 and System/370 Programming Tools and Interfaces_ refers to
the term and defines .optim and .noopt

> These directives are used to turn off and ¦ ¦ .optim ¦ on span optimizations.

There is no more information. It's possible that AIX PS/2 and
System/370 only use .optim/.noopt for branches, a restricted form of
the general span-dependent expressions.
Hmm, I cannot find information about the directives in other assemblers.

Should gas invent a new directive, given that optimize_encoding does
other kinds of optimizations, unrelated to "span" (which is related to
an address per the paper "if that instruction's operand has an address
between m+a and m+b")

> > If push/pop are to be added, it seems clear if push/pop/setting use
> > the same directive like .noopt push, .noopt pop, .noopt optimize,
> > .noopt nooptimize, but the negative directive name now causes
> > confusion.
> > In addition, "opt" option means "option" (mips/riscv have .option).
> > Perhaps switch to a more meaningful directive name?
>
> Well, did you look at, in particular, ...
>
> >> @@ -1231,7 +1232,7 @@ const pseudo_typeS md_pseudo_table[] =
> >>    {"value", cons, 2},
> >>    {"slong", signed_cons, 4},
> >>    {"insn", s_insn, 0},
> >> -  {"noopt", s_ignore, 0},
> >> +  {"noopt", s_noopt, 0},
> >>    {"optim", s_ignore, 0},
> >>    {"code16gcc", set_16bit_gcc_code_flag, CODE_16BIT},
> >>    {"code16", set_code_flag, CODE_16BIT},
>
> ... this hunk? The respective "positive" directive is .optim. Yet
> implementing that without knowing its origin seemed less desirable to
> me. If we were entirely free, I certainly would agree with having
> push/pop forms.
>
> Jan

Re: [PATCH] x86: actually implement .noopt

[Jan Beulich]

On 08.01.2024 09:57, Fangrui Song wrote:
> On Sun, Jan 7, 2024 at 11:48 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 08.01.2024 02:18, Fangrui Song wrote:
>>> On Fri, Jan 5, 2024 at 12:31 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> For quite some time we've had support for -O command line options. With
>>>> that ignoring at least .noopt isn't really a good idea.
>>>>
>>>> Re-purpose the optimize-3 test for testing this directive's effect as
>>>> well.
>>>>
>>>> As to the doc addition - this uses the same text as is there for the
>>>> {nooptimize} pseudo-prefix, despite me not being convinced of the "size"
>>>> part being fully accurate there (and hence also here).
>>>> ---
>>>> I haven't been able to find any applicable documentation for other
>>>> (perhaps legacy) assemblers' support of .noopt (and .optim). An option
>>>> to also handle .optim would be to
>>>> - restore previously saved settings (if optimization was enabled on the
>>>>   command line),
>>>> - enable a pre-determined level of optimization if optimization was not
>>>>   enabled on the command line.
>>>> Thoughts?
>>>
>>> Interesting. This is from
>>> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b6f8c7c45229a8a5405079e586bfbaad396d2cbe
>>> ("x86: Add -O[2|s] assembler command-line options").
>>
>> I'm confused by your reply: How is the above related (other than being,
>> as stated, the reason for wanting to no longer ignore .noopt)?
> 
> Sorry, I did not realize that .optim and .noopt have been available
> (bug ignored) for a long time (since 1992)...
> 
> % grep 'optim and' gas/ChangeLog-9295
>         (md_pseudo_table): Ignore .optim and .noopt.
> 
> Thomas G. Szymanski, "Assembling Code for Machines with Span-Dependent
> Instructions", April 1978, coined the term "span-dependent
> instructions".
> _AIX PS/2 and System/370 Programming Tools and Interfaces_ refers to
> the term and defines .optim and .noopt
> 
>> These directives are used to turn off and ¦ ¦ .optim ¦ on span optimizations.
> 
> There is no more information. It's possible that AIX PS/2 and
> System/370 only use .optim/.noopt for branches, a restricted form of
> the general span-dependent expressions.
> Hmm, I cannot find information about the directives in other assemblers.
> 
> Should gas invent a new directive, given that optimize_encoding does
> other kinds of optimizations, unrelated to "span" (which is related to
> an address per the paper "if that instruction's operand has an address
> between m+a and m+b")

It is precisely this uncertainty which made me deal with .noopt only,
leaving .optim alone. Personally I consider ignoring directives quite
bad: Either we understand what the programmer wants, or we don't. As
a result an alternative to my (partial) proposal would be to simply
purge the two lines from i386's pseudo table. Specifically here,
ignoring a request to do optimization is possibly okay-ish, but
ignoring a request to suppress some/all optimization clearly isn't.

Jan