[PATCH] D26858: [AArch64] Don't constrain the assembler when using -mgeneral-regs-only

jmolloy accepted this revision. jmolloy added a comment. This revision is now accepted and ready to land. It's a bit awkward that you need to copy the LastOpt loop from below, but I can't see a simpler way to do this. LGTM. https://reviews.llvm.org/D26858

[PATCH] D26606: Protect tests for std::uninitialized_{copy, fill} under libcpp-no-exceptions

jmolloy added a comment. Hi, This looks fairly obviously correct to me, but perhaps someone more familiar with libcxx should look too: Asiri? James https://reviews.llvm.org/D26606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D25761: Use linker flag --fix-cortex-a53-843419 on Android ARM64 compilation.

jmolloy accepted this revision. jmolloy added a reviewer: jmolloy. jmolloy added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D25761 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D25387: When optimizing for size, enable loop rerolling by default.

jmolloy accepted this revision. jmolloy added a comment. This revision is now accepted and ready to land. Good idea! LGTM! https://reviews.llvm.org/D25387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D25210: [ARM] Add Cortex-R52 target to CLANG

jmolloy added a comment. Still LGTM. https://reviews.llvm.org/D25210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25210: [ARM] Add Cortex-R52 target to CLANG

jmolloy accepted this revision. jmolloy added a comment. This revision is now accepted and ready to land. LGTM, thanks Javed! https://reviews.llvm.org/D25210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: r280613 - [Modules] Add 'freestanding' to the 'requires-declaration' feature-list.

Hi Elad, This commit broke all buildbots that don't default to targetting x86. I've committed a fix with r280658, but could you please double check it's correct. Cheers, James On Sun, 4 Sep 2016 at 07:09 Elad Cohen via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: eladcohen >

r280658 - Attempt to fix buildbots not targetting x86

Author: jamesm Date: Mon Sep 5 07:28:49 2016 New Revision: 280658 URL: http://llvm.org/viewvc/llvm-project?rev=280658=rev Log: Attempt to fix buildbots not targetting x86 r280613 introduced failures for all builds that don't target x86 by default. Add an explicit target to avoid a missing

r280220 - Attempt to pacify buildbots after r280217

Author: jamesm Date: Wed Aug 31 06:01:41 2016 New Revision: 280220 URL: http://llvm.org/viewvc/llvm-project?rev=280220=rev Log: Attempt to pacify buildbots after r280217 These clang tests check diagnostics from the backend by giving it an unvectorizable loop. This loop is now vectorized :/

Re: [PATCH] D23840: New options -fexceptions-fp-math and -fdenormal-fp-math

jmolloy accepted this revision. jmolloy added a comment. This revision is now accepted and ready to land. LGTM. Cheers, James https://reviews.llvm.org/D23840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D23840: New options -fexceptions-fp-math and -fdenormal-fp-math

jmolloy added a comment. Hi Sjoerd, This still needs a docs patch. I'd also really appreciate someone else to help sign this off - While it looks fine, I don't commit enough to clang to give the go-ahead. Cheers, James https://reviews.llvm.org/D23840

Nomination for 3.9: r278786 - Left shifts of negative values are defined if -fwrapv is set

Hi Hans, [cc. Richard as code owner] I'd like to nominate this commit for 3.9. The actual code churn is tiny, and this fixes a problem noticed by and causing pain to the qemu project. Cheers, James On Tue, 16 Aug 2016 at 10:53 James Molloy via cfe-commits < cfe-commits@lists.llvm.org>

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

jmolloy accepted this revision. jmolloy added a reviewer: jmolloy. jmolloy added a comment. This revision is now accepted and ready to land. Thanks, r278786! https://reviews.llvm.org/D23498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

r278786 - Left shifts of negative values are defined if -fwrapv is set

Author: jamesm Date: Tue Aug 16 04:45:36 2016 New Revision: 278786 URL: http://llvm.org/viewvc/llvm-project?rev=278786=rev Log: Left shifts of negative values are defined if -fwrapv is set This means we shouldn't emit ubsan detection code or warn. Fixes PR25552. Added:

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

jmolloy added a comment. Hi Filipe, Did this look good to you after those changes? Cheers, James https://reviews.llvm.org/D23498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

jmolloy updated this revision to Diff 68033. jmolloy added a comment. Hi Filipe, Thanks for the review! SGTM on both counts. https://reviews.llvm.org/D23498 Files: lib/CodeGen/CGExprScalar.cpp lib/Sema/SemaExpr.cpp test/CodeGen/wrapv-lshr-sanitize.c test/Sema/negative-shift-wrapv.c

[PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

jmolloy created this revision. jmolloy added reviewers: davide, aaron.ballman. jmolloy added a subscriber: cfe-commits. This means we shouldn't emit ubsan detection code or warn. Fixes PR25552. https://reviews.llvm.org/D23498 Files: lib/CodeGen/CGExprScalar.cpp lib/Sema/SemaExpr.cpp

Re: [PATCH] D21295: Add a negative TBAA test

jmolloy added a comment. Private reply: fine by me but a clang person needs to a accept it. http://reviews.llvm.org/D21295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21295: Add a negative TBAA test

jmolloy added a subscriber: jmolloy. Comment at: test/CodeGen/tbaa.cpp:57 @@ +56,3 @@ +// NO-TBAA-LABEL: define i32 @_Z1g +// NO-TBAA: store i32 1, i32* %{{.*}}, align 4{{ *$}} +// NO-TBAA: store i32 4, i32* %{{.*}}, align 4{{ *$}} Would it just be simpler to do:

Re: [PATCH] D20089: Adding a TargetParser for AArch64

jmolloy added a comment. As far as I'm concerned, if you're happy I'm happy. You know this area more than me. Repository: rL LLVM http://reviews.llvm.org/D20089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20325: Add ARM cdp intrinsics

Hi, To add my oar in, I agree with Tim here. It is regrettable but true that documentation, be that the ARMARM or ACLE tends to lag behind our development. If LLVM wants to be at the leading edge of architecture support (I hope it does!) then patches will just have to be accepted without pointers

Re: [PATCH] D16929: add support for -print-imm-hex for AArch64

jmolloy accepted this revision. jmolloy added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rL LLVM http://reviews.llvm.org/D16929 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D16929: add support for -print-imm-hex for AArch64

jmolloy added a comment. Hi Pawel, Thanks for this - it generally looks good to me. Just one comment. James Comment at: lib/Target/AArch64/InstPrinter/AArch64InstPrinter.h:107 @@ -106,3 @@ - template - void printAMIndexedWB(const MCInst *MI, unsigned OpNum, -

Re: [PATCH] D16056: [Gold] Pass -mllvm options to the gold plugin

jmolloy abandoned this revision. jmolloy added a comment. Abandoning - this isn't as clear-cut as I thought. Repository: rL LLVM http://reviews.llvm.org/D16056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D16056: [Gold] Pass -mllvm options to the gold plugin

Ok, I'll abandon this. It wasn't meant to be contentious and this is a reasonable objection! James > On 12 Jan 2016, at 00:53, Mehdi Amini wrote: > > I’d fear the same thing. On our platform you have to use explicitly > -Wl,-mllvm (or -Xlinker -plugin-opt). > > — >

Re: [PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

jmolloy accepted this revision. jmolloy added a reviewer: jmolloy. jmolloy added a comment. This revision is now accepted and ready to land. Hi, As we got to the bottom of why this is actually needed, I committed this with the changes suggested by Rafael in r256146. Cheers, James

r256146 - [Driver] Pass -O* to the gold plugin via -plugin-opt

Author: jamesm Date: Mon Dec 21 04:44:36 2015 New Revision: 256146 URL: http://llvm.org/viewvc/llvm-project?rev=256146=rev Log: [Driver] Pass -O* to the gold plugin via -plugin-opt The gold plugin understands -O0..-O3, but these are not currently being passed to it. Modified:

Re: [PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

It's interesting to think about, but not something I'm intending to attack right now. I mainly just wanted to know the answer to Joerg's question, and now I do :) James On Sat, 19 Dec 2015 at 17:17, Mehdi Amini via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > > Sent from my iPhone > > >

Re: [PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

Hi Mehdi, > On 18 Dec 2015, at 23:17, Mehdi Amini wrote: > > The alternative to the command line flag is to encode the optimization level > in the bitcode itself. You may have answered Joerg's question here - I take it this is not yet implemented? James IMPORTANT

[PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

jmolloy created this revision. jmolloy added a reviewer: joerg. jmolloy added a subscriber: cfe-commits. jmolloy set the repository for this revision to rL LLVM. Herald added a subscriber: joker.eph. The gold plugin understands -O0..-O3, but these are not currently being passed to it.

Re: [PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

Hi Joerg, This is about the standard generic optimizer flags. Currently when using -flto, the backend behaves always as if "-O2" were passed (because this is the default codegen optimization level inside the gold plugin and it's never overridden!). So CodeGenOpt::Aggressive is never picked, and

Re: [PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

jmolloy added a comment. Hi Rafael, Thanks for the review! > This introduces a meaning to -ON during the link. That normally show up by > people passing CFLAGS when linking. Yes. The rationale is that with -flto, the link is also part of the compile. I think it's more surprising that the

Re: [PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

Evidently not, at least not completely. I see codegen differences with this patch (and at least one significant improvement). James On Fri, 18 Dec 2015 at 16:31, Joerg Sonnenberger via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Fri, Dec 18, 2015 at 03:59:03PM +0000, James M

Re: [PATCH] D15641: [Driver] Pass -O* to the gold plugin via -plugin-opt

< cfe-commits@lists.llvm.org> wrote: > On Fri, Dec 18, 2015 at 05:31:45PM +0000, James Molloy via cfe-commits > wrote: > > Evidently not, at least not completely. I see codegen differences with > this > > patch (and at least one significant improvement). > > I don't m

Re: [PATCH] D15223: [ARM] [AARCH64] Add CodeGen IR tests for {VS}QRDML{AS}H v8.1a intrinsics.

Fwiw, I am certainly in Tim'a camp here! Writing a test for that output is doable, and if that's what people want then that's what we'll do. But it's certainly not nice or readable ! On Mon, 14 Dec 2015 at 19:25, Tim Northover via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On 14 December

Re: [PATCH] D15223: [ARM] [AARCH64] Add CodeGen IR tests for {VS}QRDML{AS}H v8.1a intrinsics.

jmolloy resigned from this revision. jmolloy removed a reviewer: jmolloy. jmolloy added a comment. Eric is reviewing this; resigning myself. http://reviews.llvm.org/D15223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

Hi Eric, This isn't just a NEON intrinsics thing, and this isn't just an ARM/AArch64 thing. There needs to be some way to test the compiler from start to finish. Not being able to do so leaves serious coverage holes. Unit testing is great, but integration testing is required sometimes to ensure

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

yet be allowed when they add value? James On Tue, 1 Dec 2015 at 18:29, David Blaikie <dblai...@gmail.com> wrote: > On Tue, Dec 1, 2015 at 9:56 AM, Renato Golin via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> On 1 December 2015 at 17:23, James Molloy v

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

hri...@gmail.com> wrote: > On Tue, Dec 1, 2015 at 10:43 AM James Molloy via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Hi, >> >> > That would sort of defeat the point of having the testing and projects >> separated though - it would tie the tests

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

Hi, FWIW, I'm happy with moving these tests into the test-suite. They do make sense to be expressed in a compiler-neutral manner, even though some may test clang-specific behaviour. There is a UnitTests directory already, and adding the ability to run LIT tests should be simple within the new

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

Hi Renato, > by a non-trivial amount. Why do you think it would be non trivial? Some simple lit tests aren't exactly arduous on most targets. James On Tue, 1 Dec 2015 at 19:39, Renato Golin wrote: > On 1 December 2015 at 19:09, David Blaikie

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

Ok, understood. I think I'd be OK with demoting these tests to the test-suite, and dealing with the slightly lower amount of testing that comes with it if it means we can keep the clang tests in a nice shape. We'd need to ensure there were equivalent tests written that test only clang produced

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

Hi Eric, While I agree with you in principle, Alexandros has just pointed out to me that all the other NEON intrinsics have such -O3 tests, and thinking about it I do think they add value. They test the full-trip through the compiler and ensure that Clang and LLVM have matching ideas of the IR

Re: [PATCH] D14570: Handle ARMv6KZ naming

Hi Renato, Ideally, shouldn't the clang tests be checking that the LLVM target parsing library is called with the correct arguments? then separate tests inside LLVM check that the target parser works correctly? As it stands, it seems like a very deliberate layering violation that could really do

r252872 - Update clang regression tests for 'norecurse'

Author: jamesm Date: Thu Nov 12 04:56:51 2015 New Revision: 252872 URL: http://llvm.org/viewvc/llvm-project?rev=252872=rev Log: Update clang regression tests for 'norecurse' FunctionAttrs has just been taught how to infer 'norecurse'. Update clang tests for LLVM r252871. Modified:

Re: [PATCH] D14615: [C++] Add the "norecurse" attribute to main() if in C++ mode

jmolloy closed this revision. jmolloy added a comment. Thanks Aaron, committed in r252902. Repository: rL LLVM http://reviews.llvm.org/D14615 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D14615: [C++] Add the "norecurse" attribute to main() if in C++ mode

jmolloy updated this revision to Diff 40050. jmolloy added a comment. Hi Aaron, Thanks, this should all be fixed now. Cheers, James Repository: rL LLVM http://reviews.llvm.org/D14615 Files: lib/CodeGen/CodeGenFunction.cpp test/CodeGenCXX/main-norecurse.cpp

Re: [PATCH] D14615: [C++] Add the "norecurse" attribute to main() if in C++ mode

jmolloy added inline comments. Comment at: test/CodeGenCXX/main-norecurse.cpp:1 @@ +1,2 @@ +// RUN: %clang_cc1 -emit-llvm %s -o - + Wow, I managed to forget in my copy-paste from another test to add "| FileCheck %s" here. I'll update this before committing.

[PATCH] D14615: [C++] Add the "norecurse" attribute to main() if in C++ mode

jmolloy created this revision. jmolloy added a reviewer: aaron.ballman. jmolloy added a subscriber: cfe-commits. jmolloy set the repository for this revision to rL LLVM. The C++ spec (3.6.1.3) says "The function `main` shall not be used within a program". This implies that it cannot recurse, so

r252902 - [C++] Add the "norecurse" attribute to main() if in C++ mode

Author: jamesm Date: Thu Nov 12 09:36:04 2015 New Revision: 252902 URL: http://llvm.org/viewvc/llvm-project?rev=252902=rev Log: [C++] Add the "norecurse" attribute to main() if in C++ mode The C++ spec (3.6.1.3) says "The function `main` shall not be used within a program". This implies that it

Re: [PATCH] D14570: Handle ARMv6KZ naming

Hi Artyom, Why isn't this just part of D14568? Cheers, James On Wed, 11 Nov 2015 at 12:08 A. Skrobov via cfe-commits < cfe-commits@lists.llvm.org> wrote: > tyomitch created this revision. > tyomitch added reviewers: rengolin, joerg, bogden. > tyomitch added a subscriber: cfe-commits. > Herald

Re: r248154 - [ARM] Handle +t2dsp feature as an ArchExtKind in ARMTargetParser.def

Hi Artyom, I have reverted this in r248173 because the pre-commmit review was not completed. Cheers, James On Mon, 21 Sep 2015 at 06:20 Artyom Skrobov via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: askrobov > Date: Mon Sep 21 08:19:25 2015 > New Revision: 248154 > > URL:

Re: [PATCH] D12938: [ARM] Handle +t2dsp feature as an ArchExtKind in ARMTargetParser.def

Hi Artyom, This, too, looks to be unreviewed. Could you please explain what's going on here? James On Mon, 21 Sep 2015 at 06:21, Phabricator via cfe-commits < cfe-commits@lists.llvm.org> wrote: > This revision was automatically updated to reflect the committed changes. > Closed by commit

Re: [PATCH] D10414: Attach function attribute "arm-restrict-it" instead of passing arm-restrict-it as a backend-option

jmolloy added a subscriber: jmolloy. jmolloy added a comment. Hi Akira, I'm sorry to be contrary (and I missed the discussion on Tuesday because I was away on vacation) but I think there *is* a usecase for -mno-restrict-it to work, and I would hate to see it broken. Non-restricted IT blocks

Re: [PATCH] D10414: Attach function attribute "arm-restrict-it" instead of passing arm-restrict-it as a backend-option

jmolloy added a comment. Hi Jim, In an ideal world, yes. However there's no guarantee that all ARM implementors will (a) be able to commit to LLVM or (b) use ToT. Perhaps they're building a project that uses clang, or a specific version of clang, and this tuning option makes things go faster

Re: [PATCH] D12148: [ARM] Allow passing/returning of __fp16 arguments

jmolloy added a comment. This looks alright to me, but I'd like to wait for someone more familiar with Clang to approve it. http://reviews.llvm.org/D12148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D12148: [ARM] Allow passing/returning of __fp16 arguments

jmolloy added a subscriber: jmolloy. Comment at: lib/CodeGen/TargetInfo.cpp:4717 @@ -4716,1 +4716,3 @@ + // __fp16 gets passed as if it were an int or float, but with the top 32 bits + // unspecified. Top 16 bits? Comment at: