r210625 - [AArch64] Add default features for CPUs on AArch64 target.

[Eric Christopher]

Hi Kevin,

On Thu, Jun 12, 2014 at 6:57 PM, Kevin Qin <kevinqindev at gmail.com> wrote:
> Hi Eric,
>
> As far as I know, -march is useless for AArch64 as there is only one arch
> available at moment. Also, features are bound to CPUs, not architecture. For
> AArch64, features like neon, crypto and crc are all optional.
>

You seem to have a fundamental misunderstanding of how the -march
option works for every other target than the legacy arm32 and m68k
targets. The latter because no one bothered to change it, the former
because it's legacy and there was actually an entrenched amount of
users. The aarch64 target doesn't have the weird .cpu directive that
arm had making it make even less sense to have a -mcpu option that
changes target architecture.

See, for example:

https://gcc.gnu.org/onlinedocs/gcc-4.8.1/gcc/i386-and-x86_002d64-Options.html
https://gcc.gnu.org/onlinedocs/gcc-4.8.1/gcc/MIPS-Options.html
https://gcc.gnu.org/onlinedocs/gcc-4.8.1/gcc/HPPA-Options.html#HPPA-Options

I've already fought against this a few times and I still insist that
it doesn't make sense to copy a set of command line options that don't
make any sense. See, for example, how the mips port does this as a
guideline for when you have multiple ISAs and multiple cpu
architectures based on them - with optional features. Keep in mind
that this method for new targets in gcc has been around for even
longer than llvm has existed.

-eric

>
> Regards,
> Kevin Qin
>
>
> 2014-06-13 2:19 GMT+08:00 Eric Christopher <echristo at gmail.com>:
>
>> Depressing.
>>
>> What's wrong with using -march?
>>
>> -eric
>>
>> On Tue, Jun 10, 2014 at 6:42 PM, Kevin Qin <Kevin.Qin at arm.com> wrote:
>> > Author: kevinqin
>> > Date: Tue Jun 10 20:42:16 2014
>> > New Revision: 210625
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=210625&view=rev
>> > Log:
>> > [AArch64] Add default features for CPUs on AArch64 target.
>> >
>> > For ARM target, we can use CRYPTO and CRC features if we select
>> > cortex-a57 by '-mcpu', but for AArch64 target, it doesn't work
>> > unless adding with '-mfpu=crypto-neon-fp-armv8'. To keep consistency
>> > between front-end and back-end and get end-users more easier to use,
>> > we'd better add default feature for CPUs on AArch64 target as well.
>> >
>> > Modified:
>> >     cfe/trunk/lib/Basic/Targets.cpp
>> >     cfe/trunk/test/Preprocessor/aarch64-target-features.c
>> >
>> > Modified: cfe/trunk/lib/Basic/Targets.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=210625&r1=210624&r2=210625&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Basic/Targets.cpp (original)
>> > +++ cfe/trunk/lib/Basic/Targets.cpp Tue Jun 10 20:42:16 2014
>> > @@ -4243,6 +4243,7 @@ class AArch64TargetInfo : public TargetI
>> >      NeonMode
>> >    };
>> >
>> > +  std::string CPU;
>> >    unsigned FPU;
>> >    unsigned CRC;
>> >    unsigned Crypto;
>> > @@ -4302,6 +4303,8 @@ public:
>> >                          .Cases("cortex-a53", "cortex-a57", true)
>> >                          .Case("cyclone", true)
>> >                          .Default(false);
>> > +    if (CPUKnown)
>> > +      CPU = Name;
>> >      return CPUKnown;
>> >    }
>> >
>> > @@ -4373,6 +4376,23 @@ public:
>> >        (Feature == "neon" && FPU == NeonMode);
>> >    }
>> >
>> > +  void getDefaultFeatures(llvm::StringMap<bool> &Features) const
>> > override {
>> > +
>> > +  if (CPU == "cyclone") {
>> > +    Features["fp-armv8"] = true;
>> > +    Features["neon"] = true;
>> > +    Features["crypto"] = true;
>> > +    Features["crc"] = true;
>> > +    Features["zcm"] = true;
>> > +    Features["zcz"] = true;
>> > +  } else if (CPU == "cortex-a53" || CPU == "cortex-a57") {
>> > +    Features["fp-armv8"] = true;
>> > +    Features["neon"] = true;
>> > +    Features["crypto"] = true;
>> > +    Features["crc"] = true;
>> > +  }
>> > +}
>> > +
>> >    bool handleTargetFeatures(std::vector<std::string> &Features,
>> >                              DiagnosticsEngine &Diags) override {
>> >      FPU = FPUMode;
>> >
>> > Modified: cfe/trunk/test/Preprocessor/aarch64-target-features.c
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/aarch64-target-features.c?rev=210625&r1=210624&r2=210625&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/test/Preprocessor/aarch64-target-features.c (original)
>> > +++ cfe/trunk/test/Preprocessor/aarch64-target-features.c Tue Jun 10
>> > 20:42:16 2014
>> > @@ -49,3 +49,13 @@
>> >  // RUN: %clang -target arm64-none-linux-gnu -mfpu=neon -x c -E -dM %s
>> > -o - | FileCheck --check-prefix=CHECK-NEON %s
>> >  // CHECK-NEON: __ARM_NEON 1
>> >  // CHECK-NEON: __ARM_NEON_FP 0xe
>> > +
>> > +// RUN: %clang -target aarch64-none-linux-gnu -mcpu=cortex-a53 -x c -E
>> > -dM %s -o - | FileCheck --check-prefix=CHECK-FEATURE %s
>> > +// RUN: %clang -target aarch64-none-linux-gnu -mcpu=cortex-a57 -x c -E
>> > -dM %s -o - | FileCheck --check-prefix=CHECK-FEATURE %s
>> > +// RUN: %clang -target aarch64-none-linux-gnu -mcpu=cyclone -x c -E -dM
>> > %s -o - | FileCheck --check-prefix=CHECK-FEATURE %s
>> > +// CHECK-FEATURE: __ARM_FEATURE_CRC32 1
>> > +// CHECK-FEATURE: __ARM_FEATURE_CRYPTO 1
>> > +// CHECK-FEATURE: __ARM_NEON 1
>> > +// CHECK-FEATURE: __ARM_NEON_FP 0xe
>> > +
>> > +
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
>
> --
> Best Regards,
>
> Kevin Qin