[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #30 from Tamar Christina  ---
Author: tnfchris
Date: Mon Feb 12 12:52:29 2018
New Revision: 257586

URL: https://gcc.gnu.org/viewcvs?rev=257586=gcc=rev
Log:
2018-02-12  Tamar Christina  

PR target/82641
* gcc.target/arm/pragma_arch_switch_2.c: Use armv5te.


Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/arm/pragma_arch_switch_2.c

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #29 from Tamar Christina  ---
Author: tnfchris
Date: Fri Feb  9 12:23:46 2018
New Revision: 257524

URL: https://gcc.gnu.org/viewcvs?rev=257524=gcc=rev
Log:
2018-02-09  Tamar Christina  

PR target/82641
* config/arm/arm-c.c (arm_cpu_builtins): Un-define __ARM_FEATURE_LDREX,
__ARM_ARCH_PROFILE, __ARM_ARCH_ISA_THUMB, __ARM_FP and __ARM_NEON_FP.

gcc/testsuite
2018-02-09  Tamar Christina  

PR target/82641
* gcc.target/arm/pragma_arch_switch_2.c: Use armv6 and armv5t.


Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm-c.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/arm/pragma_arch_switch_2.c

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #28 from Tamar Christina  ---
Author: tnfchris
Date: Tue Feb  6 11:20:55 2018
New Revision: 257410

URL: https://gcc.gnu.org/viewcvs?rev=257410=gcc=rev
Log:
2018-02-06  Tamar Christina  

PR target/82641
* config/arm/arm.c (arm_print_asm_arch_directives): Record already
emitted arch directives.
* config/arm/arm-c.c (arm_cpu_builtins): Undefine __ARM_ARCH and
__ARM_FEATURE_COPROC before changing architectures.

gcc/testsuite
2018-02-06  Tamar Christina  

PR target/82641
* gcc.target/arm/pragma_arch_switch_2.c: New.


Added:
trunk/gcc/testsuite/gcc.target/arm/pragma_arch_switch_2.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm-c.c
trunk/gcc/config/arm/arm.c
trunk/gcc/testsuite/ChangeLog

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #27 from Arnd Bergmann  ---
(In reply to Richard Earnshaw from comment #26)
> (In reply to Arnd Bergmann from comment #25)
> 
> > or to apply more force and add the ".arch" to each inline
> > asm individually.
> 
> No, that would not be guaranteed to be supported: and you'd be lying to the
> compiler again.  At the end of each asm block the compiler *could* emit new
> .arch directive to forcibly reset the architecture to what IT thinks it
> should be.

That's fine though: we won't have any invalid instructions outside of the
inline asm, the whole point of setting .arch to a higher arch revision is to
make the inline asm work and avoid the build error from the assembler.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #26 from Richard Earnshaw  ---
(In reply to Arnd Bergmann from comment #25)

> or to apply more force and add the ".arch" to each inline
> asm individually.

No, that would not be guaranteed to be supported: and you'd be lying to the
compiler again.  At the end of each asm block the compiler *could* emit new
.arch directive to forcibly reset the architecture to what IT thinks it should
be.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #25 from Arnd Bergmann  ---
(In reply to Tamar Christina from comment #24)
> Do you have a repro for this one? compiling the kernel with
> `CFLAGS="march=-armv4t"` doesn't seem to reproduce the original issue.

It needs to be a kernel configuration that enables both an ARMv4-based target
platform (e.g. ARCH_MOXART) and another target platform with ARMv5TE+IWMMXT
(e.g. ARCH_MMP).

> But the scenario should be working without needing to separate out the
> functions, as long as you're in-lining the right direction.

Ah, interesting.

> what would generate the error you're getting is if you're in-lining the
> armv5te code into armv4t which is an actual error
> 
> __attribute__((always_inline, target("arch=armv5te")))
> static inline int do_this (int x)
> {
>   return x*x;
> }
> 
> #pragma GCC target("arch=armv4t")   
> 
> 
> int do_that (int x, int y)
> {
>   return do_this (x - y);
> }
> 
> The compiler only rejects the inlining if you've told it to always inline
> and when the function to be inline's feature bits are not a strict subset of
> the function in which it is to inline

I can't reproduce it here myself now, no idea what I did earlier.

Anyway, since neither the #pragma nor the attribute work on existing
compilers, and making the hack version dependent would be worse,
I don't think we can use that anyway.

The best workaround I see so far is to either move all the affected
inline assembly statements into an external .S file to sidestep the
problem, or to apply more force and add the ".arch" to each inline
asm individually.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #24 from Tamar Christina  ---
Do you have a repro for this one? compiling the kernel with
`CFLAGS="march=-armv4t"` doesn't seem to reproduce the original issue.

But the scenario should be working without needing to separate out the
functions, as long as you're in-lining the right direction.

Making a handwritten example works fine:

__attribute__((always_inline, target("arch=armv4t")))
static inline int do_this (int x)
{
  return x*x;
}

#pragma GCC target("arch=armv5te")  

int do_that (int x, int y)
{
  return do_this (x - y);
}

what would generate the error you're getting is if you're in-lining the armv5te
code into armv4t which is an actual error

__attribute__((always_inline, target("arch=armv5te")))
static inline int do_this (int x)
{
  return x*x;
}

#pragma GCC target("arch=armv4t")   

int do_that (int x, int y)
{
  return do_this (x - y);
}

The compiler only rejects the inlining if you've told it to always inline and
when the function to be inline's feature bits are not a strict subset of the
function in which it is to inline

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #23 from Arnd Bergmann  ---
I've done some more testing with '#pragma GCC target("arch=armv5te")' in place,
but ran into another problem:

: note: this is the location of the previous definition
In file included from /git/arm-soc/include/linux/thread_info.h:38,
 from /git/arm-soc/include/asm-generic/current.h:5,
 from ./arch/arm/include/generated/asm/current.h:1,
 from /git/arm-soc/include/linux/sched.h:12,
 from /git/arm-soc/arch/arm/kernel/xscale-cp0.c:14:
/git/arm-soc/arch/arm/kernel/xscale-cp0.c: In function 'dsp_do':
/git/arm-soc/arch/arm/include/asm/thread_info.h:88:35: error: inlining failed
in call to always_inline 'current_thread_info': target specific option mismatch
 static inline struct thread_info *current_thread_info(void)
   ^~~
/git/arm-soc/arch/arm/kernel/xscale-cp0.c:48:18: note: called from here
   dsp_save_state(current_thread_info()->cpu_context.extra);
  ^

I've worked around that now by separating the parts that use inline assembly
into standalone functions with GCC push_options/pop_options around them, so
they are not mixed with normal code that might call an inline function, but
this is getting increasingly ugly.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #22 from Tamar Christina  ---
ACLE macros nor pragma.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #21 from Tamar Christina  ---
I'll make the compiler not emit the warning so it shouldn't fail anymore.
To answer your question about the pragma

> and presumably would lead to the while file being built for armv5te,
> possibly generating instructions that may be invalid for armv4 or armv4t 
> outside of the inline assembly that is known to be safe.

It's only a warning that the ACLE pragmas are being re-defined. Semantically it
should behave the same way as the original in-line assembly, with the one
exception that the compiler will only emit it if it emits any code. So it
should have compiled the file already (you can check the assembly output, it
only emits the .arch and nothing else in this case).

Additionally it would also validate the new extension against the other
options, so it should be somewhat safer.

It doesn't change it again until you either do a POP or use another pragma.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[yyc1992 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #20 from Yichao Yu  ---
Just want to mention that the lack of a way to locally change the arch settings
without lying to the compiler is exactly why I reported this issue.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #19 from Arnd Bergmann  ---
(In reply to Richard Earnshaw from comment #18)
> 
> So you're changing the targetted architecture behind the compilers back.  Ie
> you're lying to it.  Frankly, you deserve to get burnt if you do things like
> that.

Du you have a suggestion on what to do instead?

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #18 from Richard Earnshaw  ---
(In reply to Arnd Bergmann from comment #14)
> It looks like r255468 broke compilation of a couple of files in the Linux
> kernel,
> which use a top-level statement like
> 
> linux/arch/arm/kvm/hyp/banked-sr.c:
> __asm__(".arch_extension virt");
> 
> linux/arch/arm/kernel/xscale-cp0.c
> asm("   .arch armv5te\n");
> 

So you're changing the targetted architecture behind the compilers back.  Ie
you're lying to it.  Frankly, you deserve to get burnt if you do things like
that.

Patient: Doctor, if I bang my head against a wall, it hurts.
Doctor: Don't do that then!.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #17 from Tamar Christina  ---
Ah!, thanks for that repro.

I see what's changed. I'll work on a fix.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #16 from Arnd Bergmann  ---
Here is a simplified version of the file in question, to try as standalone:

typedef unsigned int u32;
asm(".arch armv5te\n");
extern int cpuid;
static _Bool cpu_is_xscale_family()
{
/* this code must be compiled to execute on other CPUs, so
   we cannot just use -march=armv5te */
switch (cpuid & 0xe000) {
case 0x69052000: /* Intel XScale 1 */
case 0x69054000: /* Intel XScale 2 */
case 0x69056000: /* Intel XScale 3 */
case 0x56056000: /* Marvell XScale 3 */
case 0x56158000: /* Marvell Mohawk */
return 1;
}
return 0;
}
static int cpu_has_iwmmxt(void)
{
u32 lo;
u32 hi;

/*
 * This sequence is interpreted by the DSP coprocessor as:
 *  mar acc0, %2, %3
 *  mra %0, %1, acc0
 *
 * And by the iWMMXt coprocessor as:
 *  tmcrr   wR0, %2, %3
 *  tmrrc   %0, %1, wR0
 */
__asm__ __volatile__ (
"mcrr   p0, 0, %2, %3, c0\n"
"mrrc   p0, 0, %0, %1, c0\n"
: "=r" (lo), "=r" (hi)
: "r" (0), "r" (0x100));

return !!hi;
}
int xscale_cp0_init(void)
{
/* do not attempt to probe iwmmxt on non-xscale family CPUs */
if (!cpu_is_xscale_family())
return 0;

if (!cpu_has_iwmmxt())
return 0;

/* ... start using iwmmxt */

return 0;
}

$ arm-linux-gnueabi-gcc-7.2.1 -Wall -O2 -c test.c -march=armv4t
# no output

$ arm-linux-gnueabi-gcc-8.0.1 -Wall -O2 -c test.c -march=armv4t
/tmp/ccobFwz5.s: Assembler messages:
/tmp/ccobFwz5.s:34: Error: selected processor does not support `mcrr
p0,0,r0,r3,c0' in ARM mode
/tmp/ccobFwz5.s:35: Error: selected processor does not support `mrrc
p0,0,r3,r2,c0' in ARM mode

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #15 from Tamar Christina  ---
Hi Arnd,

What's the original error you're seeing without using the pragma?

That should have worked fine still.

I wasn't able to reproduce the failure using the default flags to the compiler,
are you by chance specifying anything else to the compiler?

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[arnd at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

Arnd Bergmann  changed:

   What|Removed |Added

 CC||arnd at linaro dot org

--- Comment #14 from Arnd Bergmann  ---
It looks like r255468 broke compilation of a couple of files in the Linux
kernel,
which use a top-level statement like

linux/arch/arm/kvm/hyp/banked-sr.c:
__asm__(".arch_extension virt");

linux/arch/arm/kernel/xscale-cp0.c
asm("   .arch armv5te\n");

to allow compilation for a target other than the one specified by with -march=
to the compiler.

I tried using

#if GCC_VERSION >= 80
#pragma GCC target("arch=armv5te")
#else
asm(".arch armv5te\n");
#endif

but that results in a build failure:
/git/arm-soc/arch/arm/kernel/xscale-cp0.c:21: warning: "__ARM_ARCH" redefined
: note: this is the location of the previous definition
/git/arm-soc/arch/arm/kernel/xscale-cp0.c:21: warning: "__ARM_FEATURE_COPROC"
redefined

and presumably would lead to the while file being built for armv5te, possibly
generating instructions that may be invalid for armv4 or armv4t outside of the
inline assembly that is known to be safe.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #13 from Tamar Christina  ---
Author: tnfchris
Date: Tue Jan  9 11:04:50 2018
New Revision: 256375

URL: https://gcc.gnu.org/viewcvs?rev=256375=gcc=rev
Log:
2018-01-09  Tamar Christina    

PR target/82641
* gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use
no NEON and require softfp or hard float-abi.
* gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.


Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
trunk/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

Tamar Christina  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #12 from Tamar Christina  ---
That should do it.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #11 from Tamar Christina  ---
Author: tnfchris
Date: Thu Dec  7 14:54:22 2017
New Revision: 255468

URL: https://gcc.gnu.org/viewcvs?rev=255468=gcc=rev
Log:
2017-12-07  Tamar Christina  

PR target/82641
* config/arm/arm.c (INCLUDE_STRING): Define.
(arm_last_printed_arch_string, arm_last_printed_fpu_string): New.
(arm_declare_function_name): Conservatively emit .arch,
.arch_extensions
and .fpu.

gcc/testsuite/
2017-12-07  Tamar Christina  

PR target/82641
* gcc.target/arm/pragma_arch_attribute_2.c: New.
* gcc.target/arm/pragma_arch_attribute_2.c: New.
* gcc.target/arm/pragma_arch_attribute_3.c: New.
* gcc.target/arm/pragma_fpu_attribute.c: New.
* gcc.target/arm/pragma_fpu_attribute_2.c: New.


Added:
trunk/gcc/testsuite/gcc.target/arm/pragma_arch_attribute_2.c
trunk/gcc/testsuite/gcc.target/arm/pragma_arch_attribute_3.c
trunk/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
trunk/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm.c
trunk/gcc/testsuite/ChangeLog

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

Richard Earnshaw  changed:

   What|Removed |Added

   Target Milestone|--- |8.0
   Severity|normal  |enhancement

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #10 from Tamar Christina  ---
Author: tnfchris
Date: Fri Nov 17 15:53:51 2017
New Revision: 254878

URL: https://gcc.gnu.org/viewcvs?rev=254878=gcc=rev
Log:
2017-11-17  Tamar Christina  

PR target/82641
* config/arm/arm.c (arm_valid_target_attribute_rec):
Parse "arch=" and "+".
(arm_valid_target_attribute_tree): Re-init global options.
(arm_option_override): Make non-static.
(arm_options_perform_arch_sanity_checks): Make errors fatal.
* gcc/config/arm/arm-c.c (__ARM_FEATURE_CMSE): Support undef.
(__ARM_FEATURE_CRC32): Support undef.
* config/arm/arm_acle.h (__ARM_FEATURE_CRC32): Replace with pragma.
* doc/extend.texi (ARM Function Attributes): Add pragma and target.

gcc/testsuite/
2017-11-17  Tamar Christina  

PR target/82641
* gcc.target/arm/pragma_arch_attribute.c: New.


Added:
trunk/gcc/testsuite/gcc.target/arm/pragma_arch_attribute.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm.c
trunk/gcc/config/arm/arm_acle.h
trunk/gcc/doc/extend.texi
trunk/gcc/testsuite/ChangeLog

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #9 from Tamar Christina  ---
Author: tnfchris
Date: Fri Nov 10 17:14:28 2017
New Revision: 254632

URL: https://gcc.gnu.org/viewcvs?rev=254632=gcc=rev
Log:
2017-11-10  Tamar Christina  

PR target/82641
* config/arm/arm.c
(arm_option_override): Refactor.
(arm_option_reconfigure_globals): New.
(arm_options_perform_arch_sanity_checks): New.
* config/arm/arm-protos.h (arm_option_reconfigure_globals): New
prototype.
(arm_options_perform_arch_sanity_checks): Likewise


Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm-protos.h
trunk/gcc/config/arm/arm.c

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #8 from Tamar Christina  ---
> It would be great if `+crc` can work if it's not ambiguous. Requiring 
> `arch=armv8-a+crc` works for me too, and it'll just require more preprocessor 
> checks.

Yes I'm adding `+crc` and `arch=...`.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[yyc1992 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #7 from Yichao Yu  ---
It would be great if `+crc` can work if it's not ambiguous. Requiring
`arch=armv8-a+crc` works for me too, and it'll just require more preprocessor
checks.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #6 from Richard Earnshaw  ---
(In reply to Tamar Christina from comment #5)
> My patch adds support for 
> 
> 
> ```
> #pragma GCC push_options
> #pragma GCC target("arch=armv8-a+crc")
> __attribute__((target("arch=armv8-a+crc"))) uint32_t crc32cw(uint32_t crc,
> uint32_t val)
> ```
> 
> and just
> 
> 
> ```
> #pragma GCC push_options
> #pragma GCC target("+crc")
> __attribute__((target("+crc"))) uint32_t crc32cw(uint32_t crc, uint32_t val)
> ```
> 
> if the compiler was invoked with `-march=armv8-a`.
> 
> However it currently doesn't allow
> 
> 
> ```
> #pragma GCC push_options
> #pragma GCC target("armv8-a+crc")
> __attribute__((target("armv8-a+crc"))) uint32_t crc32cw(uint32_t crc,
> uint32_t val)
> ```
> 
> as it doesn't know if you wanted to change the `fpu` or `arch` here.
> Should I support this case as well?


No, I think we should insist on an arch= or fpu= prefix for all but the adding
extensions case.

Note that extensions can only modify the existing architecture if it supports
that extension.  This is because the modifiers are context dependent: for
example the +fp extension (new in gcc-8) will enable floating-point
instructions that come with the specified architecture.

Also note that in the long-term -mfpu and pragma/attribute variants thereof are
likely to be deprecated in favour of specifying all this through the
architecture string.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

Tamar Christina  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |tnfchris at gcc dot 
gnu.org

--- Comment #5 from Tamar Christina  ---
My patch adds support for 


```
#pragma GCC push_options
#pragma GCC target("arch=armv8-a+crc")
__attribute__((target("arch=armv8-a+crc"))) uint32_t crc32cw(uint32_t crc,
uint32_t val)
```

and just


```
#pragma GCC push_options
#pragma GCC target("+crc")
__attribute__((target("+crc"))) uint32_t crc32cw(uint32_t crc, uint32_t val)
```

if the compiler was invoked with `-march=armv8-a`.

However it currently doesn't allow


```
#pragma GCC push_options
#pragma GCC target("armv8-a+crc")
__attribute__((target("armv8-a+crc"))) uint32_t crc32cw(uint32_t crc, uint32_t
val)
```

as it doesn't know if you wanted to change the `fpu` or `arch` here.
Should I support this case as well?

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

Ramana Radhakrishnan  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-10-31
 CC||ramana at gcc dot gnu.org,
   ||tnfchris at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #4 from Ramana Radhakrishnan  ---
(In reply to Yichao Yu from comment #3)
> > ARMv8-a is the only architecture variant where the CRC extension is optional
> 
> Not really. There's also armv8-r and armv8-m. Also, I believe code compiled

armv8-m does not have the CRC extension though armv8-r does have CRC as
optional IIRC.

Tamar isn't this something you've been looking at recently ?

Anyway confirmed with the testcase.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[yyc1992 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #3 from Yichao Yu  ---
> ARMv8-a is the only architecture variant where the CRC extension is optional

Not really. There's also armv8-r and armv8-m. Also, I believe code compiled for
armv7-a can run on armv8-a hardware and can also optionally enable armv8
features including CRC extension. I was hoping that GCC can be smart enough to
enable the correct armv8 variant automatically.

Test case is just

```
#include 

#pragma GCC push_options
#pragma GCC target("armv8-a+crc")
__attribute__((target("armv8-a+crc"))) uint32_t crc32cw(uint32_t crc, uint32_t
val)
{
uint32_t res;
/* asm(".arch armv8-a"); */
/* asm(".arch_extension crc"); */
asm("crc32cw %0, %1, %2" : "=r"(res) : "r"(crc), "r"(val));
/* asm(".arch armv7-a"); */
return res;
}
#pragma GCC pop_options
```

Compiled with either armv7-a or armv8-a march.

[Bug target/82641] Unable to enable crc32 for a certain function with target attribute on ARM (aarch32)

[rearnsha at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82641

--- Comment #2 from Richard Earnshaw  ---
ARMv8-a is the only architecture variant where the CRC extension is optional. 
In later variants it is enabled by default; in earlier versions of the
architecture it doesn't exist.

Your report lacks a testcase we can use to examine this more fully.