Re: [PATCH][AArch64] Only update assembler .arch directive when necessary

[Andrew Pinski <pinskia at gmail dot com>]

On Thu, Feb 4, 2016 at 5:50 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi all,
>
> As part of the target attributes and pragmas support for GCC 6 I changed the
> aarch64 port
> to emit a .arch assembly directive for each function that describes the
> architectural features
> used by that function.  This is a change from GCC 5 behaviour where we
> output a single .arch directive
> at the beginning of the assembly file corresponding to architectural
> features given on the command line.
>
> We need that change to accommodate use cases where a user tags a single
> function with a target attribute
> that changes the arch features from those given on the command line and then
> proceeds to use an inline
> assembly instruction that needs those new features.  We need to emit the new
> architecture state for that
> function as a .arch directive to prevent the assembler from rejecting the
> assembly.
>
> Unfortunately, the new behaviour has caused some headaches when building the
> Linux kernel.
> There, they have a header called lse.h that is used to access some
> armv8.1-a-specific inline assembly
> (using the "lse" architecture feature in particular)
> This header has the line:
> __asm__(".arch_extension        lse");
> to tell the assembler to accept LSE instructions.
> This header is included in various .c files so the .arch_extension appears
> at the beginning of the
> assembly file.
>
> But since we now emit a .arch directive for every function, we effectively
> reset the architectural state
> on each function, rendering that .arch_extension ineffective.
>
> GCC is arguably right in emitting the .arch directive on each function, but
> I'd like to make life easier
> for the kernel folk so this patch helps with that.

I had suggested a change to the Linux kernel already to fix this issue:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-December/395191.html

Thanks,
Andrew Pinski

>
> With this patch we go back to emitting the .arch directive at the top of the
> assembly file, same in GCC 5.
> We will still emit per-function .arch directives but only if they differ
> from the last .arch directive
> that we emitted i.e. when the architecture features were changed due to a
> target attribute or pragma.
> For code that doesn't make use of target attributes or pragmas the behaviour
> of GCC 5 is thus preserved
> and we still get the correct .arch updates when needed.
>
> The TARGET_ASM_FILE_START hook implementation is brought back after I
> deleted it during the target
> attributes work last August and is used again to output the .arch directive
> at the top of the file.
> We keep track of the architecture and the architecture features we last
> output in the asm directive
> in a new static variable (yuck, but unavoidable IMO) and output the new
> .arch directive only if
> what we want to output is different from the previously output directive.
>
> This still allows the kernel to do naughty things with .arch_extension
> directives in header files,
> but this usecase is one where they really know what they're doing and don't
> want the assembler
> to get in their way.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
> With this patch I managed to build a recent allyesconfig Linux kernel where
> before the build would fail
> when assembling the LSE instructions.
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2016-02-04  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/aarch64/aarch64.c (struct aarch64_output_asm_info):
>     New struct definition.
>     (aarch64_previous_asm_output): New variable.
>     (aarch64_declare_function_name): Only output .arch assembler
>     directive if it will be different from the previously output
>     directive.
>     (aarch64_start_file): New function.
>     (TARGET_ASM_FILE_START): Define.
>
> 2016-02-04  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * gcc.target/aarch64/assembler_arch_1.c: Add -dA to dg-options.
>     Delete unneeded -save-temps.
>     * gcc.target/aarch64/assembler_arch_7.c: Likewise.
>     * gcc.target/aarch64/target_attr_15.c: Scan assembly for
>     .arch armv8-a\n.
>     * gcc.target/aarch64/assembler_arch_1.c: New test.