Re: [PATCH] Warning for main returning a bool.

2016-11-15 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Aaron Ballman" > To: "Hal Finkel" > Cc: "cfe-commits" , "Joshua Hurwitz" > > Sent: Tuesday, November 15, 2016 4:42:05 PM > Subject: Re: [PATCH] Warning for main returning a bool. > > On Tue, Nov 15, 2016 at 5:22 PM, Hal Finkel wrote: > > - Original Me

[PATCH] D26807: Fix a comment for -fsave-optimization-record

2016-11-17 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D26807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D24933: Enable configuration files in clang

2016-11-20 Thread Hal Finkel via cfe-commits
hfinkel added a comment. What happens with unused arguments in the configuration files? This feature looks potentially useful for me, but only if it suppresses unused-argument warnings. For example, if I put -L/usr/local/lib -stdlib=libc++ into a configuration file, I can't have these: cl

r287999 - Adjust type-trait evaluation to properly handle Using(Shadow)Decls

2016-11-27 Thread Hal Finkel via cfe-commits
Author: hfinkel Date: Sun Nov 27 10:26:14 2016 New Revision: 287999 URL: http://llvm.org/viewvc/llvm-project?rev=287999&view=rev Log: Adjust type-trait evaluation to properly handle Using(Shadow)Decls Since r274049, for an inheriting constructor declaration, the name of the using declaration (and

Re: r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain

2016-12-02 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Jason Henline via cfe-commits" > To: cfe-commits@lists.llvm.org > Sent: Thursday, December 1, 2016 7:42:54 PM > Subject: r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain > > Author: jhen > Date: Thu Dec 1 19:42:54 2016 > New Revision: 288448 > > U

Re: [libcxx] r288544 - Work around a bug in Clang's implementation of noexcept function types

2016-12-02 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Eric Fiselier via cfe-commits" > To: cfe-commits@lists.llvm.org > Sent: Friday, December 2, 2016 4:30:53 PM > Subject: [libcxx] r288544 - Work around a bug in Clang's implementation of > noexcept function types > > Author: ericwf > Date: Fri Dec 2 16:30:52

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-05-01 Thread Hal Finkel via cfe-commits
Richard, et al., Any feedback on my comments below on what C/C++ allows? I'd like to just be missing something ;) Thanks again, Hal On 04/21/2017 06:03 AM, Hal Finkel via Phabricator wrote: ... Our struct-path TBAA does the following: struct X { int a, b; }; X x { 50, 100 }; X *

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-05-01 Thread Hal Finkel via cfe-commits
On 05/01/2017 12:49 PM, Daniel Berlin wrote: On Fri, Apr 21, 2017 at 4:03 AM, Hal Finkel via Phabricator mailto:revi...@reviews.llvm.org>> wrote: hfinkel added a comment. In https://reviews.llvm.org/D32199#732737 , @rsmith wrote: > I

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-05-01 Thread Hal Finkel via cfe-commits
On 05/01/2017 02:31 PM, Daniel Berlin wrote: So you believe that you can index into an object randomly by pointer arithmetic and pull out a different field? For starters, this is illegal because you don't know where the padding bytes are. You cannot assume that X.a + 1

