[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] D136784: [Clang] Improve diagnostic message for loop hint pragma

SjoerdMeijer added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:1306 StringRef Str = PragmaName.getIdentifierInfo()->getName(); + if (Str == "loop") +return (llvm::Twine("clang loop ") + Option.getIdentifierInfo()->getName()).str();

[PATCH] D136783: Pre-commit test case for D136784

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks like a good set of extra tests to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136783/new/

[PATCH] D136784: [Clang] Improve diagnostic message for loop hint pragma

SjoerdMeijer added a reviewer: fhahn. SjoerdMeijer added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:1306 StringRef Str = PragmaName.getIdentifierInfo()->getName(); + if (Str == "loop") +return (llvm::Twine("clang loop ") +

[PATCH] D136611: [Clang][AArch64] Add TargetParser support for defining CPU aliases

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9bd273047d4f: [Clang][AArch64] Add TargetParser support for defining CPU aliases (authored by SjoerdMeijer). Herald added a project: clang. Herald

[PATCH] D136425: [Clang][AArch64] Add support for -mcpu=grace

This revision was automatically updated to reflect the committed changes. Closed by commit rG9ec7448857c1: [Clang][AArch64] Add support for -mcpu=grace (authored by SjoerdMeijer). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

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

SjoerdMeijer added a comment. In D113779#3496589 , @fhahn wrote: > In D113779#3207936 , @SjoerdMeijer > wrote: > >>> If anybody has contacts to GCC that would be very helpful. Unfortunately I >>> don't think I

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

[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

[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] 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) \

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

[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

[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

[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

[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

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

SjoerdMeijer added a comment. This introduces another way of setting (optional) architecture extensions and having two ways to do the same is nearly always a bad thing, which is how one of my colleagues phrased it. This was already a complex area and thus I don't think introducing another is

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

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:419 + + // Enable SVE2 by default on Armv9-A + // It can still be disabled if +nosve2 is present

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

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks, looks reasonable to me. Comment at: llvm/unittests/Support/TargetParserTest.cpp:495 ARMBuildAttrs::CPUArch::v8_A)); +

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

[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

[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: 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");

[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"); If

[PATCH] D105331: [CFE][X86] Enable complex _Float16.

SjoerdMeijer added a comment. The patch on itself looks reasonable to me. I was just wondering about the _Float16 support on X86 in general because the Clang docs says: _Float16 is currently only supported on the following targets, with further targets pending ABI standardization: Does X86

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

SjoerdMeijer added a comment. In D101696#2800790 , @SaurabhJha wrote: > In D101696#2798713 , @SjoerdMeijer > wrote: > >> Perhaps for bonus points, update the Clang documentation in >>

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

SjoerdMeijer added a comment. Perhaps for bonus points, update the Clang documentation in https://clang.llvm.org/docs/LanguageExtensions.html#matrix-types with some examples? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101696/new/

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks, nice one, LGTM. Comment at: llvm/test/CodeGen/AArch64/rand.ll:2 +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN:

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/arm_acle.c:908 +#if __ARM_64BIT_STATE && defined(__ARM_FEATURE_RNG) +// AArch64-v8_3-LABEL: @test_rndr( +// AArch64-v8_3-NEXT: entry: stelios-arm wrote: > SjoerdMeijer wrote: > > Not sure if I

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:364 + if (HasRandGen) +Builder.defineMacro("__ARM_FEATURE_RNG", "1"); + This needs a clang preprocessor test: both a check for its presence and absence. Have a look/grep

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495 let DecoderNamespace = "Fallback"; + let Defs = [NZCV]; } dmgreen wrote: > stelios-arm wrote: > > SjoerdMeijer wrote: > > > SjoerdMeijer wrote: > > > >

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495 let DecoderNamespace = "Fallback"; + let Defs = [NZCV]; } SjoerdMeijer wrote: > dmgreen wrote: > > SjoerdMeijer wrote: > > > dmgreen wrote: > > > >

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495 let DecoderNamespace = "Fallback"; + let Defs = [NZCV]; } dmgreen wrote: > SjoerdMeijer wrote: > > dmgreen wrote: > > > SjoerdMeijer wrote: > > > > Do all

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495 let DecoderNamespace = "Fallback"; + let Defs = [NZCV]; } dmgreen wrote: > SjoerdMeijer wrote: > > Do all MRS instructions do this? > No, but some do and

[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:363 + if (HasRandGen) +Builder.defineMacro("__ARM_FEATURE_RNG", "1"); Where/when is `HasRandGen` set? Comment at:

[PATCH] D98012: [RFC][doc] Document that RISC-V's __fp16 has different behavior

SjoerdMeijer added a comment. > However we would like have slight different behavior for __fp16 other than ACLE: The evaluation format of __fp16 set same as _Float16, which means no promotion are performed if there is no hardware half-precision supported. Well, this is really problematic,

[PATCH] D96866: [AArch64] Add some missing Neoverse features

This revision was automatically updated to reflect the committed changes. Closed by commit rG260f90bb3d1a: [AArch64] Add some missing Neoverse features (authored by SjoerdMeijer). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D93585: [AArch64][Clang][Linux] Enable out-of-line atomics by default.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks, LGTM too. One nit inlined that can be addressed before committing. Comment at: clang/test/Driver/aarch64-features.c:47 +// CHECK-OUTLINE-ATOMICS-ON:

[PATCH] D93585: [AArch64][Clang][Linux] Enable out-of-line atomics by default.

SjoerdMeijer added a comment. Sorry one last question: how about `-mno-outline-atomics` in combination with this? Think we need to add a test for that too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93585/new/ https://reviews.llvm.org/D93585

[PATCH] D93585: [AArch64][Clang][Linux] Enable out-of-line atomics by default.

SjoerdMeijer added a comment. In D93585#2526774 , @ilinpv wrote: > Clang driver tests for outline atomics were added. Thanks! Is there a way we can test `-rtlib=libgcc`? Comment at: clang/lib/Driver/ToolChains/Linux.cpp:855 +

[PATCH] D93585: [AArch64][Clang][Linux] Enable out-of-line atomics by default.

SjoerdMeijer added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6448 + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("+outline-atomics"); +} This needs a Clang driver tests. Repository: rG LLVM Github Monorepo

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

SjoerdMeijer added a comment. In D94779#2515157 , @dmgreen wrote: > Thanks. @fhahn @SjoerdMeijer what do we think about the edge case where the > width==1? As far as I understand (with this patch): > > #pragma clang loop vectorize_predicate(disable)

[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

[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

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

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

[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

[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

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

SjoerdMeijer added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3080 } Opts.setFPExceptionMode(FPEB); jansvoboda11 wrote: > SjoerdMeijer wrote: > > jansvoboda11 wrote: > > > The parsing of `OPT_ftrapping_math` and

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

SjoerdMeijer added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3080 } Opts.setFPExceptionMode(FPEB); jansvoboda11 wrote: > The parsing of `OPT_ftrapping_math` and `OPT_fno_trapping_math` is > immediately overwritten here.

[PATCH] D91994: [AArch64] Cortex-R82: remove crypto

This revision was automatically updated to reflect the committed changes. Closed by commit rG99ad078b91ed: [AArch64] Cortex-R82: remove crypto (authored by SjoerdMeijer). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE

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

SjoerdMeijer added a comment. Thanks @david-arm for posting this proposal to the cfe list. My confusion has been cleared up. The (new) proposal is to have: 1. vectorize_width(X) where X is an integer. 2. vectorize_width(X, fixed|scalable) 3. vectorize_width(fixed|scalable) And with that 3rd

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

SjoerdMeijer added a comment. Because I was not understanding, we have discussed this further offline. I think the conclusion was: pragma `vectorize_width` controls the vectorisation vector `VF` in ``. where `vscale` is not just a separate thing but it defines a VectorType. That's why it would

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

SjoerdMeijer added a comment. > This approach should be fully complementary to `vectorize_with` so that it > would be possible to have: > > // Use scalable vectors, but leave it to the cost-model to choose the most > efficient N in . > // If the pragma is not specified, it defaults to

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

SjoerdMeijer added a comment. In D89031#2391160 , @david-arm wrote: > Hi @SjoerdMeijer I think that given we now support scalable vectors we > thought it made sense to be able to specify whether the user wants 'fixed' or > 'scalable' vectorisation along

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

SjoerdMeijer added a comment. I am very sorry that I am late to this... but I do have some concerns. The concern that I have is that we extend vecorize_width with a scalable/fixed boolean, but there are more vectorisation pragma that set vectorisation options which imply enabling

[PATCH] D89972: Add pipeline model for HiSilicon's TSV110

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. I haven't checked the instruction descriptions in detail, but the overall structure looks good to me. Perhaps wait a day with committing in case @bryanpkc has more comments.

[PATCH] D89972: Add pipeline model for HiSilicon's TSV110

SjoerdMeijer added inline comments. Comment at: clang/test/Driver/aarch64-cpus.c:298 +// RUN: %clang -target aarch64 -mcpu=tsv110 -### -c %s 2>&1 | FileCheck -check-prefix=TSV110 %s +// RUN: %clang -target aarch64 -mlittle-endian -mcpu=tsv110 -### -c %s 2>&1 | FileCheck

[PATCH] D88660: [AArch64] Add CPU Cortex-R82

This revision was automatically updated to reflect the committed changes. Closed by commit rG8825fec37e73: [AArch64] Add CPU Cortex-R82 (authored by SjoerdMeijer). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D84703: [clang codegen][AArch64] Use llvm.aarch64.neon.fcvtzs/u where it's necessary

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

[PATCH] D84180: [Matrix] Add LowerMatrixIntrinsics to the NPM

This revision was automatically updated to reflect the committed changes. Closed by commit rG5567c62afa55: [Matrix] Add LowerMatrixIntrinsics to the NPM (authored by SjoerdMeijer). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D83079: [clang][aarch64] Generate preprocessor macros for -march=armv8.6a+sve.

SjoerdMeijer added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:118 + + if (!llvm::AArch64::getArchFeatures(ArchKind, Features)) +return false; Would it be more consistent to move this Comment at:

[PATCH] D83079: [clang] Default target features implied by `-march` on AArch64.

SjoerdMeijer added a comment. I haven't looked into details of these arch extensions yet, will do that tomorrow, but I don't think there's any disagreement, i.e., these options and their behaviour are synchronised with the GCC community. Thus, it's better if you remove this wording from both

[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml

SjoerdMeijer marked an inline comment as done. SjoerdMeijer added inline comments. Comment at: test/Driver/aarch64-cpus.c:518 +// RUN: %clang -target aarch64 -march=armv8.4-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV84A-NO-FP16FML %s +// GENERICV84A-NO-FP16FML-NOT:

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

SjoerdMeijer added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:524 + CallConv)) +return; EVT ValueVT = Val.getValueType(); efriedma wrote: > pratlucas wrote: > > efriedma wrote: >

[PATCH] D80716: [AArch64]: BFloat Load/Store Intrinsics

SjoerdMeijer added inline comments. Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:264 +; Function Attrs: argmemonly nounwind readonly +declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld2lane.v8bf16.p0i8(<8 x bfloat>, <8 x bfloat>, i64, i8*) #3

[PATCH] D80716: [AArch64]: BFloat Load/Store Intrinsics

SjoerdMeijer added inline comments. Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:264 +; Function Attrs: argmemonly nounwind readonly +declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld2lane.v8bf16.p0i8(<8 x bfloat>, <8 x bfloat>, i64, i8*) #3

[PATCH] D80716: [AArch64]: BFloat Load/Store Intrinsics

SjoerdMeijer added inline comments. Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:264 +; Function Attrs: argmemonly nounwind readonly +declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld2lane.v8bf16.p0i8(<8 x bfloat>, <8 x bfloat>, i64, i8*) #3

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

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

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

SjoerdMeijer added a comment. > At the moment when going through the GCC compatibility driver (standard > interface), we get __bf16 is not supported on this target. If this is the current behaviour, and consistent with GCC, that sounds reasonable. Probably best to be explicit about this and

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

SjoerdMeijer added a comment. Can you summarise where we are? I.e., - float-abi=soft doesn't work. But what is the problem? Are we not simply passing i16s, is that not what we are supposed to do? Can you also update the description of this patch, I got totally confused by: - "introduces an

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

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/arm-bf16-params-returns.c:5 +// RUN: %clang_cc1 -triple aarch64-arm-none-eabi -target-abi aapcs -mfloat-abi softfp -target-feature +bf16 -target-feature +neon -emit-llvm -O2 -o - %s | opt -S -mem2reg -sroa |

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

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/arm-mangle-bf16.cpp:3 +// RUN: %clang_cc1 -triple arm-arm-none-eabi -target-feature +bf16 -mfloat-abi hard -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK32-HARD +// RUN: %clang_cc1 -triple

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

SjoerdMeijer added a comment. As I wrote in one of the inlined comments, I am relatively unhappy that we have both bfloat and bfloat16 as names. But that looks like a complain for another patch, and not really this one. Comment at: clang/lib/Basic/Targets/AArch64.cpp:74 +

[PATCH] D79708: [clang][BFloat] add NEON emitter for bfloat

SjoerdMeijer added a comment. No objections. Some nits inlined, which you can ignore if you think they are not correct. Comment at: clang/include/clang/Basic/arm_neon_incl.td:218 // d: double +// b: bfloat // nit: perhaps bfloat16? Comment

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. Best to ask the release manager. Today Tom Stellard posted a message to the llvm dev list with subject: [llvm-dev] LLVM 10.0.1-rc1 release update So best to reply to this(*) asap with your query if it's possible to get this onto the branch. (*)

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

SjoerdMeijer added a comment. There are Sema and CodeGen tests, but was wondering if there would be some value in having an AST test too? There are some other types that do have AST tests. Comment at: clang/lib/AST/ASTContext.cpp:2052 + Width =

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

SjoerdMeijer added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8111 "pointer cannot be cast to type %0">; +def err_cast_to_bfloat : Error<"cannot type-cast to __bf16">; +def err_cast_from_bfloat : Error<"cannot type-cast from __bf16">;

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

SjoerdMeijer added inline comments. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three half-precision (16-bit) floating point types: ``__fp16``, +``_Float16`` and ``__bf16``. These types are supported in all language modes. stuij wrote: >

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

SjoerdMeijer added inline comments. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three half-precision (16-bit) floating point types: ``__fp16``, +``_Float16`` and ``__bf16``. These types are supported in all language modes. stuij wrote: >

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

SjoerdMeijer added inline comments. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three half-precision (16-bit) floating point types: ``__fp16``, +``_Float16`` and ``__bf16``. These types are supported in all language modes. Not my field

[PATCH] D78190: Add Bfloat IR type

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. Nothing much has changed here: there was already broad consensus on this change and direction, and the last few weeks we have only seen a few rounds of minor comments and nits, so still LGTM. Please wait a day with committing to

[PATCH] D79708: [clang][BFloat] add NEON emitter for bfloat

SjoerdMeijer added inline comments. Comment at: clang/include/clang/Basic/arm_bf16.td:1 +//===--- arm_fp16.td - ARM FP16 compiler interface ===// +// typo: fp16 - > bf16? Here, and a few more places, or is it intentional? If so, I guess

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. Forgot to ask/add: can you commit this, do you have commit rights? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78129/new/ https://reviews.llvm.org/D78129 ___

[PATCH] D78129: Add Marvell ThunderX3T110 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/D78129/new/ https://reviews.llvm.org/D78129

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. This looks good now, but sorry, one more request: I've just noticed a Clang driver test is missing. Can you add a test for this to `clang/test/Driver/aarch64-cpus.c`? And related to this, the relevant tests to `llvm/unittests/Support/TargetParserTest.cpp`?

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. In D78129#2023772 , @joelkevinjones wrote: > I don't think it makes sense to combine two unrelated things SVE and PA > support into a combined thing. Since we already have UnsupportedFeatures in > every sub-target .td

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. Let's separate out HasPA from SVEFeatures now while we are at it, probably it's more work to do this as a follow up, and after that this looks good to me. Bonus points for adding some llvm-mca tests, see the `llvm-project/llvm/test/tools/llvm-mca/` directory for

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. Agreed about the UnsupportedFeatures list. But I just wanted to unblock this work, create a first version that compiles, so you can pick it up and clean things further up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. Since debugging on phabricator is a bit difficult, I took the patch and had a little play. Now I actually remember seeing this before. I think the way this works is that when you describe new instructions (PA in this case) that other models don't have, they start

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added a comment. In D78129#2022307 , @joelkevinjones wrote: > In email Wei asked for help about he following error message: > > error message from tblgen > Included from >

[PATCH] D78129: Add Marvell ThunderX3T110 support

SjoerdMeijer added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64.td:849 + HasV8_1aOps, + HasV8_3aOps]>; + `HasV8_3aOps` implies `HasV8_2aOps`, which implies

[PATCH] D78995: [SveEmitter] NFCI: Describe splat operands like any other modifier

SjoerdMeijer added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:68 TypeSpec TS; + bool IsSplat; bool Float, Signed, Immediate, Void, Constant, Pointer; I was wondering if IsSplat belongs here or somewhere else. It doesn't sound like a

[PATCH] D78190: Add Bfloat IR type

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. This direction of an IR type was taken after a discussion on the list. All previous comments have been addressed, and the changes here all look very reasonable to me. So, LGTM,

[PATCH] D78565: [clang][doc] Clang ARM CPU command line argument reference

SjoerdMeijer added a comment. To be completely open about this, I had an offline chat with @psmith and @kristof.beyls about this. The reasoning is that this is probably related to my relatively poor choice of the 2 first cores that I describe here. They are microcontrollers, and Clang/LLVM

  1   2   3   4   >