[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] 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 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] 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] 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] 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 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] 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] 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] 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: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/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] 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] 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] 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] 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] 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] 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 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. 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] 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 added a comment. Did a first scan, looks very reasonable, just some first nits/questions inlined. Comment at: clang/include/clang/Basic/arm_sve.td:153 +} +def ImmCheckPredicatePattern: ImmCheckType<0>; // 0..31 +def ImmCheck1_16:

[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] D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags

SjoerdMeijer added a comment. > Yes you're right, the patch that I've made dependent needs this one to work > properly. Ah ok, I may have missed that, will have a look Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76617/new/

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

SjoerdMeijer added a comment. Patches with functional changes but without tests are a bit "suspicious". In this case, I see it might be tricky. You could argue that it should be incorporated in the patch that includes the tests, so we can see what's happening. But perhaps separating this out

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

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

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

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

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

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

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

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

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

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

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

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

[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] D75861: [SVE] Generate overloaded functions for ACLE intrinsics.

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

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

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

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

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

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

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

[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

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

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] 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] 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] 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] 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. 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] 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] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

SjoerdMeijer added a comment. Many thanks! I've passed on the message, hopefully we can do something about this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61717/new/ https://reviews.llvm.org/D61717 ___

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

This revision was automatically updated to reflect the committed changes. Closed by commit rL372082: [Clang] Pragma vectorize_width() implies vectorize(enable) (authored by SjoerdMeijer, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

SjoerdMeijer updated this revision to Diff 220446. SjoerdMeijer added a comment. Just uploading new diff for completeness; I only had to change a test-case, and thus thought that committing this is okay. Many thanks again for reviewing and helping with the discussions! CHANGES SINCE LAST

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

SjoerdMeijer added a comment. I would have definitely preferred a revert much earlier, I guess it's too late now, but not having a build is really inconvenient. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67368/new/ https://reviews.llvm.org/D67368

[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] 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] D66796: [clang] Loop pragma vectorize(disable)

SjoerdMeijer added a comment. > Now on the practical side. If whatever we decide here changes how the pragma > behaves from today, we need to have a wider discussion and agreement at > llvm-dev. Yep, forgot about that, thanks for the suggestion. I've just posted this message to the list:

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

SjoerdMeijer added a comment. And thanks Florian for getting this discussion going again! Will definitely make this clear(er) in this description and commit message once we agree on it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66796/new/ https://reviews.llvm.org/D66796

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

SjoerdMeijer added a comment. Hi Hideki, I think you're comments are spot on: > It all depends on to whom we are providing these pragmas. Pragma's are user-facing "options" to override or force compiler decision making. I don't think there's another way to look at it, but please correct me

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

SjoerdMeijer added a comment. friendly ping. Shall we get this in, so that I can commit this and D66290 ? Then, we can perhaps continue the interleave discussion separately? I will then return to the doc patch first in D66199

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

SjoerdMeijer added a comment. > Therefore, vectorize(disable) would also disable interleaving? I don't have strong opinions on this. I think there's something to say for both options that we have (i.e. `vectorize(disable)` disables interleaving or enables it). But I think it was @fhahn who

[PATCH] D66199: [docs] loop pragmas

SjoerdMeijer added a comment. Many thanks for your clarification! > What we were discussing was that these settings would remove 0) from the > candidate list as well. Yep, that's crystal clear now. And my expectation would indeed be that this would be the case. CHANGES SINCE LAST ACTION

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

SjoerdMeijer updated this revision to Diff 217383. SjoerdMeijer edited the summary of this revision. SjoerdMeijer added a parent revision: D66796: [clang] Loop pragma vectorize(disable). SjoerdMeijer added a comment. Stripped out the functional change related to vectorize(disable). CHANGES

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

SjoerdMeijer added a comment. > I think it would be slightly better to split off the change to disable > vectorization via llvm.loop.vectorize.enable=false instead of width=1. This is now D66796 . I will now start stripping it out from this patch. CHANGES

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

SjoerdMeijer created this revision. SjoerdMeijer added reviewers: fhahn, Meinersbur, hsaito, Ayal. This changes the behaviour of vectorize(disable). I.e., disabling vectorization now means disabling vectorization, and not setting the vectorization width to 1. This is a follow up of the

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

SjoerdMeijer added a comment. Thanks, and sorry for the delay. Back in the office now, and I am addressing this: > I think it would be slightly better to split off the change to disable > vectorization via llvm.loop.vectorize.enable=false instead of width=1. Yep, I agree CHANGES SINCE LAST

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

SjoerdMeijer added a comment. Thanks for looking again! Good catch, feedback addressed. (forgot to add this message when I uploaded the new diff) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66290/new/ https://reviews.llvm.org/D66290 ___

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

SjoerdMeijer updated this revision to Diff 215388. SjoerdMeijer edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66290/new/ https://reviews.llvm.org/D66290 Files: clang/lib/CodeGen/CGLoopInfo.cpp clang/test/CodeGenCXX/pragma-loop-predicate.cpp

[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

SjoerdMeijer created this revision. SjoerdMeijer added reviewers: Meinersbur, fhahn, hsaito, dorit. Specifying the vectorization width was supposed to implicitly enable vectorization, except that it wasn't really doing this. It was only setting the `vectorize.width` metadata, but not

[PATCH] D64564: Loop pragma parsing. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rL368976: [clang] Loop pragma parsing. NFC. (authored by SjoerdMeijer, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64564: Loop pragma parsing. NFC.

SjoerdMeijer added a comment. Thanks, and will fix your suggestions before committing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64564/new/ https://reviews.llvm.org/D64564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL368970: [Clang] Pragma vectorize_predicate implies vectorize (authored by SjoerdMeijer, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

<    1   2   3   4   >