[PATCH] D66796: [clang] Loop pragma vectorize(disable)

SjoerdMeijer abandoned this revision. SjoerdMeijer added a comment. Every day is a school day, and Hal taught me something ;-) As I wrote in the thread on the llvm dev list, with Hal's explanations, I don't think I have a case for this anymore, and so will abandon it (please let me know if you

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

SjoerdMeijer added a comment. Yeah, sorry about that Do you perhaps have a test case or error that I can look at? Perhaps I or someone else here can help out a bit here. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61717/new/

[PATCH] D64564: Loop pragma parsing. NFC.

SjoerdMeijer marked an inline comment as done. SjoerdMeijer added inline comments. Comment at: lib/Parse/ParsePragma.cpp:1011 + Str = llvm::StringSwitch(Str) + .Case("loop", "clang loop " + Str.str()) + .Case("unroll_and_jam", Str)

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

SjoerdMeijer added a comment. > I do not know whether/how "setting a transformation option implicitly enables > the transformation" should be implemented, maybe we should discuss this. Yep, agreed. I've sent a mail to the dev list: http://lists.llvm.org/pipermail/cfe-dev/2019-August/063054.html

[PATCH] D64564: Loop pragma parsing. NFC.

SjoerdMeijer updated this revision to Diff 212878. SjoerdMeijer added a comment. Fixed the string problems. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64564/new/ https://reviews.llvm.org/D64564 Files: clang/lib/Parse/ParsePragma.cpp Index: clang/lib/Parse/ParsePragma.cpp

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

SjoerdMeijer created this revision. SjoerdMeijer added reviewers: Meinersbur, hsaito, fhahn. New pragma "vectorize_predicate(enable)" now implies "vectorize(enable)", and it is ignored when vectorization is disabled with e.g. "vectorize(disable) vectorize_predicate(enable)".

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

SjoerdMeijer added a comment. Forgot that this requires a doc change too. Will add that once we're happy with the proposed behaviour. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65776/new/ https://reviews.llvm.org/D65776 ___ cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

SjoerdMeijer added a comment. Hi Florian, thanks for your input! > IMO it would make sense to have the more specific pragmas imply > vectorize(enable) here (or update the docs accordingly). Yep, fully agree with that, as I also wrote in my previous comment. And thanks for digging up that PR.

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

SjoerdMeijer added a comment. Thanks for pointing this all out! I am not entirely sure yet what to think about all this as I am new to the loop pragma business, but I think it looks inconsistent to me! I think I find `allowReordering()` a little bit ugly, because it is also checking

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

SjoerdMeijer updated this revision to Diff 210774. SjoerdMeijer retitled this revision from " #pragma clang loop predicate(enable|disable)" to " #pragma clang loop vectorize_predicate(enable|disable)". SjoerdMeijer edited the summary of this revision. SjoerdMeijer added a comment. Hi Michael,

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

SjoerdMeijer updated this revision to Diff 210857. SjoerdMeijer added a comment. Removed the separate function that created the loop.llvm.vectorize.predicate metadata. This is now just part of function `createLoopVectorizeMetadata`, that creates all other vectorize metadata. CHANGES SINCE

[PATCH] D64744: #pragma clang loop predicate(enable|disable)

SjoerdMeijer updated this revision to Diff 210314. SjoerdMeijer retitled this revision from "Loop #pragma tail_predicate" to " #pragma clang loop predicate(enable|disable)". SjoerdMeijer edited the summary of this revision. Herald added a subscriber: zzheng. CHANGES SINCE LAST ACTION

[PATCH] D64744: #pragma clang loop predicate(enable|disable)

SjoerdMeijer added a comment. So I went for: predicate(enable) as I think that is most general, best capturing it, but I am of course completely open to bikeshedding names :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64744/new/ https://reviews.llvm.org/D64744

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

SjoerdMeijer added a comment. Apologies for the early ping! Bu I'm off next weeks, so it would be nice to get this in before that if there are no further comments. Tomorrow, I will upload another diff that builds on top D64916 , which enables code-generation

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

SjoerdMeijer added a comment. Many thanks for reviewing! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64744/new/ https://reviews.llvm.org/D64744 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

This revision was automatically updated to reflect the committed changes. Closed by commit rL366989: [Clang] New loop pragma vectorize_predicate (authored by SjoerdMeijer, committed by ). Changed prior to commit: https://reviews.llvm.org/D64744?vs=211096=211678#toc Repository: rL LLVM

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

SjoerdMeijer updated this revision to Diff 211042. SjoerdMeijer added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - Moved the codegen test to a separate file - Added a langref description for this new metadata node. CHANGES SINCE LAST ACTION

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

SjoerdMeijer added a comment. > Is it intentional that this review has no reviewers listed (like, is this a > work in progress you don't expect review on yet)? No, sorry about this, that's not intentional. It started indeed as a work-in-progress patch when I wrote to the clang/llvm with an

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

SjoerdMeijer updated this revision to Diff 211096. SjoerdMeijer added a comment. More doc changes added to `AttrDocs.td` and `LangRef.rst` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64744/new/ https://reviews.llvm.org/D64744 Files: clang/docs/LanguageExtensions.rst

[PATCH] D64471: Loop pragma parsing. NFC.

SjoerdMeijer created this revision. SjoerdMeijer added reviewers: Meinersbur, dmgreen, samparker. I would like to add some pragma handling here, but couldn't resist a little NFC and tidy up first. https://reviews.llvm.org/D64471 Files: clang/lib/Sema/SemaStmtAttr.cpp Index:

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

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

[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

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

[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

[PATCH] D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Yep, thanks again! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68838/new/ https://reviews.llvm.org/D68838

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

[PATCH] D66199: [docs] loop pragmas

SjoerdMeijer added a comment. I have commit all my pragma patches, so now back to the last bit, this doc update. This doc change should now reflect our implementation. Are we happy for this to go in? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66199/new/

[PATCH] D68743: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks again, looks good. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68743/new/ https://reviews.llvm.org/D68743

[PATCH] D66199: [docs] loop pragmas

SjoerdMeijer updated this revision to Diff 224421. SjoerdMeijer added a comment. Thanks! Typo fixed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66199/new/ https://reviews.llvm.org/D66199 Files: clang/docs/LanguageExtensions.rst Index: clang/docs/LanguageExtensions.rst

[PATCH] D66199: [docs] loop pragmas

SjoerdMeijer added a comment. Sure, will do, thanks again for taking a look. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66199/new/ https://reviews.llvm.org/D66199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68683: ARM] Fix arm_neon.h with -flax-vector-conversions=none

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Nice one, thanks for fixing! I didn't have the bandwidth to look into this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68683/new/

[PATCH] D66199: [docs] loop pragmas

This revision was automatically updated to reflect the committed changes. Closed by commit rG52bfa73af841: [docs] loop pragmas: options implying transformations (authored by SjoerdMeijer). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68862: [ARM] Allocatable Global Register Variables for ARM

SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:902 + std::vector = getTargetOpts().Features; + std::string SearchFeature = "+reserve-" + RegName.str(); + for (std::string : Features) { chill wrote: > SjoerdMeijer wrote: > >

[PATCH] D68862: [ARM] Allocatable Global Register Variables for ARM

SjoerdMeijer added a comment. Bit of a drive-by comment, but I can't say I am big fan of all the string matching on the register names. Not sure if this is a fair comment, because I haven't looked closely at it yet, but could we use more the `ARM::R[0-9]` values more? Perhaps that's difficult

[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

[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

[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

[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

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

[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

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

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

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

[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

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

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

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 ___

[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: [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 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=247822#toc

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

[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

[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] D57054: [MachineOutliner][ARM][RFC] Add Machine Outliner support for ARM

SjoerdMeijer added a subscriber: samparker. SjoerdMeijer added a comment. Just a quick message, linking in @samparker, and I guess moving Low Overhead Loops to run before ConstantIslands could be problematic, but we can/should have a proper look tomorrow. Repository: rG LLVM Github Monorepo

[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

[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

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

[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] D76612: [Matrix] Add draft specification for matrix support in Clang.

SjoerdMeijer added inline comments. Comment at: clang/docs/MatrixTypes.rst:27 +internal layout, overall size and alignment are implementation-defined. +A *matrix element type* must be a real type (as in C99 6.2.5p17) excluding +enumeration types or an implementation-defined

[PATCH] D76612: [Matrix] Add draft specification for matrix support in Clang.

SjoerdMeijer added inline comments. Comment at: clang/docs/MatrixTypes.rst:12 +fixed-size matrices as language values and perform arithmetic on them. + +This feature is currently experimental, and both its design and its Would it be good to set expectations here

[PATCH] D76612: [Matrix] Add draft specification for matrix support in Clang.

SjoerdMeijer added inline comments. Comment at: clang/docs/MatrixTypes.rst:27 +internal layout, overall size and alignment are implementation-defined. +A *matrix element type* must be a real type (as in C99 6.2.5p17) excluding +enumeration types or an implementation-defined

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

SjoerdMeijer created this revision. SjoerdMeijer added a reviewer: dblaikie. This is a doc change documenting that option -mtune does not perform any CPU type specific tuning but exists for compatability reasons with GCC. https://reviews.llvm.org/D78511 Files:

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

SjoerdMeijer updated this revision to Diff 258823. SjoerdMeijer added a comment. Cheers, that's probably what I wanted to say. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78511/new/ https://reviews.llvm.org/D78511 Files: clang/docs/ClangCommandLineReference.rst

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

SjoerdMeijer added a comment. Thank you both for reviewing! And I will wait a day. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78511/new/ https://reviews.llvm.org/D78511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

SjoerdMeijer created this revision. SjoerdMeijer added reviewers: psmith, ostannard, kristof.beyls, chill. Herald added subscribers: danielkiss, arphaman. Following the discussion about -mtune on the cfe dev list, I thought it would be good to make a start with documenting common command line

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

SjoerdMeijer added a comment. Hi Peter, Thanks for reviewing again! I thought these examples to be relevant here, because it shows usage and examples of this tool (i.e. open-source clang/llvm). Additional benefits is that source and documentation is in one place, and it allows others, non-Arm

[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

SjoerdMeijer accepted this revision. SjoerdMeijer added a reviewer: efriedma. SjoerdMeijer added a comment. This revision is now accepted and ready to land. This is a big patch, but looks reasonable to me. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7451 + } +

[PATCH] D77594: [SveEmitter] Add support for _n form builtins

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks reasonable to me Comment at: clang/utils/TableGen/SveEmitter.cpp:212 + bool hasSplat() const { +return Proto.find_first_of("ajfrKLR") !=

[PATCH] D78401: [SveEmitter] IsInsertOp1SVALL and builtins for svqdec[bhwd] and svqinc[bhwd]

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks reasonable Comment at: clang/include/clang/Basic/arm_sve.td:530 +class sat_type { string U = u; string T = t; } +def SIGNED_BYTE : sat_type<"",

[PATCH] D76612: [Matrix] Add draft specification for matrix support in Clang.

SjoerdMeijer added inline comments. Comment at: clang/docs/MatrixSupport.rst:254 + +Example +=== Hi Florian, just reading this for the first time, this is cool stuff, and just a drive-by comment: this section, Example, looks like a good candidate to be

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

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Cheers, LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76617/new/ https://reviews.llvm.org/D76617 ___ cfe-commits

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

[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

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

[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

[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

[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] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM, please wait a day in case Eli has more comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76678/new/ https://reviews.llvm.org/D76678

[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] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rG35cf2f42dda4: [Driver][docs] Document option -mtune as a no-op. (authored by SjoerdMeijer). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

SjoerdMeijer abandoned this revision. SjoerdMeijer added a comment. Fair enough, perhaps the audience is too small here on llvm.org for this and this is too niche. In A-profile we have the same problem, so could the exercise for an A-core here, but can't spend time on that now, so will abandon

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

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

<    1   2   3   4   >