[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

dmgreen added a comment. I hadn't realized this came from someone at Arm. The performance results I had were overall roughly flat, with some improvements and regressions. I think there were still some people working through some fixes for some of the knock-on effects but with those nothing

[PATCH] D159480: [Clang][AArch64] Fine-grained ldp and stp policies.

dmgreen added a comment. We do not usually add front-end clang options for optimizations like this. Users are more likely to use them incorrectly, or just not know that they exist. The usual method would be to make a subtarget tuning feature that controls whether ldp are created, and enable it

[PATCH] D158626: [AArch64] Add missing vrnd intrinsics

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Thanks. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158626/new/ https://reviews.llvm.org/D158626 ___ cfe-commits mailing list

[PATCH] D159480: [Clang][AArch64] Fine-grained ldp and stp policies.

dmgreen added a comment. Can you give more details about why this is wanted and in which cases it helps with? Is it an optimization, as opposed to working around some correctness issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159480/new/

[PATCH] D158626: [AArch64] Add missing vrnd intrinsics

dmgreen added a comment. Other than where the pattern gets defined this looks OK to me. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:6309 + + def : Pat<(v1f64 (OpNode (v1f64 FPR64:$Rn))), + (!cast(NAME # Dr) FPR64:$Rn)>; miyengar

[PATCH] D158626: [AArch64] Add missing vrnd intrinsics

dmgreen added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:6309 + + def : Pat<(v1f64 (OpNode (v1f64 FPR64:$Rn))), + (!cast(NAME # Dr) FPR64:$Rn)>; I think the instructions in this multiclass are the vector variants. Can

[PATCH] D159174: [Clang] Use stable_sort in AppendTargetMangling

dmgreen added a comment. Do you have a test-case where they were out of order? Or is that dependent on the C++ library? I think I just moved this code from elsewhere when I changed it, but it sounds like a sensible change to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D158626: [AArch64] Add missing vrnd intrinsics

dmgreen added a comment. Sounds good. Can you make sure you upload with context, it makes the patches easier to read in phabricator: https://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:6297

[PATCH] D158008: [AArch64] Add patterns for FMADD, FMSUB

dmgreen added a comment. It looks like there are patterns for A[0]*B+C and B*A[0]*C to handle commutivity. Are there tests for both forms too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158008/new/ https://reviews.llvm.org/D158008

[PATCH] D156799: Update generic scheduling to use A510 scheduling model

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Thanks. Looks good. You may need to rebase over new tests, make sure everything is still passing when you do. LGTM Comment at:

[PATCH] D147266: [AArch64] Sink operands to allow for bitselect instructions

dmgreen accepted this revision. dmgreen added a comment. Thanks. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147266/new/ https://reviews.llvm.org/D147266 ___ cfe-commits mailing list

[PATCH] D147266: [AArch64] Sink operands to allow for bitselect instructions

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Thanks. LGTM with a few extra suggestions. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14357 + // Non-mask operands of both Ands should also be in

[PATCH] D147266: [AArch64] Sink operands to allow for bitselect instructions

dmgreen added a comment. Thanks for working on this. I noticed there was another instance of vbsl being reported recently in https://github.com/llvm/llvm-project/issues/62642. Hopefully it can be addresses via extra optimizations too. Can you add a testcase for the issues in

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

dmgreen added a comment. It looks like there is quite a lot more optimization that happens to the function being always-inlined (__SSAT) before this change. Through multiple rounds of instcombine, almost to the end of the pass pipeline. The new version runs a lot less before inlining, only

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

dmgreen added subscribers: nikic, spatel, efriedma. dmgreen added a comment. Hello. It sounds like it is really close to being OK. The combine of the shift just seem to make things more difficult. The `icmp ult i1 %cmp4.i, true` is just a not, would it help if it was actually an xor? Or if the

[PATCH] D147266: [AArch64] Add IR intrinsics for vbsl* C intrinsics

dmgreen added a comment. My preference would be for fixing the code we have, not introducing new intrinsics. Intrinsics act as black-boxes for the optimizer, and I'm pretty sure I've heard of cases in the past of the compiler optimizing the or/and/xor's to nicer sequences of instructions. It

[PATCH] D148919: [Clang][Sema] Fix invalid cast when validating SVE types within CheckVariableDeclarationType.

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. This LGTM, but I don't remember why it was written this way. (I am not an expert in this area either). It seems to still give correct error messages with/without

[PATCH] D146987: [Assignment Tracking] Enable by default

dmgreen added a comment. Hello, I also noticed this potentially causing problems for scalable vectors: https://godbolt.org/z/qdr8P86aW That probably counts as one of the "edge cases for things we hadn't accounted for". Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

dmgreen accepted this revision. dmgreen added a comment. Sounds OK to me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147497/new/ https://reviews.llvm.org/D147497 ___ cfe-commits mailing list

[PATCH] D142135: [AArch64] Basic target("+crypto") handling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG43aa293aeaf0: [AArch64] Basic target(+crypto) handling (authored by dmgreen). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

dmgreen added a comment. Hi. I created this issue about the use of Ofast and -1: https://github.com/llvm/llvm-project/issues/60781 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109239/new/ https://reviews.llvm.org/D109239 ___ cfe-commits

[PATCH] D127910: [Clang][AArch64][SME] Add vector load/store (ld1/st1) intrinsics

dmgreen added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:1477 + + OS << "#if !defined(__ARM_FEATURE_SME)\n"; + OS << "#error \"SME support not enabled\"\n"; bryanpkc wrote: > dmgreen wrote: > > We have been changing how the existing SVE

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

dmgreen added a comment. > I’ll take a look, but this indicates to me that there’s something missing > from the vectoriser or later passes rather than a problem with the inliners > behaviour. Sure. I'm not saying that this patch is wrong. I'm just saying that unfortunately it leads to some

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

dmgreen added a comment. > It’s not clear from the original commit message why the test is related to > inlining order? It seems entirely testing vectorization cost model which > should be insensitive to these kind of changes, right? It's a phase ordering test - it's testing the entire

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

dmgreen added a comment. Hello - I had to revert this because of some large regressions we got from routines in CMSIS-DSP. The llvm/test/Transforms/PhaseOrdering/ARM/arm_mult_q15.ll test shows the problem - that's why that test exists to ensure that any pipeline changes don't negatively

[PATCH] D142963: [AArch64] Handle negative architecture features

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf559e781b2bd: [AArch64] Handle negative architecture features (authored by dmgreen). Herald added a project: clang. Herald added a subscriber:

[PATCH] D142988: [AArch64] Make nosimd imply nocrypto

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG00ce96b02e87: [AArch64] Make nosimd imply nocrypto (authored by dmgreen). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D142548: [AArch64] Replace AEK_CRYPTO with relevant features in cpu definitions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8f6c623e8746: [AArch64] Replace AEK_CRYPTO with relevant features in cpu definitions (authored by dmgreen). Herald added a project: clang. Herald

[PATCH] D127910: [Clang][AArch64][SME] Add vector load/store (ld1/st1) intrinsics

dmgreen added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:1477 + + OS << "#if !defined(__ARM_FEATURE_SME)\n"; + OS << "#error \"SME support not enabled\"\n"; We have been changing how the existing SVE and NEON instrinsics work a little.

[PATCH] D142396: [AArch64] Add the Ampere1A core

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. OK Thanks for the clarification. LGTM then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142396/new/ https://reviews.llvm.org/D142396

[PATCH] D142396: [AArch64] Add the Ampere1A core

dmgreen added inline comments. Comment at: llvm/include/llvm/TargetParser/AArch64TargetParser.h:492 +{"ampere1a", ARMV8_6A, + (AArch64::AEK_FP16 | AArch64::AEK_RAND | AArch64::AEK_SM4 | + AArch64::AEK_SHA3 | AArch64::AEK_SHA2 | AArch64::AEK_AES |

[PATCH] D142396: [AArch64] Add the Ampere1A core

dmgreen added a comment. Sounds OK, but do you mind splitting FeatureFuseAddSub2RegAndConstOne into a separate patch. They seem to be logically separable, and it can help in case there are problems found in one of the patches. Comment at:

[PATCH] D142410: [AArch64] ARMv8.5-A implies both FEAT_SB and FEAT_SSBS

dmgreen added reviewers: lenary, tmatheson, pratlucas. dmgreen added a comment. I believe this is correct, according to at least one reference I have. FEAT_SSBS isn't spelled out very clearly in the reference manual though. Adding some more people who might be able to check. Repository: rG

[PATCH] D141606: [AArch64] Remove AES, SHA2, SHA3 and SM4 features from armv8.6-a+

dmgreen added a comment. In D141606#4074204 , @ilinpv wrote: > Sorry, commit rG5474d7d93271 > is not > related to this, I put wrong differential revision link Not a problem, I was

[PATCH] D142087: [AArch64] Armv9-A implies FP16

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGee700dec052a: [AArch64] Armv9-A implies FP16 (authored by dmgreen). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed

[PATCH] D141518: [AArch64] Move default extensions from clang Driver to TargetParser

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0f422215ac63: [AArch64] Move default extensions from clang Driver to TargetParser (authored by dmgreen). Herald added a project: clang. Herald added

[PATCH] D141056: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests.

dmgreen added a comment. I removed -O1 from clang sve tests a while ago. The intent was always to remove instcombine and tailcallelim too, but I never got around to it. Feel free to remove -O1 from the other sve2 tests too, they ideally shouldn't test the entire pipeline from clang.

[PATCH] D131058: [AArch64] Add an error if SVE scalable vector types are used in a context without sve

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG753aca0a3ab0: [AArch64] Add an error if SVE scalable vector types are used in a context… (authored by dmgreen). Herald added a project: clang.

[PATCH] D141411: [AArch64] Make -march and target("arch=..") attributes imply dependent features

dmgreen added a comment. It does look like this might be enabling Crypto in places it was not in the past though. I'll look into why that is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141411/new/ https://reviews.llvm.org/D141411

[PATCH] D141404: [AArch64][Clang] Adjust default features for v8.9-A/v9.4-A in clang driver

dmgreen added a comment. I've put up D141518 to move the existing features. Let me know what you think Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141404/new/ https://reviews.llvm.org/D141404

[PATCH] D141404: [AArch64][Clang] Adjust default features for v8.9-A/v9.4-A in clang driver

dmgreen added a comment. Do you know why this code is in the Driver? I would have expected mandatory features to be added to the AARCH64_ARCH in AArch64TargetParser.td. I don't think they are then needed in the driver. I can put up a patch to show what I mean, as I think the same applies to

[PATCH] D141411: [AArch64] Make -march and target("arch=..") attributes imply dependent features

dmgreen added a comment. In D141411#4042891 , @lenary wrote: > Can we check this logic, especially around adding both AEK_SVE and AEK_SVE2 > in the `AARCH64_ARCH` descriptions, this means that `-march=armv9-a+nosve2` > still can generate sve

[PATCH] D141411: [AArch64] Make -march and target("arch=..") attributes imply dependent features

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0c0127bb9f00: [AArch64] Make -march and target(arch=..) attributes imply dependent features (authored by dmgreen). Herald added a project: clang.

[PATCH] D132639: [AArch64] Alter arm_neon_sve_bridge.h to be target-based, not preprocessor based.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG997852920d52: [AArch64] Alter arm_neon_sve_bridge.h to be target-based, not preprocessor… (authored by dmgreen). Herald added a project: clang.

[PATCH] D131064: [AArch64] Alter arm_sve.h to be target-based, not preprocessor based.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6cac7c285e69: [AArch64] Alter arm_sve.h to be target-based, not preprocessor based. (authored by dmgreen). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D140432: [AArch64] Guard {vmull_p64, vmull_high_p64} with 'aes' target guard.

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Thanks for putting together the patch. It may be worth mentioning in the commit message that aes currently includes both FEAT_AES and FEAT_PMULL. Otherwise LGTM. Repository: rG LLVM

[PATCH] D139525: Add implementation isTargetCanonicalConstantNode for hexagon.

dmgreen added a reviewer: kparzysz. dmgreen added a comment. Can you add the test cases from D137140 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139525/new/ https://reviews.llvm.org/D139525

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

dmgreen added inline comments. Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:115 +AARCH64_ARCH_EXT_NAME("rdm", AArch64::AEK_RDM, "+rdm", "-rdm", \ +RDM, "+rdm,+fp-armv8,+neon,+jsconv,+complxnum",

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

dmgreen added a comment. I can only comment on the target features part of the patch - I've been hoping it would add something similar and looks very useful in its own right. Comment at: clang/lib/Basic/Targets/AArch64.cpp:63 + switch (ArchKind) { + case

[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

dmgreen added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:11772 + : I, +Sched<[]> { + bits<5> Rt; One extra nit: Can we add a scheduling description? These sound like they can use WriteAtomic. Repository: rG

[PATCH] D138488: [AArch64][clang] implement 2022 General Data-Processing instructions

dmgreen accepted this revision. dmgreen added a comment. Thanks. I haven't looked though all the details, but thanks for fixing the scheduling info. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138488/new/ https://reviews.llvm.org/D138488

[PATCH] D138488: [AArch64][clang] implement 2022 General Data-Processing instructions

dmgreen added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:11725 +: BaseTwoOperandRegReg, + Sched<[]> { + let Inst{11} = isMin; stuij wrote: > dmgreen wrote: > > Can we make this WriteI, maybe. I think that would probably

[PATCH] D138488: [AArch64][clang] implement 2022 General Data-Processing instructions

dmgreen added inline comments. Comment at: llvm/include/llvm/Support/AArch64TargetParser.h:79 + AEK_SMEF16F16 = 1ULL << 47, // FEAT_SMEF16F16 + AEK_CSSC =1ULL << 49, // FEAT_CSSC }; 48!? Comment at:

[PATCH] D137617: [AArch64] Allow users-facing feature names in clang target attributes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf0e6c403c2d3: [AArch64] Allow users-facing feature names in clang target attributes (authored by dmgreen). Herald added a project: clang. Herald

[PATCH] D137256: [AArch64] Alter arm_fp16.h to be target-based, not preprocessor based.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe7deca525058: [AArch64] Alter arm_fp16.h to be target-based, not preprocessor based. (authored by dmgreen). Herald added a project: clang. Herald

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Thanks. LGTM if there are not other comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136589/new/ https://reviews.llvm.org/D136589

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

dmgreen added inline comments. Comment at: clang/docs/ReleaseNotes.rst:696 -- Add driver and tuning support for Neoverse V2 via the flag ``-mcpu=neoverse-v2``. - Native detection is also supported via ``-mcpu=native``. vhscampos wrote: > tschuett wrote: > >

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

dmgreen added a subscriber: david-arm. dmgreen added inline comments. Comment at: clang/docs/ReleaseNotes.rst:696 -- Add driver and tuning support for Neoverse V2 via the flag ``-mcpu=neoverse-v2``. - Native detection is also supported via ``-mcpu=native``.

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

dmgreen added a comment. Thanks. What was wrong with the v9-A features? Comment at: llvm/unittests/Support/TargetParserTest.cpp:1083 + AArch64::AEK_SVE | AArch64::AEK_SVE2 | + AArch64::AEK_SVE2BITPERM | AArch64::AEK_SB |

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

dmgreen added inline comments. Comment at: clang/docs/ReleaseNotes.rst:629-631 - Add driver and tuning support for Neoverse V2 via the flag ``-mcpu=neoverse-v2``. Native detection is also supported via ``-mcpu=native``. +- Support has been added for the following processors

[PATCH] D136957: [AArch64] Add support for the Cortex-A715 CPU

dmgreen added inline comments. Comment at: llvm/lib/Support/Host.cpp:216 .Case("0xd41", "cortex-a78") +.Case("0xd00", "cortex-a715") .Case("0xd44", "cortex-x1") Should be 0xd4d I think, according to the MIDR in the TRM.

[PATCH] D135647: [AArch64][ARM] Alter v8.3a complex neon intrinsics to be target-based, not preprocessor based

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaf1bb287b4de: [AArch64][ARM] Alter v8.3a complex neon intrinsics to be target-based, not… (authored by dmgreen). Herald added a project: clang.

[PATCH] D135615: [AArch64][ARM] Alter v8.1a neon intrinsics to be target-based, not preprocessor based

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9c48b7f0e7a9: [AArch64][ARM] Alter v8.1a neon intrinsics to be target-based, not preprocessor… (authored by dmgreen). Herald added a project: clang.

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

dmgreen added a comment. Can you also add a test to clang/test/Driver/aarch64-mcpu.c and the right code to llvm/lib/Support/Host.cpp if you can find it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136589/new/ https://reviews.llvm.org/D136589

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

dmgreen added reviewers: lenary, dmgreen. dmgreen added inline comments. Comment at: clang/docs/ReleaseNotes.rst:397 configuration files. +- Support has been added for the following processors (-mcpu identifiers in parentheses): + This should be in the "Arm

[PATCH] D135646: [AArch64] Alter v8.5a FRINT neon intrinsics to be target-based, not preprocessor based

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f1e43036059: [AArch64] Alter v8.5a FRINT neon intrinsics to be target-based, not… (authored by dmgreen). Herald added a project: clang. Herald

[PATCH] D131504: [ARM][AArch64] Dont use macros for half instrinsics in NeonEmitter

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4987ae84622b: [ARM][AArch64] Dont use macros for half instrinsics in NeonEmitter (authored by dmgreen). Herald added a project: clang. Herald added

[PATCH] D133848: [Clang][AArch64] Support AArch64 target(..) attribute formats.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG781b491bba9d: [Clang][AArch64] Support AArch64 target(..) attribute formats. (authored by dmgreen). Herald added a project: clang. Herald added a

[PATCH] D133885: [Clang][Arm] Convert -fallow-half-arguments-and-returns to a target option. NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG123064dc397d: [Clang][Arm] Convert -fallow-half-arguments-and-returns to a target option. NFC (authored by dmgreen). Herald added a project: clang.

[PATCH] D134717: [Clang][AArch64] Fix va_arg with -mgeneral-regs-only

dmgreen added a comment. > With -mgeneral-regs-only, all arguments are passed in GPRs, so we should use > gr_top/gr_offs in va_list even for floating-point types. I don't believe that -mgeneral-regs-only was intended to define an ABI passing floats in GPRs for AArch64. If it has that is likely

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Thanks. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134352/new/ https://reviews.llvm.org/D134352 ___ cfe-commits mailing list

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

dmgreen added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64.td:1112 + FeatureNEON, FeatureSVE2BitPerm, FeatureFP16FML, + FeatureMTE, FeatureRandGen]; list Saphira= [HasV8_4aOps,

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

dmgreen added a comment. Oh, A release note would be good to add too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134352/new/ https://reviews.llvm.org/D134352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

dmgreen added a comment. Thanks. Should RNG be added? And Why is Crypto no longer enabled? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134352/new/ https://reviews.llvm.org/D134352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D134127: [ARM] Check target feature support for __builtin_arm_crc*

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Whether you do that here or in another patch, the change LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134127/new/

[PATCH] D134127: [ARM] Check target feature support for __builtin_arm_crc*

dmgreen added a comment. The arm_acle.h header is shared across Arm and AArch64 - they use the same builtin names even if they are replicated to a certain degree across the backends. As this is used in both it would be good to fix them at the same time. It doesn't need to change the guards as

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

dmgreen added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64.td:941 + "Neoverse V2 ARM processors", [ + FeaturePostRAScheduler]>; + Please add FeatureFuseAES and

[PATCH] D134127: [ARM] Check target feature support for __builtin_arm_crc*

dmgreen added reviewers: danielkiss, ilinpv. dmgreen added a comment. This looks like a subset of D133359 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134127/new/ https://reviews.llvm.org/D134127

[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

dmgreen added a comment. > Is there a formal requirement that LLVM must remain backward compatible with > older LLVM IR (beyond the target-independent parts)? We have always done it in the past, and I don't see a good reason to change. This change is essentially for llvm 16, so we are talking

[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

dmgreen added a comment. In D131547#3728842 , @sdesmalen wrote: > In D131547#3723083 , @dmgreen wrote: > >> I think we usually try to do the same, if the intrinsics have been in >> released compilers. There is

[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

dmgreen added a comment. I think we usually try to do the same, if the intrinsics have been in released compilers. There is an example in https://reviews.llvm.org/D98487#change-tOTTgECYYAO5, hopefully these would be equally simple. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D127812: [AArch64] Function multiversioning support added.

dmgreen added inline comments. Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:188 +AARCH64_CPU_FEATURE("sve_aes",SVE_AES, "+sve2-aes", 330) +AARCH64_CPU_FEATURE("sve_pmull128", SVE_PMULL128, "", 340)

[PATCH] D130973: [AArch64] Always allow the __bf16 type

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8c30f4a5ab3e: [AArch64] Always allow the __bf16 type (authored by dmgreen). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D128415: [ARM] Add Support for Cortex-M85

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Other than the release note change it might be worth adding some tests for -mcpu=cortex-m85+nopacbti and related configurations. Otherwise LGTM Comment at:

[PATCH] D128415: [ARM] Add Support for Cortex-M85

dmgreen added inline comments. Comment at: llvm/lib/Target/ARM/ARM.td:1443 +def : ProcessorModel<"cortex-m85", CortexM4Model, [ARMv81mMainline, + FeatureDSP, Please use the CortexM7 schedule - it is

[PATCH] D125604: [FileCheck] Catch missspelled directives.

dmgreen added inline comments. Comment at: llvm/test/CodeGen/AArch64/fp16-v8-instructions.ll:682-688 +; TODO-CVT-DAG: fcvtl [[LO:v[0-9]+\.4s]], v0.4h +; TODO-CVT-DAG: fcvtl2 [[HI:v[0-9]+\.4s]], v0.8h +; TODO-CVT-DAG: fcvtzs [[LOF32:v[0-9]+\.4s]], [[LO]] +;

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen accepted this revision. dmgreen 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/D119720/new/ https://reviews.llvm.org/D119720

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:146 + case ARM::VMVNd: + case ARM::VMVNq: + // VMOV of 64-bit value between D registers (when condition = al) lenary wrote: > dmgreen wrote: > > Perhaps add

[PATCH] D117112: [AArch64] Support for Ampere1 core

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG64816e68f441: [AArch64] Support for Ampere1 core (authored by philipp.tomsich, committed by dmgreen). Repository: rG LLVM Github Monorepo

[PATCH] D117112: [AArch64] Support for Ampere1 core

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. I don't know the details of the scheduling latencies for the core, but this looks perfectly sensible. There are a few comments inline, but other than those this patch LGTM.

[PATCH] D115620: [AArch64] Lowering and legalization of strict FP16

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. OK Thanks. This LGTM then. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115620/new/ https://reviews.llvm.org/D115620 ___ cfe-commits

[PATCH] D115620: [AArch64] Lowering and legalization of strict FP16

dmgreen added a comment. I might have misunderstood which this patch was. Can you move the clang test into D118259 with the other? It seems like the same problem, and it looks like there should be enough llc tests to cover all the cases.

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added a comment. Looks OK to me, as far as I can see. If it worth adding a few extra instructions that may come up? Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:146 + case ARM::VMVNd: + case ARM::VMVNq: + // VMOV of 64-bit value between D

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:200-207 + case ARM::VMOVv2f32: + case ARM::VMOVv4f32: + case ARM::VMOVv2i32: + case ARM::VMOVv4i32: + case ARM::VMOVv4i16: + case ARM::VMOVv8i16: + case ARM::VMOVv8i8:

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:145 + case ARM::VMVNq: +return CondCodeIsAL(3); + // VMOV of 64-bit value between D registers (when condition = al) Can/should all these use

[PATCH] D122046: [clang] Remove Address::deprecated from MveEmitter

dmgreen added a reviewer: simon_tatham. dmgreen added a comment. > I'm very out of my depth with tablegen, let me know if there's a more elegant > way of doing this I think this sounds OK. Providing all the tests pass it looks OK (but I'm not a big expert myself) Repository: rG LLVM Github

[PATCH] D121792: [AArch64][SVE] InstCombine llvm.aarch64.sve.sel to select

dmgreen added a comment. Why do we have llvm.aarch64.sve.sel if we are always going to replace it with a select? Why not remove llvm.aarch64.sve.sel entirely? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121792/new/

[PATCH] D120527: [OpaquePtr][AArch64] Use elementtype on ldxr/stxr

dmgreen added a comment. Seems OK. Thanks for the patch. Do opaque pointer variants (like `i32 @llvm.aarch64.stxr.p0(i64 1, ptr elementtype(i64) %ptr.0)`) get tested automatically from the existing tests once -opaque-pointers is the default? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D121206: [AARCH64] ssbs should be enabled by default for cortex-x1c

dmgreen added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64.td:978 FeatureNEON, FeatureRCPC, FeaturePerfMon, FeatureSPE, FeatureFullFP16, FeatureDotProd]; list X1C = [HasV8_2aOps,

[PATCH] D120876: [Driver] Split up huge arm-cortex-cpus.c test.

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Same as D120875 , if anyone wants to split this up more naturally that would be good, but until then this LGTM. Repository: rG LLVM Github Monorepo

  1   2   3   4   >