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
___
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
http://lis
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
http://lists.l
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
http://lists.llvm.org/cgi-bin/mailman/
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
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
http://lists.llvm.org/cgi-bin/mailm
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
> Date
Author: jamesm
Date: Mon Sep 5 07:28:49 2016
New Revision: 280658
URL: http://llvm.org/viewvc/llvm-project?rev=280658&view=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
Author: jamesm
Date: Wed Aug 31 06:01:41 2016
New Revision: 280220
URL: http://llvm.org/viewvc/llvm-project?rev=280220&view=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 :/
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
http://lists.llvm.org/cgi-bin/mai
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
_
jmolloy added a comment.
Hi Sjoerd,
These make sense - we currently lack any way to inform the backend of the
user's FP strictness requirements for exceptions and denormals which forces us
to be conservative in the backend build attribute generation.
Your new -fno-exceptions-fp-math appears to
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.or
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
htt
Author: jamesm
Date: Tue Aug 16 04:45:36 2016
New Revision: 278786
URL: http://llvm.org/viewvc/llvm-project?rev=278786&view=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:
cfe/trunk/test/Co
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
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
In
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
test/C
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
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:
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
http://lists.llvm.org/cgi-bi
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
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
http://lists.llvm.o
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,
-
jmolloy resigned from this revision.
jmolloy removed a reviewer: jmolloy.
jmolloy added a comment.
I'm not the right person to review this.
http://reviews.llvm.org/D16586
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
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
http://lists.llvm.org/cgi-bin/
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).
>
> —
> Mehdi
>
>
>> On Jan 11, 2
jmolloy created this revision.
jmolloy added a reviewer: rafael.
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 can take LLVM options, but the syntax is confusing:
-Wl,-plugin-opt= or -Xlinker
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
Repository:
Author: jamesm
Date: Mon Dec 21 04:44:36 2015
New Revision: 256146
URL: http://llvm.org/viewvc/llvm-project?rev=256146&view=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:
cfe/trunk
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
>
> >
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 NOTICE: The contents of this
rger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> On Fri, Dec 18, 2015 at 05:31:45PM +, 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).
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
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 th
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 com
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.
Repository
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 2
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
http://lists.llvm.org
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 IR
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 wrote:
> > Not sure I follow - I'm not suggesting a
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 CMa
> 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 together and produc
be allowed when they add value?
James
On Tue, 1 Dec 2015 at 18:29, David Blaikie 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 via cfe-commits
>> wrot
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 m
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 int
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
Author: jamesm
Date: Fri Nov 13 11:29:18 2015
New Revision: 253055
URL: http://llvm.org/viewvc/llvm-project?rev=253055&view=rev
Log:
Slacken the norecurse test slightly
It has been reported that this test currently fails on some Power buildbots due
to them adding a "signext" function attribute.
Hi David,
Honestly, I really don't know. I just updated them due to a change in LLVM
- I hadn't considered doing an audit.
James
On Thu, 12 Nov 2015 at 16:08 David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> On Thu, Nov 12, 2015 at 2:56 AM, James Mol
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
test/CodeGenObjC/objc-lite
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
Author: jamesm
Date: Thu Nov 12 09:36:04 2015
New Revision: 252902
URL: http://llvm.org/viewvc/llvm-project?rev=252902&view=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 tha
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.
Re
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 a
Author: jamesm
Date: Thu Nov 12 04:56:51 2015
New Revision: 252872
URL: http://llvm.org/viewvc/llvm-project?rev=252872&view=rev
Log:
Update clang regression tests for 'norecurse'
FunctionAttrs has just been taught how to infer 'norecurse'. Update clang tests
for LLVM r252871.
Modified:
cfe/
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
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: http:/
Author: jamesm
Date: Mon Sep 21 11:34:58 2015
New Revision: 248173
URL: http://llvm.org/viewvc/llvm-project?rev=248173&view=rev
Log:
Revert "[ARM] Handle +t2dsp feature as an ArchExtKind in ARMTargetParser.def"
This was committed without the code review (http://reviews.llvm.org/D12938)
being app
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 rL2481
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 o
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 ar
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
http://lists.llvm.org/cgi-bin/mailman
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: lib/CodeGen/Ta
63 matches
Mail list logo
