[PATCH] D60699: [ARM] add CLI support for 8.1-M and MVE.

2019-05-30 Thread Sjoerd Meijer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362096: [ARM] Add CLI support for Armv8.1-M and MVE (authored by SjoerdMeijer, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

2019-05-31 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 202388. SjoerdMeijer added a comment. This addresses @t.p.northover comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60697/new/ https://reviews.llvm.org/D60697 Files: clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/lib/Driver/ToolChai

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

2019-05-31 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer marked an inline comment as done. SjoerdMeijer added a comment. Ah yes, the school boy error! ;-) Actually, there was a test, but in a different patch; I will move it to here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60697/new/ https://reviews.llvm.org/D60697 _

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

2019-05-31 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 202433. SjoerdMeijer added a comment. This time with tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60697/new/ https://reviews.llvm.org/D60697 Files: clang/lib/Driver/ToolChains/Arch/ARM.cpp clang/lib/Driver/ToolChains/Arch/ARM.h c

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

2019-06-04 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 202934. SjoerdMeijer added a comment. Hi Oliver, thanks for your comments! This was the easy one, they have been added: > I also don't see any tests for the negated forms of either feature. The trouble begun with this: > +fp.dp, but the FPU is already

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

2019-06-05 Thread Sjoerd Meijer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362600: [ARM] Allow "-march=foo+fp" to vary with foo (authored by SjoerdMeijer, committed by ). Herald added a subscriber: kristina. Changed prior to commit: https://reviews.llvm.org/D60697?vs=202934&id

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 203507. SjoerdMeijer added a comment. Added tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60710/new/ https://reviews.llvm.org/D60710 Files: clang/lib/Basic/Targets/ARM.cpp clang/lib/Basic/Targets/ARM.h clang/test/Preprocessor/arm-t

[PATCH] D62998: [ARM] Fix bugs introduced by the fp64/d32 rework.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:103 std::vector &Features, + std::vector &FeaturesAfter, const llvm::Triple &Triple) {

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer marked an inline comment as done. SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:465 + MVE |= MVE_INT | MVE_FP; + HW_FP |= HW_FP_SP | HW_FP_HP; } ostannard wrote: > Does this also need to set FPU and Ha

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 203522. SjoerdMeijer added a comment. Set FullFP16 for +mve.fp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60710/new/ https://reviews.llvm.org/D60710 Files: clang/lib/Basic/Targets/ARM.cpp clang/lib/Basic/Targets/ARM.h clang/test/Prepr

[PATCH] D62998: [ARM] Fix bugs introduced by the fp64/d32 rework.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Cheers, LGTM too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62998/new/ https://reviews.llvm.org/D62998 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 203555. SjoerdMeijer added a comment. Yep, sorry, missed that one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60710/new/ https://reviews.llvm.org/D60710 Files: clang/lib/Basic/Targets/ARM.cpp clang/lib/Basic/Targets/ARM.h clang/test/P

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Thanks for reviewing! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60710/new/ https://reviews.llvm.org/D60710 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

2019-06-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362806: [ARM] Add ACLE feature macros for MVE (authored by SjoerdMeijer, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.l

[PATCH] D64564: Loop pragma parsing. NFC.

2019-07-11 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 209299. SjoerdMeijer marked an inline comment as not done. SjoerdMeijer added a comment. thanks for taking a look and the suggestions! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64564/new/ https://reviews.llvm.org/D64564 Files: lib/Parse/

[PATCH] D64564: Loop pragma parsing. NFC.

2019-07-11 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. oh no, the diff got messed up here. I.e. Phab shows 2 modified files, because the first patch set was created in a monorepo, and the 2nd patch set in a another and separate clang repo. Just for clarity, file `lib/Parse/ParsePragma.cpp` is the latest with the feedb

[PATCH] D64744: Loop #pragma tail_predicate

2019-07-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer created this revision. https://reviews.llvm.org/D64744 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Parse/Parser.h clang/lib/CodeGen/CGLoopInfo.cpp clang/lib/CodeGen/CGLoopInfo.h clang/lib/Parse/ParsePragma.cpp clang/lib/Sema/SemaStmtAttr.cpp clang/test/

[PATCH] D64744: Loop #pragma tail_predicate

2019-07-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Thanks for taking a look and your suggestions! I noticed your comment here after I replied to the list. As I wrote there, and long story short, I thought I could kill 2 birds with 1 stone, but that doesn't seem to be the case. I agree that for the vectorizer an opt

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

2019-07-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 https://re

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

2019-07-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
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)

2019-07-19 Thread Sjoerd Meijer via Phabricator via cfe-commits
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, th

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

2019-07-19 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 LAST

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

2019-07-22 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 https://review

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

2019-07-22 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 idea

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

2019-07-22 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 clang/include

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

2019-07-23 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 with

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

2019-07-25 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 https://lists.llvm.org/cgi-bin/mailman

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

2019-07-25 Thread Sjoerd Meijer via Phabricator via cfe-commits
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&id=211678#toc Repository: rL LLVM CH

[PATCH] D64564: Loop pragma parsing. NFC.

2019-07-30 Thread Sjoerd Meijer via Phabricator via cfe-commits
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) Me

[PATCH] D64564: Loop pragma parsing. NFC.

2019-08-01 Thread Sjoerd Meijer via Phabricator via cfe-commits
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

2019-08-05 Thread Sjoerd Meijer via Phabricator via cfe-commits
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)". https://reviews.llvm

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

2019-08-05 Thread Sjoerd Meijer via Phabricator via cfe-commits
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

2019-08-05 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 `getWidth(

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

2019-08-06 Thread Sjoerd Meijer via Phabricator via cfe-commits
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. I

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

2019-08-07 Thread Sjoerd Meijer via Phabricator via cfe-commits
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] D65776: [Clang] Pragma vectorize_predicate implies vectorize

2019-08-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. thanks @Meinersbur and @fhahn for the discussions on this, also on the dev list. It looks like we are happy with this behaviour? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65776/new/ https://reviews.llvm.org/D65776 __

[PATCH] D64564: Loop pragma parsing. NFC.

2019-08-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. this is not important at all, but might be nice clean up, so friendly ping :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64564/new/ https://reviews.llvm.org/D64564 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D64564: Loop pragma parsing. NFC.

2019-08-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Sorry, should have done my std::string, StringRef, and Twine homework a lot better! Thanks for your help and suggestions, will fix this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64564/new/ https://reviews.llvm.org/D64564 __

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

2019-08-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Thanks for your help! And I will wait a few days. After this, I will look at that PR, will have a look at diagnostics, and then at the LLVM side of things. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65776/new/ https://reviews.llvm.org/D65776 ___

[PATCH] D66199: [docs] loop pragmas

2019-08-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer created this revision. SjoerdMeijer added reviewers: fhahn, Meinersbur, dorit, hsaito. Following our discussion on the cfe dev list (http://lists.llvm.org/pipermail/cfe-dev/2019-August/063054.html), I have added a paragraph that is explicit about transformation options implying the co

[PATCH] D64564: Loop pragma parsing. NFC.

2019-08-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 215162. SjoerdMeijer added a comment. thanks for the suggestions; comments addressed. 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/Pa

[PATCH] D66199: [docs] loop pragmas

2019-08-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer marked an inline comment as done. SjoerdMeijer added a comment. > Since this is user documentation, we should only add it here once it is true. Yep, good point, I also wanted to capture our discussions on the list. But yes, let's hold this back until we're ready with the pragmas.

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

2019-08-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
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: ht

[PATCH] D64564: Loop pragma parsing. NFC.

2019-08-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 https://

[PATCH] D64564: Loop pragma parsing. NFC.

2019-08-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
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: https://reviews.llvm.

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

2019-08-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 `vectorize.en

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

2019-08-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 clan

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

2019-08-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
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] D63936: [ARM] Minor fixes in command line option parsing

