[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1 +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s + -

[PATCH] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

SjoerdMeijer added a comment. Did a first scan, looks very reasonable, just some first nits/questions inlined. Comment at: clang/include/clang/Basic/arm_sve.td:153 +} +def ImmCheckPredicatePattern: ImmCheckType<0>; // 0..31 +def ImmCheck1_16: ImmCheckType<1

[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76062/new/ https://reviews.llvm.org/D76062 ___

[PATCH] D76238: [SveEmitter] Implement builtins for contiguous loads/stores

SjoerdMeijer added a comment. Thanks for the ping, hadn't noticed the updates. I may have also missed why you need the SVE_ACLE_FUNC macro. Can you quickly comment why you need that? Just asking because in my opinion it doesn't really make it more pleasant to read (so it's there for another rea

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

SjoerdMeijer added a comment. Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1 +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s +

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1 +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s + -

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1 +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s + -

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

SjoerdMeijer added a comment. Just to clarify my last sentence: > Now I am wondering why the ARM SVE ACLE is using float16_t, and not just > _Float16. Do you have any insights in that too perhaps? What I meant to say is why SVE intrinsics are not using _Float16? Repository: rG LLVM Github M

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. I think the float16 discussion is an interesting one, but doesn't necessarily need to be done here. I am asking some questions offline, but if we ever come to a different opinion o

[PATCH] D76680: [SveEmitter] Add immediate checks for lanes and complex imms

SjoerdMeijer added a comment. Looks good to me, but just one question about the tests. If I haven't overlooked anything, I don't see tests that check the new diagnostics: "argument should be the value 90 or 270" "argument should be the value 0,90,180 or 270" Should they be here, or are they some

[PATCH] D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags

SjoerdMeijer added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:229 + // Returns the SVETypeFlags for a given value and mask. + unsigned encodeFlag(unsigned V, StringRef MaskName) const { +auto It = FlagTypes.find(MaskName); Should `V` n

[PATCH] D75056: [ARM][AArch64] Default to -fno-common

SjoerdMeijer created this revision. Herald added a subscriber: kristof.beyls. This patch proposes to default to `-fno-common` for the Arm targets because this has performance and code-size benefits. Additionally, GCC now also defaults to this, so we would be behaving the same as GCC: commit

[PATCH] D75056: [Driver] Default to -fno-common

SjoerdMeijer updated this revision to Diff 246457. SjoerdMeijer retitled this revision from "[ARM][AArch64] Default to -fno-common" to "[Driver] Default to -fno-common". SjoerdMeijer edited the summary of this revision. SjoerdMeijer added reviewers: tstellar, jyknight. SjoerdMeijer added a comment

[PATCH] D75056: [Driver] Default to -fno-common

SjoerdMeijer updated this revision to Diff 246491. SjoerdMeijer added a comment. Thanks, have added a note to the release notes, and also to the command line reference. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ https://reviews.llvm.org/D75056 Files: clang/docs/ClangCo

[PATCH] D75056: [Driver] Default to -fno-common

SjoerdMeijer updated this revision to Diff 246770. SjoerdMeijer added a comment. minor test update CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ https://reviews.llvm.org/D75056 Files: clang/docs/ClangCommandLineReference.rst clang/docs/ReleaseNotes.rst clang/include/cl

[PATCH] D75056: [Driver] Default to -fno-common

SjoerdMeijer updated this revision to Diff 246768. SjoerdMeijer added a comment. Herald added subscribers: kerbowa, nhaehnle, jvesely. Thanks for catching that. Now it shows more changes in tests, so updated a bunch of tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ htt

[PATCH] D75056: [Driver] Default to -fno-common for all targets

SjoerdMeijer updated this revision to Diff 246808. SjoerdMeijer retitled this revision from "[Driver] Default to -fno-common" to "[Driver] Default to -fno-common for all targets". SjoerdMeijer edited the summary of this revision. SjoerdMeijer added a comment. Removed the CHECK-NOTs from some test

[PATCH] D75056: [Driver] Default to -fno-common for all targets

SjoerdMeijer updated this revision to Diff 246917. SjoerdMeijer edited the summary of this revision. SjoerdMeijer added a comment. > Are there any tests remaining that check that with -fcommon, IR generation > creates "common" variables, now that all these tests have been modified? I've added a

[PATCH] D75056: [Driver] Default to -fno-common for all targets

SjoerdMeijer added a comment. Friendly ping, and just checking: are we happy with this? Good to go? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ https://reviews.llvm.org/D75056 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D75056: [Driver] Default to -fno-common for all targets

SjoerdMeijer added a comment. Thanks all for the reviews and help. I will fix these things and do a last rebase/check before committing this tomorrow. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ https://reviews.llvm.org/D75056 ___

[PATCH] D75056: [Driver] Default to -fno-common for all targets

This revision was automatically updated to reflect the committed changes. Closed by commit rG0a9fc9233e17: [Driver] Default to -fno-common for all targets (authored by SjoerdMeijer). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D75056?vs=246917&id=247822#toc

[PATCH] D75056: [Driver] Default to -fno-common for all targets

SjoerdMeijer added a comment. Reverted in: https://reviews.llvm.org/rG4e363563fa14 Put up for review some fixes for compiler-rt tests in: https://reviews.llvm.org/D75520 Now checking what these test-suite failures are about: FAIL: MultiSource/Applications/JM/ldecod/ldecod.compile_time (1 of

[PATCH] D75056: [Driver] Default to -fno-common for all targets

SjoerdMeijer added a comment. Ha, these test-suite apps fail with `multiple definition of ...` link errors, so actually require a little bit of porting! :-) But I think I will prepare a patch that adds `-fcommon` to their makefiles, as I don't want to change too many things at the same time. R

[PATCH] D75056: [Driver] Default to -fno-common for all targets

SjoerdMeijer added a comment. Patch for the test-suite: D75557 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ https://reviews.llvm.org/D75056 ___ cfe-commits

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

SjoerdMeijer added a comment. Big patch, I only did a first scan, but looks very reasonable in general. Just a first round of nits and 2 questions. Comment at: clang/include/clang/Basic/arm_sve.td:121 +// Load one vector (scalar base) +def SVLD1 : MInst<"svld1[_{2}]", "dPc",

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

SjoerdMeijer created this revision. SjoerdMeijer added reviewers: Meinersbur, fhahn, rupprecht. This got reverted because given the following source: void a() { #pragma clang loop vectorize(disable) for (;;) ; } it incorrectly enabled vectorisation and set metadata due to a lo

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

SjoerdMeijer updated this revision to Diff 227273. SjoerdMeijer added a comment. Thanks Michael! - moved the handling of vectorize.enable to 1 place, - that should have also sorted the relative ordering, and duplication of the metadata in some cases, - created a separate file for the new tests,

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

SjoerdMeijer marked an inline comment as done. SjoerdMeijer added inline comments. Comment at: clang/lib/CodeGen/CGLoopInfo.cpp:302-306 +// Imply vectorize.enable when it is not already disabled/enabled. +Args.push_back( +MDNode::get(Ctx, {MDString::get(Ctx, "llvm

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

SjoerdMeijer updated this revision to Diff 229255. SjoerdMeijer added a comment. Thanks again for the suggestion; this indeed (hopefully) looks a lot neater now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69628/new/ https://reviews.llvm.org/D69628 Files: clang/lib/CodeGen/CGLoopIn

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

SjoerdMeijer marked an inline comment as done. SjoerdMeijer added inline comments. Comment at: clang/lib/CodeGen/CGLoopInfo.cpp:302-306 +// Imply vectorize.enable when it is not already disabled/enabled. +Args.push_back( +MDNode::get(Ctx, {MDString::get(Ctx, "llvm

[Diffusion] rG825235c140e7: Revert "[Sema] Use the canonical type in function isVector"

SjoerdMeijer added a comment. I don't think I have full context of the problem but in this diff I see type `__fp16`, which is the storage-only type. But if we are talking about v8.2-A and FP16, then we are talking about the native type (e.g. source-language type `_Float16`), and for that yo

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

This revision was automatically updated to reflect the committed changes. Closed by commit rG021685491727: [Clang] Pragma vectorize_width() implies vectorize(enable) (authored by SjoerdMeijer). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

SjoerdMeijer added inline comments. Comment at: clang/include/clang/Basic/arm_sve.td:121 +// Load one vector (scalar base) +def SVLD1 : MInst<"svld1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad]>; sdesmalen wrote: > SjoerdMeijer wrote: > > This encoding, e.g, this

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

SjoerdMeijer added a subscriber: simon_tatham. SjoerdMeijer added a comment. Adding @simon_tatham in case he feels wants to have a look too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75470/new/ https://reviews.llvm.org/D75470 ___ cfe-co

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

SjoerdMeijer added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5292 + { #NameBase, SVE::BI__builtin_sve_##NameBase, 0, 0, TypeModifier } +static const NeonIntrinsicInfo AArch64SVEIntrinsicMap[] = { +#define GET_SVE_LLVM_INTRINSIC_MAP I am wonder

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Cheers, I think this looks very reasonable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75470/new/ https://reviews.llvm.org/D75470 ___

[PATCH] D75861: [SVE] Generate overloaded functions for ACLE intrinsics.

SjoerdMeijer added inline comments. Comment at: clang/include/clang/Basic/Attr.td:362 +def TargetAArch64 : TargetArch<["aarch64"]>; +def TargetARM_AArch64 : TargetArch; def TargetAVR : TargetArch<["avr"]>; nit: don't thin we use underscores in names, and looking

[PATCH] D75861: [SVE] Generate overloaded functions for ACLE intrinsics.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. With the nit addressed, this LGTM Comment at: clang/utils/TableGen/SveEmitter.cpp:634 " *\n" " * Permission is hereby granted, free of charge, t

[Diffusion] rG825235c140e7: Revert "[Sema] Use the canonical type in function isVector"

SjoerdMeijer added a comment. > I'm still not sure why __fp16, which is a storage-only type, is used for the > element type of float16x4_t if we want to avoid promotion to a float vector > type. About the types: `__fp16` this is the older, storage-only type. So, just historically, I think this

[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:184 + // Also include the Armv8.5 defines + // FIXME: Armv8.6 makes some extensions mandatory. Handle them here. + getTargetDefinesARMV85A(Opts, Builder); Can you be more speci

[PATCH] D76077: [ARM] Add __bf16 as new Bfloat16 C Type

SjoerdMeijer added a comment. That sounds good and that seems to address the feedback given here, which I agree with. Just wanted to briefly add that while it already looks like most interested people are on this ticket, perhaps it good to also share the plan and design on the cfe dev list for

[PATCH] D76238: [SveEmitter] Implement builtins for contiguous loads/stores

SjoerdMeijer added a comment. Some nits inlined Comment at: clang/include/clang/Basic/AArch64SVETypeFlags.h:72 + bool isStructStore() const { return Flags & IsStructStore; } + bool isZxtReturn() const { return Flags & IsZxtReturn; } + nit: this one is non obv

[PATCH] D76238: [SveEmitter] Implement builtins for contiguous loads/stores

SjoerdMeijer added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7467 + + return IsZxtReturn ? Builder.CreateZExt(Load, VectorTy) + : Builder.CreateSExt(Load, VectorTy); sdesmalen wrote: > SjoerdMeijer wrote: > > nit: and now l

[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

SjoerdMeijer added a comment. Besides the irrelevant formatting nits, one minor question about the clang test. Comment at: clang/test/Driver/aarch64-cpus.c:622 + +// The BFloat16 extension is a mandatory component of the Armv8.6-A extensions, but is permitted as an +// optiona

[PATCH] D76238: [SveEmitter] Implement builtins for contiguous loads/stores

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks good to me. Please wait a day in case Eli has more comments. Comment at: clang/include/clang/Basic/arm_sve.td:128 def SVLD1 : MInst<"svld1[_{2}]", "dPc",

[PATCH] D74483: [AArch64] Add Cortex-A34 Support for clang and llvm

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks like the usual business of adding a cpu to me, with one nit inlined that can be fixed before committing. Looks like you're doing the LLVM part separately. Now with the monore

[PATCH] D74483: [AArch64] Add Cortex-A34 Support for clang and llvm

SjoerdMeijer added a comment. Ah, sorry, looks like this is all there is to it, both clang and llvm. It's just that a quick grep locally (for a similar core) showed some more results. Can you (double) check if it needs adding to e.g. a switch in `llvm/lib/Support/Host.cpp`? Repository: rG L

[PATCH] D74483: [AArch64] Add Cortex-A34 Support for clang and llvm

SjoerdMeijer added a comment. Cheers, LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74483/new/ https://reviews.llvm.org/D74483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D74044: [ARM] Add initial support for Custom Datapath Extension (CDE)

SjoerdMeijer added inline comments. Comment at: clang/test/Driver/arm-cde.c:12 + +// RUN: %clang -target arm-none-none-eabi -march=armv8m.main+cdecp0+cdecp3 %s -### -c 2>&1 | FileCheck %s --check-prefix=CHECK-CDE1 +// CHECK-CDE1: "-triple" "thumbv8m.main-none-none-eabi"

[PATCH] D74732: [ARM,CDE] Cosmetic changes, additonal driver tests

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Cheers, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74732/new/ https://reviews.llvm.org/D74732 ___

[PATCH] D45240: [ARM] Compute a target feature which corresponds to the ARM version.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Agree, this is a real mess. This change looks good to me. Was only wondering, we are only testing target features +v7 and +v8, but do we now also need to check the others, like +v

[PATCH] D45515: [NEON] Support vrndns_f32 intrinsic

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D45515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

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: docs/LanguageExtensions.rs

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

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

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

This revision was automatically updated to reflect the committed changes. Closed by commit rL317558: [docs] Add section 'Half-Precision Floating Point' (authored by SjoerdMeijer). Changed prior to commit: https://reviews.llvm.org/D35295?vs=121265&id=121854#toc Repository: rL LLVM https://re

[PATCH] D147497: [AArch64] Use fneg instead of fsub -0.0, X Cin IR expansion of __builtin_neon_vfmsh_f16.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147497/new/ https://reviews.llvm.org/D147497 ___

[PATCH] D34686: [AArch64] Add hasFP16VectorArithmetic helper function. NFCI

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. https://reviews.llvm.org/D3468

[PATCH] D34695: _Float16 preprocessor macro definitions

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

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

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

[PATCH] D34686: [AArch64] Add hasFP16VectorArithmetic helper function. NFCI

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: lib/Basic/Targets.

[PATCH] D34686: [AArch64] ARMV8-A archkind and target defines helper functions

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 co

[PATCH] D34686: [AArch64] ARMV8-A archkind and target defines helper functions

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

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

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 Bu

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

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

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

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

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

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

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

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 FLT_EVAL_M

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

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

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

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

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

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

[PATCH] D42318: [ARM] Pass _Float16 as int or float

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 w

[PATCH] D42318: [ARM] Pass _Float16 as int or float

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 bits

[PATCH] D42318: [ARM] Pass _Float16 as int or float

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

[PATCH] D42318: [ARM] Pass _Float16 as int or float

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: https://reviews.llvm.org/D42318?vs=131009&id=1310

[PATCH] D117753: [AArch64] Support for memset tagged intrinsic

SjoerdMeijer added inline comments. Comment at: clang/lib/Headers/arm_acle.h:734 +/* Memory Operations Intrinsics */ +#if __ARM_FEATURE_MOPS && __ARM_FEATURE_MEMORY_TAGGING +#define __arm_mops_memset_tag(tagged_address, value, size) \ Why doe

[PATCH] D117753: [AArch64] Support for memset tagged intrinsic

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/aarch64-mops.c:3 + +// RUN: %clang_cc1 -triple aarch64-arm-unknown-eabi -target-feature +mops -S -emit-llvm -o - %s | FileCheck %s + I forgot if we add negative tests for these things, i.e. chec

[PATCH] D117753: [AArch64] Support for memset tagged intrinsic

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117753/new/ https://reviews.llvm.org/D117753 ___

[PATCH] D117753: [AArch64] Support for memset tagged intrinsic

SjoerdMeijer added inline comments. Comment at: clang/lib/Headers/arm_acle.h:734 +/* Memory Operations Intrinsics */ +#if __ARM_FEATURE_MOPS && __ARM_FEATURE_MEMORY_TAGGING +#define __arm_mops_memset_tag(tagged_address, value, size) \ dmgreen

[PATCH] D116153: [ARM][AArch64] Add missing v8.x checks

SjoerdMeijer added a comment. Ah, sorry, forgot about this. Can you upload the patch with some more context please so I can have a quick look again? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116153/new/ https://reviews.llvm.org/D116153

[PATCH] D116153: [ARM][AArch64] Add missing v8.x checks

SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:958 case llvm::ARM::ArchKind::ARMV8_6A: + case llvm::ARM::ArchKind::ARMV8_7A: case llvm::ARM::ArchKind::ARMV8_8A: I see tests for the crypto stuff, but is there or do we need a

[PATCH] D116153: [ARM][AArch64] Add missing v8.x checks

SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:958 case llvm::ARM::ArchKind::ARMV8_6A: + case llvm::ARM::ArchKind::ARMV8_7A: case llvm::ARM::ArchKind::ARMV8_8A: tyb0807 wrote: > SjoerdMeijer wrote: > > I see tests for the c

[PATCH] D116153: [ARM][AArch64] Add missing v8.x checks

SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:958 case llvm::ARM::ArchKind::ARMV8_6A: + case llvm::ARM::ArchKind::ARMV8_7A: case llvm::ARM::ArchKind::ARMV8_8A: tyb0807 wrote: > SjoerdMeijer wrote: > > tyb0807 wrote: > > >

[PATCH] D116153: [ARM][AArch64] Add missing v8.x checks

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks, looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116153/new/ https://reviews.llvm.org/D116153

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

SjoerdMeijer added a comment. Ok, fair enough, perhaps adding features is a valid use-case. I will refrain from commenting on "things are terribly broken". I agree it is broken, but in a different way than suggested in previous comments. If others also think this makes sense, then here a few f

[PATCH] D116153: [ARM][AArch64] Add missing v8.x checks

SjoerdMeijer added reviewers: dmgreen, SjoerdMeijer. SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:937 case llvm::ARM::ArchKind::ARMV9_2A: getTargetDefinesARMV83A(Opts, Builder); break; Perhaps unrelated to this patc

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

SjoerdMeijer added a comment. > If anybody has contacts to GCC that would be very helpful. Unfortunately I > don't think I will be able to drive this. Ok, I will bring this up internally first with some folks that work on GCC and see what happens. To be continued... Repository: rG LLVM Gith

[PATCH] D93014: [Clang] Add AArch64 VCMLA LANE variants.

SjoerdMeijer added inline comments. Comment at: clang/include/clang/Basic/arm_neon.td:1911 +// vcmlaq{ROT}_lane +def : SOpInst<"vcmla" # ROT # "_lane", "...qI", "Q" # type, Op<(call "vcmla" # ROT, $p0, $p1, + (bitcast $p0, (dup_typed laneqty , (call "vget_lane"

[PATCH] D93014: [Clang] Add AArch64 VCMLA LANE variants.

SjoerdMeijer accepted this revision. SjoerdMeijer added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/arm_neon.td:1911 +// vcmlaq{ROT}_lane +def : SOpInst<"vcmla" # ROT # "_lane", "...qI", "Q" # type, Op<(call "vc

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. LGTM, perhaps wait a day with committing in case there are more comments. Comment at: clang/include/clang/Basic/Attr.td:3356 EnumArgument<"State", "LoopHintState", - ["en

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks for getting to the bottom of this. Agreed, and also LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93395/new/ https://re

[PATCH] D94779: [Clang] Ensure vector predication pragma is ignored only when vectorization width is 1.

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGenCXX/pragma-loop-predicate.cpp:102 + +// CHECK-NEXT: ![[LOOP7]] = distinct !{![[LOOP7]], [[MP]], [[GEN8]], [[GEN11:![0-9]+]], [[GEN3]]} +// CHECK-NEXT: [[GEN11]] = !{!"llvm.loop.vectorize.width", i32 4} ---

[PATCH] D94779: [Clang] Ensure vector predication pragma is ignored only when vectorization width is 1.

SjoerdMeijer added a comment. I am trying to remember details here, but first about this: > vectorize_width(1) is also used, since that effectively disables > vectorization. I am not sure this is true (i.e. effectively disabling auto-vec) since in LangRef we specify: > The vector width is spe

[PATCH] D94779: [Clang] Ensure vector predication pragma is ignored only when vectorization width is 1.

SjoerdMeijer added a comment. In D94779#2504519 , @malharJ wrote: > I had a look at the Clang Language Extension > > ... and I saw this: > >> Specifying a width/

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

SjoerdMeijer added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666 +CmdArgs.push_back("-mllvm"); +if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465)) + CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1"); If `-m

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

SjoerdMeijer added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1646 - if (Args.getLastArg(options::OPT_mcmse)) + bool fix_cve_2021_35465 = false; + if (Args.getLastArg(options::OPT_mcmse)) { Nit: capital F in variable name. =

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

SjoerdMeijer added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666 +CmdArgs.push_back("-mllvm"); +if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465)) + CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1"); labrin

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

SjoerdMeijer added a comment. The driver part looks good to me. I will let Oli do the approval as he also looked at the codegen (and I didn't). Comment at: clang/include/clang/Driver/Options.td:3274 + Group, + HelpText<"Work around VLLDM erratum CVE-2021-35465 (Arm only)">;

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

SjoerdMeijer added inline comments. Comment at: llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll:6 +; RUN: FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465 + +%indirect = type { double, double, double, double, double, double, double, double } As you'r

[PATCH] D109517: [Clang][ARM][AArch64] Add support for Armv9-A, Armv9.1-A and Armv9.2-A

SjoerdMeijer added reviewers: t.p.northover, ab. SjoerdMeijer added a comment. Some first comments after just looking at the plumbing for these new options. Didn't check yet the architecture extensions for the different version. Comment at: clang/lib/Driver/ToolChains/Arch/AAr

<    1   2   3   4