Re: [PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-05-01 Thread Hal Finkel via cfe-commits
On 05/01/2017 02:35 PM, Krzysztof Parzyszek via cfe-commits wrote: On 5/1/2017 2:16 PM, Hal Finkel via cfe-commits wrote: On 05/01/2017 12:49 PM, Daniel Berlin wrote: On 04/21/2017 06:03 AM, Hal Finkel via Phabricator wrote: ... Our struct-path TBAA does the following: struct X

[PATCH] D25686: [Driver] Support "hardfloat" vendor triples used by Gentoo

2016-10-17 Thread Hal Finkel via cfe-commits
hfinkel added a comment. It seems like we should teach Triple how to parse this triples correctly (i.e. to recognize that the vendor is ARM), and then canonicalize the environment to GNUEABIHF (or whatever)? https://reviews.llvm.org/D25686 ___ cfe

[PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D21840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D21843: [Driver][OpenMP] Create tool chains for OpenMP offloading kind.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D21843 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D21845: [Driver][OpenMP] Add specialized action builder for OpenMP offloading actions.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Driver/Driver.cpp:1949 +SpecializedBuilders.push_back(new OpenMPActionBuilder(C, Args, Inputs)); + // sfantao wrote: > hfinke

[PATCH] D21848: [Driver][OpenMP] Add logic for offloading-specific argument translation.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D21848 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D21853: [Driver][OpenMP] Update actions builder to create unbundling action when necessary.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Driver/Driver.cpp:2091 +InputArg->getOption().getKind() == llvm::opt::Option::InputClass && +!types::isSrcFile(HostAction->getType()))

[PATCH] D21847: [Driver][OpenMP] Build jobs for OpenMP offloading actions for targets using gcc tool chains.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Driver/Tools.cpp:334 + LksStream << " OpenMP Offload Linker Script.\n"; + LksStream << "*/\n"; + LksStream << "TARGET(binary)\n";

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. ... > > >> I guess I don't understand the motivation for this change. > > To make work in CUDA, we do the following terrible, awful, > horrible, no good thing: ...well, I can just show you the code. > https://github.com/llvm-project/llvm-project/blob/master/clang/l

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25403#580416, @jlebar wrote: > > Is this because the functions are in instead of in are > > you don't want to mark all of as host/device? > > Yes. cmath is its own beast; we need to have our own implementation of it in > order to direct

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25403#580432, @jlebar wrote: > In https://reviews.llvm.org/D25403#580422, @hfinkel wrote: > > > Okay. Why not fix the Clang builtins so that they're evaluatable for > > constant inputs in a constexpr context? Then we can do this and test the

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25403#580439, @jlebar wrote: > > I'm not sure about that. It seems like a useful feature for the builtins to > > have. Logically speaking, they should be constexpr. > > I agree that it's logically correct for the builtins to be > constexpr-e

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. Here's what I suggest: 1. We commit this patch (I think it logically makes sense; the builtins should be constexpr evaluatable, even if they're currently not) 2. We follow up by fixing Clang to make the builtins constexpr evaluatable (since I think that makes sense rega

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-10-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25403#580444, @jlebar wrote: > //Let "CE" mean "constexpr-evaluatable". // > > libc++ attempts to be backwards-compatible with old versions of clang, right? Yea, we'd need to ifdef the test for older versions of Clang. I've just summarized

Re: r284256 - Link static PIE programs against rcrt0.o on OpenBSD

2016-10-29 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Brad Smith via cfe-commits" > To: "Ed Maste" , sisnk...@gmail.com, > cfe-commits@lists.llvm.org > Sent: Sunday, October 30, 2016 12:57:44 AM > Subject: Re: r284256 - Link static PIE programs against rcrt0.o on OpenBSD > > On 10/25/16 19:34, Brad Smith via cf

Re: r285544 - Add support for __builtin_alloca_with_align

2016-10-31 Thread Hal Finkel via cfe-commits
Hi David, On Reid's patch for this (D25581), Richard said, "This takes the alignment in bits? That's so ridiculously dumb that I would feel bad about accepting this patch unless it comes with a warning for people writing the obvious-but-wrong __builtin_alloca_with_align(sizeof(T), alignof(T))".

[PATCH] D25225: Add an option to save the backend-produced YAML optimization record to a file

2016-10-31 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25225#584010, @twoh wrote: > Is there a particular reason why "opt_record_file" is defined in > CC1Options.td, not Options.td? If -opt-record-file= is provided by > the command line, line 829-831 in CompilerInvocation.cpp won't handle it >

[PATCH] D14274: Add alloc_size attribute to clang

2016-11-02 Thread Hal Finkel via cfe-commits
hfinkel added a comment. What's the status of this? https://reviews.llvm.org/D14274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25764: Add end location of loop to loop metadata.

2016-11-07 Thread Hal Finkel via cfe-commits
hfinkel added a comment. This makes sense to me; @anemet , @rjmccall , any thoughts? https://reviews.llvm.org/D25764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-11-08 Thread Hal Finkel via cfe-commits
hfinkel added inline comments. Comment at: libcxx/test/libcxx/numerics/c.math/constexpr-fns.pass.cpp:24 + +#if __cplusplus >= 201103L +constexpr bool a = std::__libcpp_isnan(0.); I think the preferred form here is: #if TEST_STD_VER >= 11 https://reviews.llvm.o

[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

2016-11-08 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25403#590049, @jlebar wrote: > Use TEST_STD_VER macro. This is fine with me; @EricWF , @mclow.lists ? https://reviews.llvm.org/D25403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D26564: Use PIC relocation mode by default for PowerPC64 ELF

2016-11-11 Thread Hal Finkel via cfe-commits
hfinkel added a comment. @wschmidt and other IBM folks, w.r.t. this and https://reviews.llvm.org/D26566, can you please comment on what GCC does here? Does GCC use -fPIC by default for PPC64 (literally or in effect)? https://reviews.llvm.org/D26564 __

Re: r245459 - According to i686 ABI, long double size on x86 is 12 bytes not 16 bytes.

2015-09-19 Thread Hal Finkel via cfe-commits
FYI: https://llvm.org/bugs/show_bug.cgi?id=24398 was just reopened pointing to a lack of resolution here. -Hal - Original Message - > From: "Yaron Keren via cfe-commits" > To: "Martell Malone" > Cc: "Richard Smith" , "cfe-commits" > > Sent: Friday, August 21, 2015 2:47:50 AM > Subje

Re: [PATCH] Change memcpy/memmove/memset to have dest and source alignment

2015-09-25 Thread Hal Finkel via cfe-commits
Hi Pete, Thanks for working on this. + class IntegerAlignment { + private: +uint64_t Align; You explain in the patch summary why this is here, but please add a comment with the explanation as well. Regarding the auto-upgrade, are we going to run into problems if we separate our the 'vol

r249009 - [PowerPC] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* macros on all PPC cores

2015-10-01 Thread Hal Finkel via cfe-commits
Author: hfinkel Date: Thu Oct 1 08:39:49 2015 New Revision: 249009 URL: http://llvm.org/viewvc/llvm-project?rev=249009&view=rev Log: [PowerPC] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* macros on all PPC cores We support all __sync_val_compare_and_swap_* builtins (only 64-bit on 64-bit targets) o

Re: r244526 - Append options for vectorization when pointer checking threshold is exceeded.

2015-08-10 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Tyler Nowicki via cfe-commits" > To: cfe-commits@lists.llvm.org > Sent: Monday, August 10, 2015 6:05:17 PM > Subject: r244526 - Append options for vectorization when pointer checking > threshold is exceeded. > > Author: tnowicki > Date: Mon Aug 10 18:05:16 2

Re: [Patch][LoopVectorize] Late evaluate of runtime pointer check's threshold

2015-08-10 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Tyler Nowicki" > To: "Hal Finkel" > Cc: "Gerolf Hoflehner" , "Commit Messages and Patches > for LLVM" , > "llvm cfe" > Sent: Monday, August 10, 2015 6:06:24 PM > Subject: Re: [Patch][LoopVectorize] Late evaluate of runtime pointer check's > threshold > >

Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-11 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Marshall Clow via cfe-commits" > To: "mclow lists" , chandl...@gmail.com, > rich...@metafoo.co.uk, e...@efcs.ca > Cc: jo...@netbsd.org, cfe-commits@lists.llvm.org > Sent: Tuesday, August 11, 2015 3:30:10 PM > Subject: Re: [PATCH] D11948: Add some macros to ab

Re: [PATCH] D11948: Add some macros to abstract marking of parameters as "not null", and use them in

2015-08-12 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Dan Albert via cfe-commits" > To: "Marshall Clow" > Cc: "Joerg Sonnenberger" , "cfe-commits" > > Sent: Wednesday, August 12, 2015 6:03:30 PM > Subject: Re: [PATCH] D11948: Add some macros to abstract marking of > parameters as "not null", and use them in >

Re: [PATCH] D11815: Pass subtarget feature "force-align-stack"

2015-08-13 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Eric Christopher" > To: reviews+d11815+public+324fadcdaae02...@reviews.llvm.org, > ahata...@gmail.com, dexonsm...@apple.com, > hfin...@anl.gov > Cc: cfe-commits@lists.llvm.org > Sent: Thursday, August 13, 2015 7:39:53 PM > Subject: Re: [PATCH] D11815: Pass su

Re: [PATCH] D12036: We shouldn't need to pass -fno-strict-aliasing when building clang with clang.

2015-08-14 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Eric Christopher via cfe-commits" > To: reviews+d12036+public+3cb5bf37e2ab4...@reviews.llvm.org, > m...@justinbogner.com > Cc: cfe-commits@lists.llvm.org > Sent: Friday, August 14, 2015 12:50:09 PM > Subject: Re: [PATCH] D12036: We shouldn't need to pass -fno

Re: r245304 - We shouldn't need to pass -fno-strict-aliasing when building clang with clang.

2015-08-18 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Chris Bieneman via cfe-commits" > To: cfe-commits@lists.llvm.org > Sent: Tuesday, August 18, 2015 11:15:44 AM > Subject: r245304 - We shouldn't need to pass -fno-strict-aliasing when > building clang with clang. > > Author: cbieneman > Date: Tue Aug 18 11:15

Re: r245914 - Reimplement the PPC explicit option checking to be a bit more obvious

2015-08-25 Thread Hal Finkel via cfe-commits
Hi Eric, Can you please update the test case for this change? Thanks again, Hal - Original Message - > From: "Eric Christopher via cfe-commits" > To: cfe-commits@lists.llvm.org > Sent: Monday, August 24, 2015 7:59:11 PM > Subject: r245914 - Reimplement the PPC explicit option checking t

Re: r245914 - Reimplement the PPC explicit option checking to be a bit more obvious

2015-08-25 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Eric Christopher" > To: "Hal Finkel" > Cc: cfe-commits@lists.llvm.org > Sent: Tuesday, August 25, 2015 5:51:50 PM > Subject: Re: r245914 - Reimplement the PPC explicit option checking to be a > bit more obvious > > On Tue, Aug 25, 2015 at 3:51 PM Hal Finkel

r246510 - [PowerPC] Support __builtin_ppc_get_timebase

2015-08-31 Thread Hal Finkel via cfe-commits
Author: hfinkel Date: Mon Aug 31 18:55:19 2015 New Revision: 246510 URL: http://llvm.org/viewvc/llvm-project?rev=246510&view=rev Log: [PowerPC] Support __builtin_ppc_get_timebase GCC 4.8+ has a PowerPC-specific intrinsic, __builtin_ppc_get_timebase, to do what Clang's __builtin_readcyclecounter d

r246882 - Don't crash on a self-alias declaration

2015-09-04 Thread Hal Finkel via cfe-commits
Author: hfinkel Date: Fri Sep 4 16:49:21 2015 New Revision: 246882 URL: http://llvm.org/viewvc/llvm-project?rev=246882&view=rev Log: Don't crash on a self-alias declaration We were crashing in CodeGen given input like this: int self_alias(void) __attribute__((weak, alias("self_alias"))); suc

Re: [PATCH] Change memcpy/memmove/memset to have dest and source alignment

2015-11-11 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Pete Cooper" > To: "Hal Finkel" > Cc: "Lang Hames" , "LLVM Commits" > , cfe-commits@lists.llvm.org > Sent: Monday, September 28, 2015 12:46:36 PM > Subject: Re: [PATCH] Change memcpy/memmove/memset to have dest and > source alignment > Hey Hal > Thanks for

Re: [Patch][LoopVectorize]Late evaluation of vectorization requirements

2015-08-09 Thread Hal Finkel via cfe-commits
Hi Tyler, These LGTM, thanks! -Hal - Original Message - > From: "Tyler Nowicki" > To: "Hal J. Finkel" , "Gerolf Hoflehner" > > Cc: "Commit Messages and Patches for LLVM" , > "llvm cfe" > Sent: Thursday, August 6, 2015 3:12:29 PM > Subject: Re: [Patch][LoopVectorize]Late evaluation

Re: [Patch][LoopVectorize] Late evaluate of runtime pointer check's threshold

2015-08-09 Thread Hal Finkel via cfe-commits
Hi Tyler, This looks very useful. Please don't mention '-mllvm -runtime-memory-check-threshold=' in the error message. We should add some clause to #pragma clang loop to control this feature with some associated metadata. '-mllvm' things should not be part of the advertised interface to end us

Re: [Patch][LoopVectorize] Print vectorization analysis when loop hint pragma is specified

2015-08-09 Thread Hal Finkel via cfe-commits
Hi Tyler, Thanks for working on this! +// If a loop hint is provided the diagnostic is always produced. +const char *name = Hints.isForced() ? DiagnosticInfo::AlwaysPrint : LV_NAME; name -> Name These LGTM. P.S. I assume we should add similar logic to the loop unroller (please add a

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-22 Thread Hal Finkel via cfe-commits
hfinkel added a subscriber: hfinkel. hfinkel added a comment. What's the actual desired behavior here? Should the inliner strip out calls to mcount() as it inlines? https://reviews.llvm.org/D22666 ___ cfe-commits mailing list cfe-commits@lists.llvm

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-22 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D22666#493706, @honggyu.kim wrote: > In https://reviews.llvm.org/D22666#493109, @rjmccall wrote: > > > In https://reviews.llvm.org/D22666#493026, @hfinkel wrote: > > > > > What's the actual desired behavior here? Should the inliner strip out >

Re: [PATCH] D19544: Pass for translating math intrinsics to math library calls.

2016-07-25 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D19544#492258, @mmasten wrote: > I think this is just saying that some of the weird types are not supported on > all targets. For now, is it ok to proceed with checking this code in? Correct. In https://reviews.llvm.org/D19544#493403, @mmas

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D22666#495978, @honggyu.kim wrote: > In https://reviews.llvm.org/D22666#493708, @rjmccall wrote: > > > Note that the presence of the mcount call itself will significantly impact > > inlining and, really, the entire optimization pipeline. Havi

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D22666#496218, @hfinkel wrote: > In https://reviews.llvm.org/D22666#495978, @honggyu.kim wrote: > > > In https://reviews.llvm.org/D22666#493708, @rjmccall wrote: > > > > > Note that the presence of the mcount call itself will significantly > >

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-26 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D22666#497085, @honggyu.kim wrote: > In https://reviews.llvm.org/D22666#496355, @hfinkel wrote: > > > Here's a patch for the backend part: https://reviews.llvm.org/D22825 - If > > this makes sense to everyone, we can change the frontend to add

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-28 Thread Hal Finkel via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. Comments about the comment, but otherwise, LGTM. Comment at: lib/CodeGen/CodeGenFunction.cpp:789 @@ -796,1 +788,3 @@ + // Since emitting mcount call here impacts optimiza

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-28 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D22666#500328, @honggyu.kim wrote: > In https://reviews.llvm.org/D22666#500327, @rjmccall wrote: > > > In https://reviews.llvm.org/D22666#500326, @honggyu.kim wrote: > > > > > Should we also modify clang/test/CodeGen/mcount.c as well? I'm not

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

2016-07-28 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D22666#500338, @honggyu.kim wrote: > In https://reviews.llvm.org/D22666#500329, @hfinkel wrote: > > > In this case, I think that making a simple test (changing the current test > > to be) like test/CodeGen/stackrealign.c would be fine. If you

Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

2016-08-14 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D18639#491232, @mclow.lists wrote: > And is there any reason why `__libcpp_isinf` can't just return `false` for > non-fp types? For custom numeric types that have an isinf, etc. found by ADL, they should continue to work. https://reviews.

Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

2016-08-14 Thread Hal Finkel via cfe-commits
hfinkel updated this revision to Diff 67992. hfinkel added a comment. Updated to use scheme suggested by Marshall. https://reviews.llvm.org/D18639 Files: include/cmath include/complex Index: include/complex === --- include/com

Re: [PATCH] D16482: Add builtins for bitreverse intrinsic

2016-02-03 Thread Hal Finkel via cfe-commits
hfinkel added a subscriber: hfinkel. hfinkel accepted this revision. hfinkel added a reviewer: hfinkel. hfinkel added a comment. This revision is now accepted and ready to land. LGTM. http://reviews.llvm.org/D16482 ___ cfe-commits mailing list cfe-c

Re: [PATCH] D16482: Add builtins for bitreverse intrinsic

2016-02-03 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D16482#343177, @arsenm wrote: > r259671. > > Should I put some documentation for these somewhere? > docs/LanguageExtensions.rst seems to be the place, but it only has a random > subset of the current builtins. My impression is that we priori

Re: r259931 - [SystemZ] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros

2016-02-05 Thread Hal Finkel via cfe-commits
Just a general comment: I believe we've now fixed this bug for at least four different architectures (each time as a separate effort). Is there some kind of auditing we could do to make sure we don't run into this again? -Hal - Original Message - > From: "Ulrich Weigand via cfe-commits

Re: r259931 - [SystemZ] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros

2016-02-08 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Ulrich Weigand" > To: "Hal Finkel" > Cc: cfe-commits@lists.llvm.org > Sent: Monday, February 8, 2016 7:53:29 AM > Subject: Re: r259931 - [SystemZ] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP > macros > > Hal Finkel wrote on 05.02.2016 23:14:54: > > > Just a g

Re: r259931 - [SystemZ] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros

2016-02-09 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Ulrich Weigand" > To: "Hal Finkel" > Cc: cfe-commits@lists.llvm.org > Sent: Tuesday, February 9, 2016 6:36:34 AM > Subject: Re: r259931 - [SystemZ] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP > macros > > > > Hal Finkel wrote on 09.02.2016 04:59:21: > > > W

Re: [PATCH] D17103: [CUDA] Don't crash when trying to printf a non-scalar object.

2016-02-10 Thread Hal Finkel via cfe-commits
hfinkel added a subscriber: hfinkel. hfinkel added a comment. In http://reviews.llvm.org/D17103#349182, @jlebar wrote: > Yeah, I have no idea what's the right thing to do here. We can always pass a > null pointer, that's easy. David, Reid, do you know what is the correct > behavior? I think

Re: [PATCH] D17103: [CUDA] Don't crash when trying to printf a non-scalar object.

2016-02-10 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D17103#349254, @jlebar wrote: > In http://reviews.llvm.org/D17103#349245, @hfinkel wrote: > > > In http://reviews.llvm.org/D17103#349182, @jlebar wrote: > > > > > Yeah, I have no idea what's the right thing to do here. We can always > > > pass

Re: [PATCH] D17103: [CUDA] Don't crash when trying to printf a non-scalar object.

2016-02-10 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D17103#349274, @jlebar wrote: > > Ultimately, Sema should be responsible for rejecting this, correct? > > > I guess this is the part I'm unsure of. If it's legal to pass a struct to > printf in regular C++ (seems to be?), I'd guess it should b

Re: [PATCH] D17103: [CUDA] Don't crash when trying to printf a non-scalar object.

2016-02-10 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D17103#349280, @jlebar wrote: > > > I guess this is the part I'm unsure of. If it's legal to pass a struct > > > to printf in regular C++ (seems to be?), I'd guess it should be legal in > > > CUDA, too? I'm just not sure what it's supposed t

Re: [PATCH] D17111: [CUDA] pass debug options to ptxas.

2016-02-10 Thread Hal Finkel via cfe-commits
hfinkel added a subscriber: hfinkel. Comment at: lib/Driver/Tools.cpp:10707 @@ +10706,3 @@ +// ptxas does not accept -g option if optimization is enabled, so we ignore +// compiler's -O* options if we want debug info. +CmdArgs.push_back("-g"); echristo

Re: [PATCH] D17111: [CUDA] pass debug options to ptxas.

2016-02-10 Thread Hal Finkel via cfe-commits
hfinkel added inline comments. Comment at: lib/Driver/Tools.cpp:10707 @@ +10706,3 @@ +// ptxas does not accept -g option if optimization is enabled, so we ignore +// compiler's -O* options if we want debug info. +CmdArgs.push_back("-g"); echristo wrote

Re: r259931 - [SystemZ] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros

2016-02-15 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Ulrich Weigand" > To: "Hal Finkel" > Cc: cfe-commits@lists.llvm.org > Sent: Monday, February 15, 2016 9:06:42 AM > Subject: Re: r259931 - [SystemZ] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP > macros > > > > Hal Finkel wrote on 09.02.2016 18:16:21: > > > >

Re: r261422 - Fix handling of vaargs on PPC32 when going from regsave to overflow.

2016-02-20 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Dimitry Andric via cfe-commits" > To: "Hans Wennborg" > Cc: "Roman Divacky" , "cfe-commits" > > Sent: Saturday, February 20, 2016 8:24:36 AM > Subject: Re: r261422 - Fix handling of vaargs on PPC32 when going from > regsave to overflow. > > On 20 Feb 20

[PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel created this revision. hfinkel added reviewers: rsmith, chandlerc, rcox2, jmolloy, anemet, silviu.baranga, mzolotukhin, spatel, rengolin, delena, Carrot, congh, echristo. hfinkel added a subscriber: cfe-commits. Herald added subscribers: joker.eph, mcrosier. This patch implements support

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel added a comment. > My primary inspiration here is the reports generated by Cray's tools > (http://docs.cray.com/books/S-2496-4101/html-S-2496-4101/z1112823641oswald.html). http://docs.cray.com/books/S-2315-52/html-S-2315-52/fixedds0jdeh38.html is a better link. http://reviews.llvm.or

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D19678#415902, @rsmith wrote: > You give this example: > > > 343 | Loc = ConvertBackendLocation(D, > > Context->getSourceManager()); > > > I | ^ > > > I | ^ > > > How does

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D19678#415844, @rjmccall wrote: > I see what you're going for with "listing file", but I like ICC's option name > much better, or at least something along those lines. Sounds good to me. Do you have a preference on -fopt-report vs. -foptimiz

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D19678#416127, @rcox2 wrote: > Actually, the Intel compiler distinguishes between an optimization report > (-qopt-report) and an annotated listing (-qopt-report-annotate). Interesting; thanks for pointing this out (and for the example). >

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D19678#416039, @hfinkel wrote: > In http://reviews.llvm.org/D19678#415902, @rsmith wrote: > > > You give this example: > > > > > 343 | Loc = ConvertBackendLocation(D, > > > Context->getSourceManager()); > > > > > I |

[PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel created this revision. hfinkel added reviewers: rsmith, aprantl, dexonsmith, dblaikie, echristo. hfinkel added a subscriber: cfe-commits. Herald added subscribers: joker.eph, mcrosier. We currently generate debug info for member calls in the context of the call expression. For member call

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-04-28 Thread Hal Finkel via cfe-commits
hfinkel added inline comments. Comment at: lib/CodeGen/CodeGenAction.cpp:734-737 @@ +733,6 @@ + + OS << llvm::format_decimal(L + 1, LNDigits) << " "; + OS << (LLI.Inlined.Transformed && InlinedCols < 2 ? "I" : " "); + OS << (LLI.Unrolled.Transformed && UnrolledCols

Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

2016-04-29 Thread Hal Finkel via cfe-commits
hfinkel updated this revision to Diff 55610. hfinkel added a comment. Use David's suggested approach: Modify the preferred expression location for member calls. If the callee has a valid location (not all do), then use that. Otherwise, fall back to the starting location. This seems to cleanly fi

[PATCH] D19739: Add a loop's debug location to its llvm.loop metadata

2016-04-29 Thread Hal Finkel via cfe-commits
hfinkel created this revision. hfinkel added reviewers: rsmith, rjmccall, aprantl, dblaikie, echristo, anemet. hfinkel added a subscriber: cfe-commits. Herald added subscribers: joker.eph, mcrosier. Getting accurate locations for loops is important, because those locations are used by the fronten

Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

2016-05-02 Thread Hal Finkel via cfe-commits
e 'const foo', but function is not marked const g->x()->x()->y(); ^~~ /tmp/loc.cpp:3:8: note: 'y' declared here void y(); ^ 2 errors generated. Thanks again, Hal > or perhaps just an ast dump test? > > Perhaps a test for the c

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-05-02 Thread Hal Finkel via cfe-commits
hfinkel updated this revision to Diff 55907. hfinkel added a comment. Renamed the option from -flisting to -foptimization-report as suggested. Moved I/O-related and formatting-related code into Frontend. http://reviews.llvm.org/D19678 Files: include/clang/Driver/CC1Options.td include/clang

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-05-02 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D19678#416127, @rcox2 wrote: > Actually, the Intel compiler distinguishes between an optimization report > (-qopt-report) and an annotated listing (-qopt-report-annotate). The > optimization report lists the info for optimizations in a hierar

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-05-02 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D19678#419361, @rcox2 wrote: > Of course, it would be my preference to mirror the functionality of what is > available in the "new" hierarchical form of optimization report Intel > compiler. So, I would like to distinguish between what Hal is

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

2016-05-03 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D19678#419445, @rjmccall wrote: > This discussion of the command line interface makes me think that we should > be taking Richard's suggestion one step further. Why is Clang's involvement > here more than just handing down specific requests f

Re: [Clang] Convergent Attribute

2016-05-09 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Anastasia Stulova via cfe-commits" > To: "Matt Arsenault" , "Ettore Speziale" > , "Aaron Ballman" > > Cc: "nd" , "Clang Commits" > Sent: Monday, May 9, 2016 12:39:19 PM > Subject: RE: [Clang] Convergent Attribute > > Since it's not a part of any official s

Re: [Clang] Convergent Attribute

2016-05-09 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Richard Smith via cfe-commits" > To: "Matt Arsenault" > Cc: "Clang Commits" > Sent: Monday, May 9, 2016 4:45:04 PM > Subject: Re: [Clang] Convergent Attribute > On Mon, May 9, 2016 at 2:43 PM, Richard Smith < rich...@metafoo.co.uk > > wrote: > > On Sun, M

Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

2016-05-10 Thread Hal Finkel via cfe-commits
Ping. Thanks again, Hal - Original Message - > From: "Hal Finkel via cfe-commits" > To: "David Blaikie" > Cc: "Jun Bum Lim" , "Richard Smith" > , "cfe-commits" > , > reviews+d19708+public+e9ddc42503732...@reviews.l

Re: r268721 - [OPENMP 4.0] Codegen for 'declare simd' directive.

2016-05-11 Thread Hal Finkel via cfe-commits
Hi Alexey, I'm a bit confused regarding what this patch does, in part because the test case does not test any of the function argument or return types. Does it generate these "simd" functions taking scalar arguments? I'd expect that what this does is that it generates functions taking vector ar

Re: r268721 - [OPENMP 4.0] Codegen for 'declare simd' directive.

2016-05-11 Thread Hal Finkel via cfe-commits
Hi Alexey, As I recall, Xinmin's RFC discussed encoding the various possible manglings for vector functions in some attribute. Is that what this does? It is difficult to tell from the test case what's actually happing here. -Hal - Original Message - > From: "Alexey Bataev" > To: "Hal

Re: [PATCH] D20341: [CUDA] Enable fusing FP ops for CUDA by default.

2016-05-17 Thread Hal Finkel via cfe-commits
hfinkel added a subscriber: hfinkel. hfinkel added a comment. In http://reviews.llvm.org/D20341#432461, @jlebar wrote: > I am not sure we want this? Although it matches nvcc, it does not match our > floating-point behavior for C++ in general -- it makes us non-IEEE-whatever > compliant by defa

Re: [PATCH] D20341: [CUDA] Enable fusing FP ops for CUDA by default.

2016-05-17 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D20341#432525, @tra wrote: > In http://reviews.llvm.org/D20341#432494, @hfinkel wrote: > > > > > > > > > That having been said, is this change the equivalent of -ffp-contract=fast > > or -ffp-contract=on? I think it is the latter and we want

Re: [PATCH] D20341: [CUDA] Enable fusing FP ops for CUDA by default.

2016-05-18 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D20341#432586, @jlebar wrote: > > But people also don't expect IEEE compliance on GPUs > > > Is that true? Yes. > You have a lot more experience with this than I do, but my observation of > nvidia's hardware is that it's moved to add *more*

[PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex

2016-03-31 Thread Hal Finkel via cfe-commits
hfinkel created this revision. hfinkel added reviewers: mclow.lists, EricWF, chandlerc. hfinkel added a subscriber: cfe-commits. Herald added a subscriber: mcrosier. The libc-provided isnan/isinf/isfinite macro implementations are specifically designed to function correctly, even in the presence

Re: [PATCH] D18823: Implementation of VlA of GNU C++ extension

2016-04-11 Thread Hal Finkel via cfe-commits
hfinkel added a subscriber: hfinkel. hfinkel added a comment. In http://reviews.llvm.org/D18823#397071, @rengolin wrote: > En passant comment: I really wish we wouldn't. > > The C++ standard had some very careful considerations on VLAs and decided > *not* to support. It wasn't for lack of intere

Re: [PATCH] D18823: Implementation of VlA of GNU C++ extension

2016-04-11 Thread Hal Finkel via cfe-commits
hfinkel added a comment. In http://reviews.llvm.org/D18823#397122, @rengolin wrote: > In http://reviews.llvm.org/D18823#397112, @hfinkel wrote: > > > However, as an implementation extension, this concern is not relevant. I'm > > in favor of this; I have uses who use this feature in GCC. It is ce

Re: r266186 - Enable support for __float128 in Clang

2016-04-14 Thread Hal Finkel via cfe-commits
- Original Message - > From: "Hans Wennborg via cfe-commits" > To: "Nemanja Ivanovic" , "Nico Weber" > > Cc: "cfe-commits" > Sent: Thursday, April 14, 2016 8:07:58 PM > Subject: Re: r266186 - Enable support for __float128 in Clang > > On Wed, Apr 13, 2016 at 2:49 AM, Nemanja Ivanovic v

<    1   2   3   >