2019-07-01 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. I will let Oliver finish the review (I am off for a few days), just some drive-by comments. Comment at: llvm/lib/Support/ARMTargetParser.cpp:412 - if (Extensions & AEK_CRC) -Features.push_back("+crc"); - else -Features.push_back("-crc"

[PATCH] D64471: Loop pragma parsing. NFC.

2019-07-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
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: clang/lib/Sema/

[PATCH] D64471: Loop pragma parsing. NFC.

2019-07-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL365629: Loop pragma parsing. NFC. (authored by SjoerdMeijer, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D644

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:237 +break; + case llvm::AArch64::ArchKind::ARMV8_4A: +getTargetDefinesARMV84A(Opts, Builder); It is a good change, but I think you should either add tests for these case

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:237 +break; + case llvm::AArch64::ArchKind::ARMV8_4A: +getTargetDefinesARMV84A(Opts, Builder); SjoerdMeijer wrote: > It is a good change, but I think you should either ad

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:720 def FJCVTZS : BaseFPToIntegerUnscaled<0b01, 0b11, 0b110, FPR64, GPR32, - "fjcvtzs", []> { + "fjcvtzs", [(set

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks like a good change to me, some nits inlined. It shouldn't be difficult to request an account and commit it yourself, which might be useful if you maybe intend to submit more

[PATCH] D64564: Loop pragma parsing. NFC.

2019-07-11 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer created this revision. SjoerdMeijer added reviewers: samparker, dmgreen, Meinersbur. Yesterday I was a bit distracted by loop pragma parsing (D64471 ), but still am a bit today too (this patch). https://reviews.llvm.org/D64564 Files: clang/lib/Pa

[PATCH] D64564: Loop pragma parsing. NFC.

2019-07-11 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer marked an inline comment as done. SjoerdMeijer added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:1015 + .Default(""); + assert(Str.size() && "Unexpected pragma name"); + return Str; I guess this does behave slightly d

[PATCH] D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32.

2019-05-28 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. There already was consensus that this is was a good change, and also that we don't care about auto-upgrading. With the last minor comments addressed, this looks good I think. Perha

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

2019-09-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
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: http

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

2019-09-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
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.

2019-09-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
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/ https://reviews.llvm.org/D6171

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

2019-09-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
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] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

2019-09-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 ACTIO

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

2019-09-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 commit

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

2019-09-18 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 ___ cfe-commi

[PATCH] D45544: [AAch64] Add the __ARM_FEATURE_DOTPROD macro definition

2018-04-12 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Hi, thanks for adding this. Also wanted to confirm that macro __ARM_FEATURE_DOTPROD will defined/included in the next release of the ACLE. Comment at: lib/Basic/Targets/AArch64.h:36 unsigned HasFullFP16; + unsigned HasDotProd; llvm::AArch6

[PATCH] D45544: [AAch64] Add the __ARM_FEATURE_DOTPROD macro definition

2018-04-13 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. LGTM, thanks. https://reviews.llvm.org/D45544 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Not really familiar with these 2 intrinsics, I had a quick look at the ACLE: > T vget_high_ST(T 2 a); > T vget_low_ST(T 2 a); > > Gets the high, or low, half of a 128-bit vector. There are 24 intrinsics. > ARMv8 > adds 4 more intrinsics for 128-bit vectors with f

[PATCH] D45669: [NEON] Fix the architecture condition for the crypto intrinsics

2018-04-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. This looks correct to me, the ACLE indeed says that: > This is only available when __ARM_ARCH >= 8 Thanks for fixing this. https://reviews.llvm.org/D45669 ___

[PATCH] D45670: [NEON} Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode

2018-04-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments. Comment at: test/CodeGen/arm-neon-fma.c:27 +// CHECK: [[VECINIT1_I:%.*]] = insertelement <2 x float> [[VECINIT_I]], float %n, i32 1 +// CHECK: [[TMP0:%.*]] = bitcast <2 x float> %a to <8 x i8> +// CHECK: [[TMP1:%.*]] = bitcast <2 x float

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Yep, agreed, also on the new shiny https://developer.arm.com/technologies/neon/intrinsics it is listed as A64 only. https://reviews.llvm.org/D45668

[PATCH] D45670: [NEON] Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode

2018-04-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. The corresponding LLVM tests seem be there already, so this looks all good to me. https://reviews.llvm.org/D45670 ___ cfe-commits ma

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-18 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Sorry, I have second thoughts on this. This seems more like a doc issue than anything else. There is no reason why this could not be supported in A32. GCC is also supporting this, and removing it is a bit user unfriendly. Would you mind reverting this? Repository:

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-18 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Thanks, and I am going to try to get some clarity on this doc issue. But looks like it should be "ARMv7, ARMv8", as it used to be. Make sense to comment on this in the commit message, if that's what you mean. Repository: rL LLVM https://reviews.llvm.org/D45668

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-18 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. Thanks James! Repository: rL LLVM https://reviews.llvm.org/D45668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46108: [ARM] Add __ARM_FEATURE_DOTPROD pre-defined macro

2018-04-26 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks like a straight forward fix/addition to me. Comment at: lib/Basic/Targets/ARM.cpp:737 + if (DotProd) +Builder.defineMacro("__ARM_FEATURE_DOTPROD", "1

[PATCH] D46109: [ARM, AArch64] Add intrinsics for dot product instructions

2018-04-26 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. I think this looks OK. Comment at: include/clang/Basic/arm_neon.td:1587 +// v8.2-A dot product instructions +let ArchGuard = "defined(__ARM_FEATURE_DOTPROD)" in {

[PATCH] D48440: [NEON] Support vldNq intrinsics in AArch32 (Clang part)

2018-06-27 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks OK to me. But file "test/CodeGen/vld_dup.c" looks weird/empty. I guess you're removing it? https://reviews.llvm.org/D48440 ___

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

2019-10-09 Thread Sjoerd Meijer via Phabricator via cfe-commits
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/ https://review

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

2019-10-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
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

2019-10-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
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/ https://reviews.llvm.org/D6

[PATCH] D66199: [docs] loop pragmas

2019-10-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
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

2019-10-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 https://lists.llvm

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

2019-10-11 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 f

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

2019-10-11 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:902 + std::vector &Features = getTargetOpts().Features; + std::string SearchFeature = "+reserve-" + RegName.str(); + for (std::string &Feature : Features) { chill wrote: > SjoerdMe

[PATCH] D66199: [docs] loop pragmas

2019-10-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 https://re

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

2019-10-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
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] D79708: [clang][BFloat] add NEON emitter for bfloat

2020-05-27 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. No objections. Some nits inlined, which you can ignore if you think they are not correct. Comment at: clang/include/clang/Basic/arm_neon_incl.td:218 // d: double +// b: bfloat // nit: perhaps bfloat16? Comment

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

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

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

2020-05-27 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/arm-mangle-bf16.cpp:3 +// RUN: %clang_cc1 -triple arm-arm-none-eabi -target-feature +bf16 -mfloat-abi hard -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK32-HARD +// RUN: %clang_cc1 -triple arm-arm-none-ea

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

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

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

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

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

2020-06-01 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment. > At the moment when going through the GCC compatibility driver (standard > interface), we get __bf16 is not supported on this target. If this is the current behaviour, and consistent with GCC, that sounds reasonable. Probably best to be explicit about this and doc

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

2020-07-22 Thread Sjoerd Meijer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5567c62afa55: [Matrix] Add LowerMatrixIntrinsics to the NPM (authored by SjoerdMeijer). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.ll

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

2020-07-28 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Cheers, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84703/new/ https://reviews.llvm.org/D84703 _

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

2020-04-06 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 mailin

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

2020-04-06 Thread Sjoerd Meijer via Phabricator via cfe-commits
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] D76612: [Matrix] Add draft specification for matrix support in Clang.

2020-04-09 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 move

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

2020-04-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
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.

2020-04-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
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 half-

<    1   2   3   4   >