This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Thu, Feb 11, 2016 at 01:10:33PM +0000, Kyrill Tkachov wrote:
> >>>Why not just keep the last string you printed, and use a string compare
> >>>to decide whether to print or not? Sure we'll end up doing a bit more
> >>>work, but the logic becomes simpler to follow and we don't need to pass
> >>>around another struct...
> >>I did do it this way to avoid a string comparison (I try to avoid
> >>manual string manipulations where I can as they're so easy to get wrong)
> >>though this isn't on any hot path.
> >>We don't really pass the structure around anywhere, we just keep one
> >>instance. We'd have to do the same with a string i.e. keep a string
> >>object around that we'd strcpy (or C++ equivalent) a string to every time
> >>we wanted to update it, so I thought this approach is cleaner as the
> >>architecture features are already fully described by a pointer to
> >>an element in the static constant all_architectures table and an
> >>unsigned long holding the ISA flags.
> >>
> >>If you insist I can change it to a string, but I personally don't
> >>think it's worth it.
> >Had you been working on a C string I probably wouldn't have noticed. But
> >you're already working with C++ strings in this function, so much of what
> >you are concerned about is straightforward.
> >
> >I'd encourage you to try it using idiomatic string manipulation in C++, the
> >cleanup should be worth it.
> 
> Ok, here it is using std::string.
> It is a shorter patch.

Looks good to me, OK for trunk.

Thanks,
James

> 2016-02-10  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/aarch64/aarch64.c (aarch64_last_printed_arch_string):
>     New variable.
>     (aarch64_last_printed_tune_string): Likewise.
>     (aarch64_declare_function_name): Only output .arch assembler
>     directive if it will be different from the previously output
>     directive.  Same for .tune comment but only if -dA is set.
>     (aarch64_start_file): New function.
>     (TARGET_ASM_FILE_START): Define.
> 
> 2016-02-10  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * gcc.target/aarch64/target_attr_15.c: Scan assembly for
>     .arch armv8-a\n.  Add -dA to dg-options.
>     * gcc.target/aarch64/assembler_arch_1.c: New test.
>     * gcc.target/aarch64/target_attr_7.c: Add -dA to dg-options.
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]