echristo accepted this revision.
echristo added a comment.
Couple of small nits and a request to make some of the change separately, but
otherwise LGTM. For the split part please don't actually submit another patch,
just go ahead and do it :)
Thanks!
-eric
Comment at: inclu
echristo requested changes to this revision.
echristo added a comment.
This revision now requires changes to proceed.
Sounds like Dave is asking for changes so I'll put it down as Request Changes
to get it out of my queue. :)
https://reviews.llvm.org/D31440
__
echristo added a comment.
Committed thusly:
echristo@athyra ~/s/l/t/clang> git svn dcommit
Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
M lib/Driver/ToolChains/Clang.cpp
M test/Driver/debug-options.c
Committed r299037
https://reviews.llvm.org/D30760
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
Sounds good. Do you have commit access?
https://reviews.llvm.org/D30760
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
echristo added inline comments.
Comment at: test/Driver/debug-options.c:201-202
//
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
george.burgess.iv wrote:
> echristo wrote:
> > This seems a little light on the testing, would you
echristo added inline comments.
Comment at: test/Driver/debug-options.c:201-202
//
+// GRECORD: "-dwarf-debug-flags"
+// GRECORD: -### -c -grecord-gcc-switches
+//
This seems a little light on the testing, would you mind adding some more
interesting lines here?
echristo added a comment.
Needs more testing. Might want to make sure that you actually are recording
some useful command line options and that you're looking at the cc1 command
line.
This should also be a Driver test and not CodeGen. You can then use -### to
inspect the command lines as passe
echristo added a comment.
In https://reviews.llvm.org/D30415#706370, @sfertile wrote:
> > I have a patch to do this now. I'll plan on committing it in a bit.
>
> Is there a way to mark the -f form of the option as deprecated? We should
> warn and suggest users switch to the -maltivec option for
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
LGTM.
-eric
https://reviews.llvm.org/D31213
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
echristo added a comment.
Can you add a test for fno-auto-profile?
https://reviews.llvm.org/D31213
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
echristo requested changes to this revision.
echristo added a comment.
This revision now requires changes to proceed.
Why aren't we passing the flags down as a string in the IR?
Needs a testcase: you should only check for IR in clang tests. To make sure
something is propagated to the other end u
echristo added a comment.
In https://reviews.llvm.org/D30415#705889, @echristo wrote:
> In https://reviews.llvm.org/D30415#705196, @uweigand wrote:
>
> > In https://reviews.llvm.org/D30415#704761, @echristo wrote:
> >
> > > In https://reviews.llvm.org/D30415#703652, @uweigand wrote:
> > >
> > > >
echristo added a comment.
In https://reviews.llvm.org/D30415#705196, @uweigand wrote:
> In https://reviews.llvm.org/D30415#704761, @echristo wrote:
>
> > In https://reviews.llvm.org/D30415#703652, @uweigand wrote:
> >
> > > I'm a bit confused by this discussion. -faltivec and -maltivec are
> >
echristo added a comment.
In https://reviews.llvm.org/D30415#703652, @uweigand wrote:
> In https://reviews.llvm.org/D30415#703442, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D30415#703398, @echristo wrote:
> >
> > > Different suggestion:
> > >
> > > Remove the faltivec option. Even gcc doe
echristo requested changes to this revision.
echristo added a comment.
This revision now requires changes to proceed.
Different suggestion:
Remove the faltivec option. Even gcc doesn't support it anymore afaict.
(Go ahead and commit the zvector part if you'd like).
-eric
https://reviews.llvm.
echristo added a comment.
In https://reviews.llvm.org/D29770#674961, @sanwou01 wrote:
> Use clang_cc1 -verify for testing as suggested by @rnk.
>
> @echristo, llc does use the same diags interfaces as clang, however, it is
> lacking the infrastructure to make use of LocCookies.
>
> In any case,
echristo added a comment.
Can we not get llc to use the diags interfaces here?
https://reviews.llvm.org/D29770
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
echristo closed this revision.
echristo added a comment.
This was committed:
commit d65cd1f9424369c4ae7f945fac7fd9e4357451b2
Author: Sean Fertile
Date: Thu Jan 5 21:43:30 2017 +
Add vec_insert4b and vec_extract4b functions to altivec.h
Add builtins for the functions and custom code
echristo added a comment.
Hal has given an ack (offline) as well. Go ahead Tim.
https://reviews.llvm.org/D27872
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
This looks fine to me now, might be good to get someone else to ack as well
though.
https://reviews.llvm.org/D27872
___
cfe-commits mailing
echristo added a comment.
Few comments inline. Generally looks OK, I do share Hal's comment on finding
some way of simplifying the if (someSemantics) ... if (otherSemantics) ...
llvm_unreachable(...) pattern.
What did you have in mind?
Comment at: llvm/include/llvm/ADT/APFlo
echristo added a comment.
Going to commit this?
https://reviews.llvm.org/D28037
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
echristo accepted this revision.
echristo added a comment.
Thanks for explaining all of this and going through it Dehao.
LGTM.
-eric
https://reviews.llvm.org/D25435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-
echristo added a comment.
I don't think this ever was hashed out in the llvm-dev thread?
https://reviews.llvm.org/D25435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
echristo added a reviewer: scanon.
echristo added a comment.
Adding Steve in an attempt to get him to review :)
https://reviews.llvm.org/D27872
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
Sounds fine to me. I want to modify https://reviews.llvm.org/owners/package/1/
at some point, but there's nothing wrong with consistency for now.
https://reviews.llvm.org/D28409
__
echristo added a comment.
In https://reviews.llvm.org/D27872#636147, @timshen wrote:
> In https://reviews.llvm.org/D27872#636130, @echristo wrote:
>
> > Looks pretty weird. Typically I'd suggest just:
> >
> > if (foo) {
> >
> > Foo();
> > return;
> >
> > }
> >
> > since that will keep cogniti
echristo added a comment.
In https://reviews.llvm.org/D27872#628212, @timshen wrote:
> I changed type style to early return.
>
> For constructors and destructors, I use:
>
> if (...) {
> // statement;
> return;
> }
>
>
> For normal functions that returns void, I chose:
>
> if (..
echristo added a comment.
LGTM.
Thanks!
-eric
https://reviews.llvm.org/D28037
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
echristo added a comment.
One inline comment for the clang part and then let's split the llvm part out
into a separate patch please.
Thanks!
-eric
Comment at: clang/lib/Headers/altivec.h:8045
/* vec_sl */
static __inline__ vector unsigned char __ATTRS_o_ai
--
echristo added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:2743-2745
if (LinkageName == Name || (!CGM.getCodeGenOpts().EmitGcovArcs &&
!CGM.getCodeGenOpts().EmitGcovNotes &&
+ !CGM.getCodeGenOpts().Profil
echristo added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:2743-2745
if (LinkageName == Name || (!CGM.getCodeGenOpts().EmitGcovArcs &&
!CGM.getCodeGenOpts().EmitGcovNotes &&
+ !CGM.getCodeGenOpts().Profil
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
LGTM.
Thanks!
https://reviews.llvm.org/D27066
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
201 - 233 of 233 matches
Mail list logo
