PING^2

On 8/10/21 17:52, Martin Liška wrote:
PING^1

On 7/1/21 3:13 PM, Martin Liška wrote:
On 10/23/20 1:47 PM, Martin Liška wrote:
Hey.

Hello.

I deferred the patch for GCC 12. Since the time, I messed up with options
I feel more familiar with the option handling. So ...


This is a follow-up of the discussion that happened in thread about 
no_stack_protector
attribute: https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545916.html

The current optimize attribute works in the following way:
- 1) we take current global_options as base
- 2) maybe_default_options is called for the currently selected optimization 
level, which
      means all rules in default_options_table are executed
- 3) attribute values are applied (via decode_options)

So the step 2) is problematic: in case of -O2 -fno-omit-frame-pointer and 
__attribute__((optimize("-fno-stack-protector")))
ends basically with -O2 -fno-stack-protector because -fno-omit-frame-pointer is 
default:
     /* -O1 and -Og optimizations.  */
     { OPT_LEVELS_1_PLUS, OPT_fomit_frame_pointer, NULL, 1 },

My patch handled and the current optimize attribute really behaves that same as 
appending attribute value
to the command line. So far so good. We should also reflect that in 
documentation entry which is quite
vague right now:

^^^ all these are still valid arguments, plus I'm adding a new test-case that 
tests that.


"""
The optimize attribute is used to specify that a function is to be compiled 
with different optimization options than specified on the command line.
"""

I addressed that with documentation changes, should be more clear to users. 
Moreover, I noticed that we declare 'optimize' attribute
as something not for a production use:

"The optimize attribute should be used for debugging purposes only. It is not 
suitable in production code."

Are we sure about the statement? I know that e.g. glibc uses that.


and we may want to handle -Ox in the attribute in a special way. I guess many 
macro/pragma users expect that

-O2 -ftree-vectorize and __attribute__((optimize(1))) will end with -O1 and not
with -ftree-vectorize -O1 ?

The situation with 'target' attribute is different. When parsing the attribute, 
we intentionally drop all existing target flags:

$ cat -n gcc/config/i386/i386-options.c
...
   1245                if (opt == IX86_FUNCTION_SPECIFIC_ARCH)
   1246                  {
   1247                    /* If arch= is set,  clear all bits in 
x_ix86_isa_flags,
   1248                       except for ISA_64BIT, ABI_64, ABI_X32, and CODE16
   1249                       and all bits in x_ix86_isa_flags2.  */
   1250                    opts->x_ix86_isa_flags &= (OPTION_MASK_ISA_64BIT
   1251                                               | OPTION_MASK_ABI_64
   1252                                               | OPTION_MASK_ABI_X32
   1253                                               | OPTION_MASK_CODE16);
   1254                    opts->x_ix86_isa_flags_explicit &= 
(OPTION_MASK_ISA_64BIT
   1255                                                        | 
OPTION_MASK_ABI_64
   1256                                                        | 
OPTION_MASK_ABI_X32
   1257                                                        | 
OPTION_MASK_CODE16);
   1258                    opts->x_ix86_isa_flags2 = 0;
   1259                    opts->x_ix86_isa_flags2_explicit = 0;
   1260                  }

That seems logical because target attribute is used for e.g. ifunc 
multi-versioning and one needs
to be sure all existing ISA flags are dropped. However, I noticed clang behaves 
differently:

$ cat hreset.c
#pragma GCC target "arch=geode"
#include <immintrin.h>
void foo(unsigned int eax)
{
   _hreset (eax);
}

$ clang hreset.c -mhreset  -c -O2 -m32
$ gcc hreset.c -mhreset  -c -O2 -m32
In file included from 
/home/marxin/bin/gcc/lib64/gcc/x86_64-pc-linux-gnu/12.0.0/include/x86gprintrin.h:97,
                  from 
/home/marxin/bin/gcc/lib64/gcc/x86_64-pc-linux-gnu/12.0.0/include/immintrin.h:27,
                  from hreset.c:2:
hreset.c: In function ‘foo’:
/home/marxin/bin/gcc/lib64/gcc/x86_64-pc-linux-gnu/12.0.0/include/hresetintrin.h:39:1:
 error: inlining failed in call to ‘always_inline’ ‘_hreset’: target specific 
option mismatch
    39 | _hreset (unsigned int __EAX)
       | ^~~~~~~
hreset.c:5:3: note: called from here
     5 |   _hreset (eax);
       |   ^~~~~~~~~~~~~

Anyway, I think the current target attribute handling should be preserved.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin


I'm also planning to take a look at the target macro/attribute, I expect 
similar problems:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97469

Thoughts?
Thanks,
Martin



Reply via email to