SjoerdMeijer added a comment.
Ping
https://reviews.llvm.org/D33719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
SjoerdMeijer added a comment.
I've created revision https://reviews.llvm.org/D35295 for the documentation
update.
https://reviews.llvm.org/D33719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer created this revision.
This documents the differences and interactions between _Float16 and __fp16.
https://reviews.llvm.org/D35295
Files:
docs/LanguageExtensions.rst
Index: docs/LanguageExtensions.rst
===
---
SjoerdMeijer updated this revision to Diff 106167.
SjoerdMeijer added a comment.
Thanks for review! Feedback addressed.
https://reviews.llvm.org/D35295
Files:
docs/LanguageExtensions.rst
Index: docs/LanguageExtensions.rst
===
SjoerdMeijer added a comment.
Thanks Richard. I've opened an cxx abi issue, see comments inline. I will start
working now on the doc update, and will do that in a companion change. Cheers.
Comment at: lib/AST/ItaniumMangle.cpp:2457-2460
+ case BuiltinType::Float16:
case
SjoerdMeijer added a comment.
Good points. I am thinking about how to write this down.
I am not yet sure that `_Float16` can reduce portability. I think the behaviour
will depend on FLT_EVAL_METHOD. I.e., if your architecture supports
half-precision instructions, you would like to set
SjoerdMeijer updated this revision to Diff 106228.
SjoerdMeijer added a comment.
This fixes:
- type mangling for `_Float16` (approved here:
https://github.com/itanium-cxx-abi/cxx-abi/pull/22, but not yet committed. )
- removed the argument promotion for `_Float16` that I added because it turns
SjoerdMeijer added a comment.
Ping.
(CXX ABI change committed, doc patch created)
https://reviews.llvm.org/D33719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306805: ARMV8-A archkind and target defines helper functions
(authored by SjoerdMeijer).
Changed prior to commit:
https://reviews.llvm.org/D34686?vs=104393=104830#toc
Repository:
rL LLVM
SjoerdMeijer created this revision.
Herald added subscribers: kristof.beyls, aemerson.
This is a clean-up for different ARMV8-A architecture kinds. Helper function
hasFP16VectorArithmetic makes things a bit more “scalable” if we want to add
ARMv8.3 at some point.
SjoerdMeijer updated this revision to Diff 104380.
SjoerdMeijer added a comment.
Thanks! I am now using llvm::AArch64::ArchKind. And I agree that the check for
setting __ARM_FEATURE_QRDMX is suspicious. I will address this separately.
https://reviews.llvm.org/D34686
Files:
SjoerdMeijer updated this revision to Diff 104393.
SjoerdMeijer retitled this revision from "[AArch64] Add hasFP16VectorArithmetic
helper function. NFCI" to "[AArch64] ARMV8-A archkind and target defines
helper functions".
SjoerdMeijer edited the summary of this revision.
SjoerdMeijer added a
SjoerdMeijer added a comment.
Ping
https://reviews.llvm.org/D33719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
SjoerdMeijer added a comment.
"They also implement the RCpc AArch64 extension from ARMv8.3-A."
Perhaps you need to explain why a v8.2 core implements a v8.3 extension?
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:92
+ std::vector ) {
+
SjoerdMeijer added a comment.
Ah, my message crossed with Renato's; I only noticed it after submitting mine
(looks like we agree though:-))
https://reviews.llvm.org/D36731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Looks good to me too.
Two nits (no new review required): one is inlined, and the other one in the
summary: ARMv8.2-A => Armv8.2-A :-/
Comment at:
SjoerdMeijer updated this revision to Diff 103201.
SjoerdMeijer edited the summary of this revision.
SjoerdMeijer added a comment.
I have added a fix for mixed __fp16 and _Float16 expressions: _Float16 type is
converted to __fp16 type and then the operation is completed as if both
operands were
SjoerdMeijer updated this revision to Diff 103397.
SjoerdMeijer added a comment.
This fixes the “DefaultVariadicArgumentPromotion” for Float16: they should be
promoted to double, which makes now e.g. printf work.
I have added printf tests to both the AST and codegen test to check variadic
SjoerdMeijer added a comment.
I have create a separate patch for the _Float16 preprocessor macro definitions
in https://reviews.llvm.org/D34695.
https://reviews.llvm.org/D33719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer created this revision.
Herald added subscribers: aheejin, dschuff, jfb.
This adds the _Float16 preprocessor macro definitions.
https://reviews.llvm.org/D34695
Files:
lib/Frontend/InitPreprocessor.cpp
lib/Headers/float.h
test/Preprocessor/init.c
Index:
SjoerdMeijer added a comment.
Hi Bruno, Akira,
Many thanks for your feedback! Apologies for the missing context. The patch
touches many files and thus with context it is quite big (~4MB). Thought this
would be too much if we need a few iterations. Anyway, will include it from now
on.
I am
SjoerdMeijer created this revision.
Herald added a subscriber: klimek.
This adds _Float16 as a source language type. As a first step, _Float16 behaves
the same as __fp16 and is thus an alias. This means that _Float16 also behaves
like a storage-only type. Subsequent patches will implement the
SjoerdMeijer updated this revision to Diff 102649.
SjoerdMeijer added a comment.
Thanks Roger. I did the clean up; there were indeed still a few fixmes there.
The good thing is that it's a self-contained clang patch again: we don't need
https://reviews.llvm.org/D34205, which I have abandoned.
SjoerdMeijer updated this revision to Diff 102543.
SjoerdMeijer added a comment.
Float16 is added as a native type. Implementing it as some sort of alias to
fp16 caused too many type issues in expression/literals/etc., and thus was not
an easier first step, and also in the end we want it to be
SjoerdMeijer added inline comments.
Comment at: include/clang-c/Index.h:3015
CXType_Half = 31,
+ CXType_Float16 = 30,
CXType_FirstBuiltin = CXType_Void,
rogfer01 wrote:
> This enumerator is the same as `CXType_Float128` above, is that intended?
Ah,
SjoerdMeijer updated this revision to Diff 100866.
SjoerdMeijer added a comment.
Fixed typos 'TST_float16' and 'CXType_Float16 = 30', and have also added it to
a switch that I had missed.
https://reviews.llvm.org/D33719
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313152: This adds the _Float16 preprocessor macro
definitions. (authored by SjoerdMeijer).
Changed prior to commit:
https://reviews.llvm.org/D34695?vs=114998=115050#toc
Repository:
rL LLVM
SjoerdMeijer added a comment.
many thanks for reviewing and your help.
https://reviews.llvm.org/D34695
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
SjoerdMeijer added a comment.
Many thanks for reviewing and your help!
https://reviews.llvm.org/D33719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312781: Add _Float16 as a C/C++ source language type
(authored by SjoerdMeijer).
Changed prior to commit:
https://reviews.llvm.org/D33719?vs=113218=114325#toc
Repository:
rL LLVM
SjoerdMeijer updated this revision to Diff 113218.
SjoerdMeijer added a comment.
Comments addressed. Thanks for reviewing.
https://reviews.llvm.org/D33719
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/Specifiers.h
SjoerdMeijer updated this revision to Diff 114998.
SjoerdMeijer added a comment.
Fixed the typos, and added tests.
https://reviews.llvm.org/D34695
Files:
lib/Frontend/InitPreprocessor.cpp
lib/Headers/float.h
test/Headers/float16.c
test/Preprocessor/init.c
Index:
SjoerdMeijer added a comment.
I am going to commit this within a few days. That looks reasonable to me given
that the comments in the last reviews were very minor (which I have of course
addressed already). Also, in case of issues, I am guessing fixes and/or
addition can be easily done
SjoerdMeijer added inline comments.
Comment at: lib/Headers/float.h:137
+#ifdef __STDC_WANT_IEC_60559_TYPES_EXT__
+# define FLT16_MANT_DIG __FLT16_MANT_DIG__
scanon wrote:
> rogfer01 wrote:
> > scanon wrote:
> > > rogfer01 wrote:
> > > > My understanding is
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Thanks for fixing this.
https://reviews.llvm.org/D37106
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Looks reasonable to me.
Repository:
rL LLVM
https://reviews.llvm.org/D3
___
cfe-commits mailing list
SjoerdMeijer updated this revision to Diff 113100.
SjoerdMeijer added a comment.
No changes were needed to make the conversions work, the existing logic is
taking care of that, but I agree it doesn't hurt to add a few test cases. So
I've added tests to both files, and cleaned up that comment.
SjoerdMeijer updated this revision to Diff 121265.
SjoerdMeijer added a comment.
Many thanks for the reviews and suggestions! Comments addressed.
https://reviews.llvm.org/D35295
Files:
docs/LanguageExtensions.rst
Index: docs/LanguageExtensions.rst
This revision was automatically updated to reflect the committed changes.
Closed by commit rL320019: [ARM] ACLE parallel arithmetic and DSP style
multiplications (authored by SjoerdMeijer).
Changed prior to commit:
https://reviews.llvm.org/D40888?vs=125705=125909#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rC320019: [ARM] ACLE parallel arithmetic and DSP style
multiplications (authored by SjoerdMeijer).
Repository:
rC Clang
https://reviews.llvm.org/D40888
Files:
include/clang/Basic/BuiltinsARM.def
SjoerdMeijer created this revision.
Herald added subscribers: kristof.beyls, javed.absar, aemerson.
This is a follow up of r302131, in which we forgot to add SemaChecking
tests. Adding these tests revealed two problems which have been fixed:
- added missing intrinsic __qdbl,
- properly
SjoerdMeijer added inline comments.
Comment at: include/clang/Basic/BuiltinsARM.def:39
BUILTIN(__builtin_arm_qsub, "iii", "nc")
+BUILTIN(__builtin_arm_qdbl, "ii", "nc")
BUILTIN(__builtin_arm_ssat, "iiUi", "nc")
samparker wrote:
> Do we now need a codegen tests
SjoerdMeijer updated this revision to Diff 121003.
SjoerdMeijer added a comment.
Addressed the comments about portability, and added a note that Float16 is
available in both C and C++ mode.
https://reviews.llvm.org/D35295
Files:
docs/LanguageExtensions.rst
Index:
SjoerdMeijer added a comment.
Just out of curiousity:
- How do you plan to implement this? Are you going to generate from the pragma
some sort of "script" that dictates the transformation order which is going to
be fed to the pass manager?
- About the stacking of pragmas, in your example you
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Agreed: supported architectures are v7/A32/A64.
https://reviews.llvm.org/D47446
___
cfe-commits mailing list
SjoerdMeijer added a comment.
Nits title:
- Think you can simplify the title to something along the lines of: "[AArch64]
Support the FP16 VCVTA_U16 intrinsic". No need to mention tests are added in
the subject (tests should always be added).
Nits summary:
- Arm v8.2a -> Armv8.2-A
- Aarch64
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Thanks. Looks like a straightforward fix to me.
Comment at: CodeGen/arm-v8.2a-neon-intrinsics.c:170
+// CHECK: ret <4 x i16> [[VCVT]]
+int16x4_t
SjoerdMeijer added a comment.
I know very little about SPIR, and Initially didn't understand this:
> The SPIR target currently allows for half precision floating point types to
> use the LLVM intrinsic functions to convert to floats and doubles. This is
> illegal in SPIR as the only intrinsic
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Looks OK to me.
Comment at: test/CodeGen/spir-half-type.cpp:89
+
+_Float16 fadd() {
+ _Float16 a = 1.0f16;
Nit: let one of these functions take
SjoerdMeijer added a comment.
Had only a first brief look; see some first drive by comments inline.
Comment at: lib/CodeGen/CGBuiltin.cpp:7865
}
// FIXME: Sharing loads & stores with 32-bit is complicated by the absence
// of an Align parameter here.
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
I agree: these intrinsics are available in v7/A32/A64.
Comment at: lib/CodeGen/CGBuiltin.cpp:7865
}
// FIXME: Sharing loads & stores with 32-bit is
SjoerdMeijer added inline comments.
Comment at: test/Sema/aarch64-neon-fp16-ranges.c:1
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon
-fallow-half-arguments-and-returns -target-feature +fullfp16 -ffreestanding
-fsyntax-only -verify %s
+// RUN: %clang_cc1
SjoerdMeijer added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:1409
- switch (BuiltinID) {
-#define GET_NEON_OVERLOAD_CHECK
-#include "clang/Basic/arm_neon.inc"
Why do we need to remove this?
Comment at:
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
I think this looks ok now, just some nits inline.
Can you please upload your diffs with more context next time?
Comment at: utils/TableGen/NeonEmitter.cpp:2166
SjoerdMeijer added a comment.
No problem, will commit this today.
https://reviews.llvm.org/D48188
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Looks OK to me.
But file "test/CodeGen/vld_dup.c" looks weird/empty. I guess you're removing it?
https://reviews.llvm.org/D48440
___
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Looks like a straight forward fix/addition to me.
Comment at: lib/Basic/Targets/ARM.cpp:737
+ if (DotProd)
+Builder.defineMacro("__ARM_FEATURE_DOTPROD",
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
I think this looks OK.
Comment at: include/clang/Basic/arm_neon.td:1587
+// v8.2-A dot product instructions
+let ArchGuard = "defined(__ARM_FEATURE_DOTPROD)" in
SjoerdMeijer added inline comments.
Comment at: clang/include/clang/Basic/arm_fp16.td:58
+class IInst : Inst {}
+
+// ARMv8.2-A FP16 intrinsics.
az wrote:
> SjoerdMeijer wrote:
> > There's a little bit of duplication here: the definitions above
SjoerdMeijer added a comment.
Thanks for working on this!
Some comments inline.
Comment at: clang/include/clang/Basic/arm_fp16.td:19
+// The operations are subclasses of Operation providing a list of DAGs, the
+// last of which is the return value.
+//
nit:
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: olista01, t.p.northover, rjmccall, aschwaighofer.
Herald added subscribers: kristof.beyls, javed.absar, aemerson.
Pass and return _Float16 as if it were an int or float for ARM, but with the
top 16 bits unspecified, similarly like
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323185: [ARM] Pass _Float16 as int or float (authored by
SjoerdMeijer, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
SjoerdMeijer updated this revision to Diff 131009.
SjoerdMeijer added a comment.
Moved the tests to the existing file (and fixed a few typos in the tests).
https://reviews.llvm.org/D42318
Files:
include/clang/AST/Type.h
lib/CodeGen/TargetInfo.cpp
test/CodeGen/arm-fp16-arguments.c
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D41792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer added a comment.
Thanks for fixing this. Looks very reasonable to me.
Question about the failures: I am now wondering if this means we were and still
are missing tests?
Nit: for future reviews, I think it is better to split patches up if they are
commits to
different repos.
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Thanks
https://reviews.llvm.org/D42993
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer closed this revision.
SjoerdMeijer added a comment.
Herald added a subscriber: hintonda.
Committed as r323005
https://reviews.llvm.org/D41792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: samparker, olista01, rengolin.
Herald added subscribers: kristof.beyls, javed.absar.
This adds Sema and Codegen tests for the vcvtr builtins
(because they were missing).
https://reviews.llvm.org/D43372
Files:
SjoerdMeijer added a comment.
Thanks for reviewing!
https://reviews.llvm.org/D43372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325351: [ARM] Add tests for the vcvtr builtins (authored by
SjoerdMeijer, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325351: [ARM] Add tests for the vcvtr builtins (authored by
SjoerdMeijer, committed by ).
Repository:
rL LLVM
https://reviews.llvm.org/D43372
Files:
test/CodeGen/builtins-arm.c
SjoerdMeijer added a comment.
Hi Eli, thanks for the feedback.
> Yes, this logic should be in TargetParser, not here. Trying to rewrite the
> target features afterwards is messy at best. (Actually, the target feature
> list generated by TargetParser probably shouldn't contain the string
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: samparker, olista01, john.brawn, ab,
t.p.northover.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.
This adds tests for Armv8.4-A, and also some v8.2 and v8.3 tests that were
missed in
SjoerdMeijer added inline comments.
Comment at: test/Driver/aarch64-cpus.c:9
// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s
2>&1 | FileCheck -check-prefix=GENERIC %s
// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: olista01, ab.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.
In https://reviews.llvm.org/D50068, @ab noticed that it would be better to
match aarch64-{{.*}} for
tests that use "-target aarch64_be
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338525: [AArch64][ARM] Add Armv8.4-A tests (authored by
SjoerdMeijer, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50068
Files:
lib/Basic/Targets/ARM.cpp
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: olista01, samparker, john.brawn, ab,
t.p.northover.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.
For AArch64:
1. Crypto means sm4 + sha3 + sha2 + aes for Armv8.4-A and up,
2. and sha2 +
SjoerdMeijer added inline comments.
Comment at: test/Driver/aarch64-cpus.c:10
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic"
+// GENERIC-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic"
olista01 wrote:
>
SjoerdMeijer updated this revision to Diff 159915.
SjoerdMeijer added a comment.
Addressed comments.
https://reviews.llvm.org/D50175
Files:
test/Driver/aarch64-cpus.c
Index: test/Driver/aarch64-cpus.c
===
---
SjoerdMeijer added inline comments.
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:430
+ if (ArchName.find_lower("+noaes") == StringRef::npos)
+Features.push_back("+aes");
+} else if (ArchName.find_lower("-crypto") != StringRef::npos) {
efriedma
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339347: [AArch64][NFC] better matching of AArch64 target in
aarch64-cpus.c tests (authored by SjoerdMeijer, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
SjoerdMeijer added inline comments.
Comment at: test/Driver/aarch64-cpus.c:10
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic"
+// GENERIC-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic"
olista01 wrote:
> Why do
SjoerdMeijer updated this revision to Diff 159715.
SjoerdMeijer added a comment.
Addressed comments.
https://reviews.llvm.org/D50175
Files:
test/Driver/aarch64-cpus.c
Index: test/Driver/aarch64-cpus.c
===
---
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM.
https://reviews.llvm.org/D49075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer updated this revision to Diff 159979.
SjoerdMeijer added a comment.
fixed typo
https://reviews.llvm.org/D50179
Files:
lib/Driver/ToolChains/Arch/AArch64.cpp
lib/Driver/ToolChains/Arch/ARM.cpp
test/Driver/arm-features.c
test/Preprocessor/aarch64-target-features.c
Index:
SjoerdMeijer updated this revision to Diff 159991.
https://reviews.llvm.org/D50179
Files:
lib/Driver/ToolChains/Arch/AArch64.cpp
lib/Driver/ToolChains/Arch/ARM.cpp
test/Driver/arm-features.c
test/Preprocessor/aarch64-target-features.c
Index: test/Preprocessor/aarch64-target-features.c
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
Looks reasonable to me.
https://reviews.llvm.org/D51093
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer added a comment.
Now that they are conditionally defined, do we need negative tests (in
test/Sema/arm-no-fp16.c?) to check that they are not available when fp16 is not
enabled?
https://reviews.llvm.org/D49075
___
cfe-commits mailing
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: include/clang/Basic/arm_neon.td:1419
// Vector rounding
- def FRINTZH : SInst<"vrnd", "dd", "hQh">;
- def FRINTNH :
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D48829
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: include/clang/Basic/arm_neon.td:1466
def VMINH : SInst<"vmin", "ddd", "hQh">;
- def FMAXNMH : SInst<"vmaxnm", "ddd", "hQh">;
-
SjoerdMeijer added inline comments.
Comment at: include/clang/Basic/arm_neon.td:1419
// Vector rounding
- def FRINTZH : SInst<"vrnd", "dd", "hQh">;
- def FRINTNH : SInst<"vrndn", "dd", "hQh">;
- def FRINTAH : SInst<"vrnda", "dd", "hQh">;
- def FRINTPH
SjoerdMeijer added inline comments.
Comment at: include/clang/Basic/arm_neon.td:1466
def VMINH : SInst<"vmin", "ddd", "hQh">;
- def FMAXNMH : SInst<"vmaxnm", "ddd", "hQh">;
- def FMINNMH : SInst<"vminnm", "ddd", "hQh">;
+ let ArchGuard = "__ARM_ARCH >=
SjoerdMeijer added a comment.
Thanks for reviewing!
We are trying to achieve correct AAPCS parameter passing:
"If the argument is a Half-precision Floating Point Type its size is set to 4
bytes as if it
had been copied to the least significant bits of a 32-bit register and the
remaining
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: az, evandro, olista01.
Herald added subscribers: kristof.beyls, javed.absar, rengolin.
Add 2 vmulxh_lane vector intrinsics that were commented out.
https://reviews.llvm.org/D44222
Files:
include/clang/Basic/arm_neon.td
SjoerdMeijer added inline comments.
Comment at: include/clang/Basic/arm_neon.td:1504
+ // Scalar floating point multiply extended (scalar, by element)
+ def SCALAR_FMULX_LANEH : IOpInst<"vmulx_lane", "ssdi", "Sh",
OP_SCALAR_MUL_LN>;
+ def SCALAR_FMULX_LANEQH :
SjoerdMeijer added inline comments.
Comment at: include/clang/Basic/arm_neon.td:1504
+ // Scalar floating point multiply extended (scalar, by element)
+ def SCALAR_FMULX_LANEH : IOpInst<"vmulx_lane", "ssdi", "Sh",
OP_SCALAR_MUL_LN>;
+ def SCALAR_FMULX_LANEQH :
SjoerdMeijer added a comment.
Hi @mstorsjo, thanks for reporting this!
I was waiting for @az, and only had a quick look myself, but I don't think it's
going to be
a quick fix. So that would suggest indeed that a revert is a best. Perhaps we
can wait a few
more hours to give the guys in the US
SjoerdMeijer added a comment.
FYI: I have partially recommitted this in r327455; I have separated out the
minimal functional change related to the FP16 macros.
https://reviews.llvm.org/D43650
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: samparker, olista01, evandro, az.
Herald added subscribers: kristof.beyls, javed.absar.
This adds some missing tests for the AArch64 FP16 macros.
https://reviews.llvm.org/D44512
Files:
1 - 100 of 398 matches
Mail list logo