[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

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 t

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

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 === --- test/Driver/aarch64

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

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: > SjoerdMe

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

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 === --- test/Driver/aarch64

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

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: https://revi

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

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

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

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: tes

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

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 ==

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

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 ap

[PATCH] D51093: [ARM] Set __ARM_FEATURE_SIMD32 for +dsp cores

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 http://lists.llvm.org

[PATCH] D51429: [AArch64] Return Address Signing B Key Support

SjoerdMeijer added inline comments. Comment at: test/CodeGen/aarch64-sign-return-address.c:3 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=non-leaf %s | FileCheck %s --check-prefix=CHECK-PARTIAL // RUN: %clang -target aarch64-arm-none-ea

[PATCH] D49376: [NEON] Define half-precision vrnd intrinsics only when available

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 : SInst<"vrndn"

[PATCH] D49375: [NEON] Define half-precision vmaxnm intrinsics only when available

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">; -

[PATCH] D49075: [NEON] Define fp16 vld and vst intrinsics conditionally

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 l

[PATCH] D48829: [NEON] Fix support for vrndi_f32(), vrndiq_f32() and vrndns_f32() intrinsics

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 http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D49375: [NEON] Define half-precision vmaxnm intrinsics only when available

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 >= 8

[PATCH] D49376: [NEON] Define half-precision vrnd intrinsics only when available

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

[PATCH] D50068: [AArch64][ARM] Add Armv8.4-A tests

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 previou

[PATCH] D50068: [AArch64][ARM] Add Armv8.4-A tests

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" --

[PATCH] D50068: [AArch64][ARM] Add Armv8.4-A tests

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 test/Driver/aarch64-cpus

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

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 -m

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

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 + aes

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

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 "crypto

[PATCH] D49075: [NEON] Define fp16 vld and vst intrinsics conditionally

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 http://lists.llvm.org/cgi-bin/m

[PATCH] D42993: [AArch64] Fixes for ARMv8.2-A FP16 scalar intrinsic

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. htt

[PATCH] D41792: [AArch64] Add ARMv8.2-A FP16 scalar intrinsics

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 http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D42993: [AArch64] Fixes for ARMv8.2-A FP16 scalar intrinsic

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 http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D43372: [ARM] Add tests for the vcvtr builtins

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: test/CodeGen/built

[PATCH] D43372: [ARM] Add tests for the vcvtr builtins

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

[PATCH] D43372: [ARM] Add tests for the vcvtr builtins

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 test/Sema/builtins-

[PATCH] D43372: [ARM] Add tests for the vcvtr builtins

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: https://reviews.llvm.org/D43372?vs=134570&id=1

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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 pr

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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, thanks,

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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 includ

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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 wor

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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 a

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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.

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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 functi

[PATCH] D41792: [AArch64] Add ARMv8.2-A FP16 scalar intrinsics

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: t

[PATCH] D41792: [AArch64] Add ARMv8.2-A FP16 scalar intrinsics

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 are the > > same

[PATCH] D43650: [ARM] Add ARMv8.2-A FP16 vector intrinsics

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Look like sensible cleanups/fixes/additions to me. We were struggling whether to pass an i16 or f16 type, which can both be illegal types. Therefore, it perhaps doesn't really matt

[PATCH] D44222: [AArch64] Add vmulxh_lane FP16 intrinsics

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 test/C

[PATCH] D44222: [AArch64] Add vmulxh_lane FP16 intrinsics

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 : IOpInst<"vmulx_

[PATCH] D44222: [AArch64] Add vmulxh_lane FP16 intrinsics

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 : IOpInst<"vmulx_

[PATCH] D43650: [ARM] Add ARMv8.2-A FP16 vector intrinsics

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 ti

[PATCH] D43650: [ARM] Add ARMv8.2-A FP16 vector intrinsics

SjoerdMeijer added a comment. Reverted in r327437. https://reviews.llvm.org/D43650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43650: [ARM] Add ARMv8.2-A FP16 vector intrinsics

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

[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

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: test/Preprocessor/aarch64-target-features.

[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

SjoerdMeijer added a comment. Thanks for reviewing! https://reviews.llvm.org/D44512 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

This revision was automatically updated to reflect the committed changes. Closed by commit rL327623: [AAch64] Tests for ACLE FP16 macros (authored by SjoerdMeijer, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D44512?vs=138521&id=1385

[PATCH] D44561: [ARM] Add HasFloat16 to TargetInfo

SjoerdMeijer created this revision. SjoerdMeijer added reviewers: t.p.northover, samparker, olista01. Herald added subscribers: kristof.beyls, javed.absar. For generating NEON intrinsics, this determines the NEON data type, and whether it should be a half type or an i16 type. I.e., we always pass

[PATCH] D44561: [ARM] Add HasFloat16 to TargetInfo

SjoerdMeijer added a comment. Thanks for the review. Please see a first comment inline. Comment at: include/clang/Basic/TargetInfo.h:365 + /// \brief Determine whether _Float16 is supported on this target. + virtual bool hasFloat16Type() const { return HasFloat16; } ---

[PATCH] D44561: [ARM] Add HasFloat16 to TargetInfo

SjoerdMeijer updated this revision to Diff 138722. SjoerdMeijer added a comment. Addressed comments: simplified the logic in GetNeonType. https://reviews.llvm.org/D44561 Files: include/clang/Basic/TargetInfo.h lib/Basic/TargetInfo.cpp lib/Basic/Targets/AArch64.cpp lib/Basic/Targets/ARM.

[PATCH] D44561: [ARM] Pass half or i16 types for NEON intrinsics

SjoerdMeijer updated this revision to Diff 138890. SjoerdMeijer retitled this revision from "[ARM] Add HasFloat16 to TargetInfo" to "[ARM] Pass half or i16 types for NEON intrinsics". SjoerdMeijer added a comment. Herald added a subscriber: rengolin. Removed unused function argument, and renamed

[PATCH] D44561: [ARM] Pass half or i16 types for NEON intrinsics

SjoerdMeijer added a comment. Thanks a lot for your help and reviews. https://reviews.llvm.org/D44561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44561: [ARM] Pass half or i16 types for NEON intrinsics

This revision was automatically updated to reflect the committed changes. Closed by commit rL327836: [ARM] Pass half or i16 types for NEON intrinsics (authored by SjoerdMeijer, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D44561?vs=1

[PATCH] D44222: [AArch64] Add vmulxh_lane FP16 intrinsics

SjoerdMeijer abandoned this revision. SjoerdMeijer added a comment. This is implemented in https://reviews.llvm.org/D44591. https://reviews.llvm.org/D44222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D44591: [AArch64] Add vmulxh_lane FP16 vector intrinsic

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. This looks good to me, but we need a companion LLVM patch and add codegen tests for this to: CodeGen/AArch64/fp16_intrinsic_lane.ll. https://reviews.llvm.org/D44591 ___

[PATCH] D47121: [NEON] Support VLD1xN intrinsics in AArch32 mode (Clang part)

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. --

[PATCH] D47121: [NEON] Support VLD1xN intrinsics in AArch32 mode (Clang part)

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 compli

[PATCH] D47592: [AArch64] Corrected FP16 Intrinsic range checks in Clang + added Sema tests

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 -tri

[PATCH] D47592: [AArch64] Corrected FP16 Intrinsic range checks in Clang + added Sema tests

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: lib/Sema/SemaChecking.cpp:14

[PATCH] D47592: [AArch64] Corrected FP16 Intrinsic range checks in Clang + added Sema tests

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 +

[PATCH] D47446: [NEON] Support VST1xN intrinsics in AArch32 mode (Clang part)

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 cfe-commits@lists.llvm.o

[PATCH] D48119: [AArch64] Added Clang Codegen+Test Support for FP16 VCVTA_U16 intrinsic

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 ->

[PATCH] D48119: [AArch64] Added support for the vcvta_u16_f16 instrinsic for FP16 Armv8.2-A

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 test_vcvta_u16_f

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

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 a

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

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

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

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

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

This revision was automatically updated to reflect the committed changes. Closed by commit rC335111: [SPIR] Prevent SPIR targets from using half conversion intrinsics (authored by SjoerdMeijer, committed by ). Changed prior to commit: https://reviews.llvm.org/D48188?vs=151938&id=152045#toc Rep

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

SjoerdMeijer added inline comments. Comment at: include/clang/Basic/TargetInfo.h:66 bool HasFloat128; + bool HasFloat16; unsigned char PointerWidth, PointerAlign; I think this is the same as `HasLegalHalfType`, and we can (re)use that. Or, at least, don'

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

SjoerdMeijer added a comment. FYI: a new ACLE version has been published, please find it here: https://developer.arm.com/architectures/system-architectures/software-standards/acle The "Neon Intrinsics" section contains these new intrinsics. Repository: rL LLVM CHANGES SINCE LAST ACTION

[PATCH] D60699: [ARM] add CLI support for 8.1-M and MVE.

SjoerdMeijer added inline comments. Comment at: clang/test/Driver/armv8.1m.main.c:1 +// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+dsp -### %s 2> %t +// RUN: FileCheck --check-prefix=CHECK-DSP < %t %s It doesn't really matter, I guess, but we d

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks okay to me. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61717/new/ https://reviews.llvm.org/D61717 _

[PATCH] D50229: +fp16fml feature for ARM and AArch64

SjoerdMeijer added a comment. (I am now picking this up, and will try to progress this patch and also https://reviews.llvm.org/D50179) > Do you expect that the regression tests will be affected by the TargetParser > fixes? No, and that's exactly the reason why it would be nice to get this in.

[PATCH] D50229: +fp16fml feature for ARM and AArch64

SjoerdMeijer added a comment. Ah, and just for your info, the proposal was just sent to the dev list: http://lists.llvm.org/pipermail/llvm-dev/2018-September/126346.html Repository: rC Clang https://reviews.llvm.org/D50229 ___ cfe-commits mailin

[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/D50

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

SjoerdMeijer updated this revision to Diff 166643. SjoerdMeijer added a comment. Added FIXMEs, like in https://reviews.llvm.org/D50229, that this needs reimplementation too after the TargerParser rewrite. About v8.5, the ISA description is now available here: https://developer.arm.com/products/

[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

[PATCH] D52491: [ARM/AArch64][v8.5A] Add Armv8.5-A target

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks okay to me Comment at: test/Driver/arm-cortex-cpus.c:338 + +// RUN: %clang -target armv8a-linux-eabi -march=armv8.5-a+fp16 -### -c %s 2>&1 | FileCheck --ch

[PATCH] D52492: [AArch64][v8.5A] Test optional Armv8.5-A random number extension

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D52492 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D52493: [AArch64][v8.5A] Test clang option for the Memory Tagging Extension

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D52493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

SjoerdMeijer added a comment. @efriedma : apologies for the ping, but does this look reasonable? https://reviews.llvm.org/D50179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

SjoerdMeijer added a comment. Thanks! https://reviews.llvm.org/D50179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

This revision was automatically updated to reflect the committed changes. Closed by commit rL343758: [AArch64][ARM] Context sensitive meaning of crypto (authored by SjoerdMeijer, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50179?vs

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks okay to me, with one nit inline. Comment at: test/Preprocessor/init.c:9169 // WEBASSEMBLY-NEXT:#define __FLOAT128__ 1 -// WEBASSEMBLY-NEXT:#define __FLT16_

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

SjoerdMeijer added inline comments. Comment at: cfe/trunk/test/CodeGen/aarch64-neon-fp16fml.c:12 + +float32x2_t test_vfmlal_low_u32(float32x2_t a, float16x4_t b, float16x4_t c) { +// CHECK-LABEL: define <2 x float> @test_vfmlal_low_u32(<2 x float> %a, <4 x half> %b, <4 x half> %

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

SjoerdMeijer added inline comments. Comment at: cfe/trunk/test/CodeGen/aarch64-neon-fp16fml.c:12 + +float32x2_t test_vfmlal_low_u32(float32x2_t a, float16x4_t b, float16x4_t c) { +// CHECK-LABEL: define <2 x float> @test_vfmlal_low_u32(<2 x float> %a, <4 x half> %b, <4 x half> %

[PATCH] D58306: [AArch64] Change size suffix for FP16FML intrinsics.

SjoerdMeijer added a comment. I am discussing this with our GCC team as we would like both Clang/GCC implementation to be the same. But you're right that _f16 looks like to be the more consistent choice. I will let you know as soon I know more. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D58306: [AArch64] Change size suffix for FP16FML intrinsics.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM The ACLE has been updated and a new version with change included will be released soon. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58306/n

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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.

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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 post-com

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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&id=114325#toc Repository: rL LLVM https://reviews.

[PATCH] D34695: _Float16 preprocessor macro definitions

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 t

[PATCH] D34695: _Float16 preprocessor macro definitions

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: test/Preprocessor/init.

[PATCH] D34695: _Float16 preprocessor macro definitions

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

[PATCH] D34695: _Float16 preprocessor macro definitions

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&id=115050#toc Repository: rL LLVM https:

  1   2   3   4   >