[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml
SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added inline comments.
Comment at: test/Driver/aarch64-cpus.c:518
+// RUN: %clang -target aarch64 -march=armv8.4-a -### -c %s 2>&1 | FileCheck
-check-prefix=GENERICV84A-NO-FP16FML %s
+// GENERICV84A-NO-FP16FML-NOT: "-target-feature" "{{[+-]}}fp16fml"
+// GENERICV84A-NO-FP16FML-NOT: "-target-feature" "{{[+-]}}fullfp16"
fpetrogalli wrote:
> Hi @SjoerdMeijer , I have noticed that this test does something different
> from what gcc does (well, claims to do, I haven't checked the actual behavior
> on gcc).
>
> From the table in [1], it seems that `armv8.4-a` implies `fp16fml`... who got
> it right? GCC or clang? Or am I missing something?
>
> Francesco
>
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html#AArch64-Options
> (see the description of `-march=name`)
I think the ARMARM is pretty clear on this. Copied from one of the instruction
descriptions enabled by fp16ml:
"In Armv8.2 and Armv8.3, this is an OPTIONAL instruction. From Armv8.4 it is
mandatory for all implementations to support it."
So, looks like this needs fixing.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D50229/new/
https://reviews.llvm.org/D50229
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml
fpetrogalli added inline comments.
Herald added a subscriber: danielkiss.
Comment at: test/Driver/aarch64-cpus.c:518
+// RUN: %clang -target aarch64 -march=armv8.4-a -### -c %s 2>&1 | FileCheck
-check-prefix=GENERICV84A-NO-FP16FML %s
+// GENERICV84A-NO-FP16FML-NOT: "-target-feature" "{{[+-]}}fp16fml"
+// GENERICV84A-NO-FP16FML-NOT: "-target-feature" "{{[+-]}}fullfp16"
Hi @SjoerdMeijer , I have noticed that this test does something different from
what gcc does (well, claims to do, I haven't checked the actual behavior on
gcc).
From the table in [1], it seems that `armv8.4-a` implies `fp16fml`... who got
it right? GCC or clang? Or am I missing something?
Francesco
[1] https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html#AArch64-Options
(see the description of `-march=name`)
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D50229/new/
https://reviews.llvm.org/D50229
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342862: [ARM][AArch64] Add feature +fp16fml (authored by
SjoerdMeijer, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50229
Files:
lib/Driver/ToolChains/Arch/AArch64.cpp
lib/Driver/ToolChains/Arch/ARM.cpp
test/Driver/aarch64-cpus.c
test/Driver/arm-cortex-cpus.c
test/Preprocessor/aarch64-target-features.c
test/Preprocessor/arm-target-features.c
Index: lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- lib/Driver/ToolChains/Arch/AArch64.cpp
+++ lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -193,6 +193,32 @@
Features.push_back("-crc");
}
+ // Handle (arch-dependent) fp16fml/fullfp16 relationship.
+ // FIXME: this fp16fml option handling will be reimplemented after the
+ // TargetParser rewrite.
+ const auto ItRNoFullFP16 = std::find(Features.rbegin(), Features.rend(), "-fullfp16");
+ const auto ItRFP16FML = std::find(Features.rbegin(), Features.rend(), "+fp16fml");
+ if (std::find(Features.begin(), Features.end(), "+v8.4a") != Features.end()) {
+const auto ItRFullFP16 = std::find(Features.rbegin(), Features.rend(), "+fullfp16");
+if (ItRFullFP16 < ItRNoFullFP16 && ItRFullFP16 < ItRFP16FML) {
+ // Only entangled feature that can be to the right of this +fullfp16 is -fp16fml.
+ // Only append the +fp16fml if there is no -fp16fml after the +fullfp16.
+ if (std::find(Features.rbegin(), ItRFullFP16, "-fp16fml") == ItRFullFP16)
+Features.push_back("+fp16fml");
+}
+else
+ goto fp16_fml_fallthrough;
+ }
+ else {
+fp16_fml_fallthrough:
+// In both of these cases, putting the 'other' feature on the end of the vector will
+// result in the same effect as placing it immediately after the current feature.
+if (ItRNoFullFP16 < ItRFP16FML)
+ Features.push_back("-fp16fml");
+else if (ItRNoFullFP16 > ItRFP16FML)
+ Features.push_back("+fullfp16");
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))
if (A->getOption().matches(options::OPT_mno_unaligned_access))
Index: lib/Driver/ToolChains/Arch/ARM.cpp
===
--- lib/Driver/ToolChains/Arch/ARM.cpp
+++ lib/Driver/ToolChains/Arch/ARM.cpp
@@ -391,6 +391,33 @@
} else if (HDivArg)
getARMHWDivFeatures(D, HDivArg, Args, HDivArg->getValue(), Features);
+ // Handle (arch-dependent) fp16fml/fullfp16 relationship.
+ // Must happen before any features are disabled due to soft-float.
+ // FIXME: this fp16fml option handling will be reimplemented after the
+ // TargetParser rewrite.
+ const auto ItRNoFullFP16 = std::find(Features.rbegin(), Features.rend(), "-fullfp16");
+ const auto ItRFP16FML = std::find(Features.rbegin(), Features.rend(), "+fp16fml");
+ if (Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v8_4a) {
+const auto ItRFullFP16 = std::find(Features.rbegin(), Features.rend(), "+fullfp16");
+if (ItRFullFP16 < ItRNoFullFP16 && ItRFullFP16 < ItRFP16FML) {
+ // Only entangled feature that can be to the right of this +fullfp16 is -fp16fml.
+ // Only append the +fp16fml if there is no -fp16fml after the +fullfp16.
+ if (std::find(Features.rbegin(), ItRFullFP16, "-fp16fml") == ItRFullFP16)
+Features.push_back("+fp16fml");
+}
+else
+ goto fp16_fml_fallthrough;
+ }
+ else {
+fp16_fml_fallthrough:
+// In both of these cases, putting the 'other' feature on the end of the vector will
+// result in the same effect as placing it immediately after the current feature.
+if (ItRNoFullFP16 < ItRFP16FML)
+ Features.push_back("-fp16fml");
+else if (ItRNoFullFP16 > ItRFP16FML)
+ Features.push_back("+fullfp16");
+ }
+
// Setting -msoft-float/-mfloat-abi=soft effectively disables the FPU (GCC
// ignores the -mfpu options in this case).
// Note that the ABI can also be set implicitly by the target selected.
@@ -404,7 +431,7 @@
//now just be explicit and disable all known dependent features
//as well.
for (std::string Feature : {"vfp2", "vfp3", "vfp4", "fp-armv8", "fullfp16",
-"neon", "crypto", "dotprod"})
+"neon", "crypto", "dotprod", "fp16fml"})
if (std::find(std::begin(Features), std::end(Features), "+" + Feature) != std::end(Features))
Features.push_back(Args.MakeArgString("-" + Feature));
}
Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -93,18 +93,45 @@
// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
// C
[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D50229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml
SjoerdMeijer updated this revision to Diff 166439. SjoerdMeijer retitled this revision from "+fp16fml feature for ARM and AArch64" to "[ARM][AArch64] Add feature +fp16fml". SjoerdMeijer edited the summary of this revision. SjoerdMeijer added a comment. Added FIXMEs. https://reviews.llvm.org/D50229 Files: lib/Driver/ToolChains/Arch/AArch64.cpp lib/Driver/ToolChains/Arch/ARM.cpp test/Driver/aarch64-cpus.c test/Driver/arm-cortex-cpus.c test/Preprocessor/aarch64-target-features.c test/Preprocessor/arm-target-features.c Index: test/Preprocessor/arm-target-features.c === --- test/Preprocessor/arm-target-features.c +++ test/Preprocessor/arm-target-features.c @@ -21,18 +21,58 @@ // CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_FORMAT_IEEE 1 // CHECK-V8A-ALLOW-FP-INSTR-V8A-NOT: #define __ARM_FEATURE_DOTPROD -// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xe // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1 -// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s +// +fp16fml without neon doesn't make sense as the fp16fml instructions all require SIMD. +// However, as +fp16fml implies +fp16 there is a set of defines that we would expect. +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16fml -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16fml -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s // CHECK-FULLFP16-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1 // CHECK-FULLFP16-SCALAR-NOT: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1 // CHECK-FULLFP16-SCALAR: #define __ARM_FP 0xe // CHECK-FULLFP16-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1 -// + +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML-VECTOR-SCALAR %s +// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+fp16fml+nofp16 -x c -E -dM %s -o -
