Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

On Wed, Aug 12, 2015 at 6:05 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 6:00 PM, Sean Silva chisophu...@gmail.com wrote: On Wed, Aug 12, 2015 at 2:43 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 12:08 AM, Sean Silva

Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

On Wed, Aug 12, 2015 at 2:43 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 12:08 AM, Sean Silva chisophu...@gmail.com wrote: silvas added a subscriber: silvas. Comment at: lib/Parse/Parser.cpp:2003 @@ +2002,3 @@ +Diag(Tok,

Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

On Wed, Aug 12, 2015 at 6:00 PM, Sean Silva chisophu...@gmail.com wrote: On Wed, Aug 12, 2015 at 2:43 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 12:08 AM, Sean Silva chisophu...@gmail.com wrote: silvas added a subscriber: silvas. Comment

Re: [PATCH] D11944: Nativize filename in FileManager::getFile().

On Thu, Aug 13, 2015 at 10:50 AM, Richard Smith via cfe-commits cfe-commits@lists.llvm.org wrote: rsmith added a comment. I would think most Windows users would be surprised if we showed them paths with /s instead of \s, but I'm fine with us using / internally if it doesn't leak out to the

Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

On Wed, Aug 12, 2015 at 6:05 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 6:00 PM, Sean Silva chisophu...@gmail.com wrote: On Wed, Aug 12, 2015 at 2:43 PM, Richard Smith rich...@metafoo.co.uk wrote: On Wed, Aug 12, 2015 at 12:08 AM, Sean Silva

Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

silvas added inline comments. Comment at: test/Modules/auto-module-import.m:89 @@ +88,3 @@ + expected-error {{import of module 'NoUmbrella.A' appears within function 'includeNotAtTopLevel'}} \ + expected-note {{consider

Re: [PATCH] D12129: [libcxx] Add new Sphinx documentation

silvas added a subscriber: silvas. silvas added a comment. From a Sphinx perspective, this looks fine to me. You may want to consider using `html_theme = 'haiku'` for consistency with clang, but reusing LLVM's is fine. (the reason I had clang use 'haiku' is that it is a bit better put together

Re: [PATCH] D12129: [libcxx] Add new Sphinx documentation

silvas added inline comments. Comment at: docs/BuildingLibcxx.rst:57 @@ +56,3 @@ + select a safe place to install libc++. + + * ``make install-libcxx install-libcxxabi`` --- Will install the libraries and the headers jroelofs wrote: I feel like a warning is

Re: [PATCH] D12129: [libcxx] Add new Sphinx documentation

silvas added inline comments. Comment at: docs/BuildingLibcxx.rst:57 @@ +56,3 @@ + select a safe place to install libc++. + + * ``make install-libcxx install-libcxxabi`` --- Will install the libraries and the headers silvas wrote: jroelofs wrote: I feel

Re: r244912 - [Modules] Add Darwin-specific compatibility module map parsing hacks

On Thu, Aug 13, 2015 at 4:47 PM, Ben Langmuir blangm...@apple.com wrote: Yep, that should be it! Great! -- Sean Silva On Aug 13, 2015, at 4:45 PM, Sean Silva chisophu...@gmail.com wrote: This was the last thing blocking http://reviews.llvm.org/D10423 on your side, right? -- Sean

Re: r244912 - [Modules] Add Darwin-specific compatibility module map parsing hacks

This was the last thing blocking http://reviews.llvm.org/D10423 on your side, right? -- Sean Silva On Thu, Aug 13, 2015 at 10:13 AM, Ben Langmuir via cfe-commits cfe-commits@lists.llvm.org wrote: Author: benlangmuir Date: Thu Aug 13 12:13:33 2015 New Revision: 244912 URL:

Re: r253012 - [modules] When a declaration has non-trivial visibility, check whether it's

On Thu, Nov 12, 2015 at 9:14 PM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Thu Nov 12 23:14:45 2015 > New Revision: 253012 > > URL: http://llvm.org/viewvc/llvm-project?rev=253012=rev > Log: > [modules] When a declaration has non-trivial

Re: r253012 - [modules] When a declaration has non-trivial visibility, check whether it's

On Mon, Nov 16, 2015 at 7:09 PM, Richard Smith wrote: > On Mon, Nov 16, 2015 at 7:00 PM, Sean Silva wrote: > >> On Sat, Nov 14, 2015 at 2:30 PM, Richard Smith >> wrote: >> >>> On Nov 13, 2015 7:23 PM, "Sean Silva"

Re: r253012 - [modules] When a declaration has non-trivial visibility, check whether it's

On Sat, Nov 14, 2015 at 2:30 PM, Richard Smith wrote: > On Nov 13, 2015 7:23 PM, "Sean Silva" wrote: > > > > > > > > On Thu, Nov 12, 2015 at 9:14 PM, Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> > >> Author: rsmith > >>

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Sun, Nov 1, 2015 at 10:20 AM, Manuel Klimek wrote: > > > On Fri, Oct 23, 2015 at 9:31 PM Sean Silva wrote: > >> On Tue, Oct 20, 2015 at 1:52 AM, Manuel Klimek wrote: >> >>> On Tue, Oct 20, 2015 at 10:41 AM Sean Silva

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Wed, Nov 4, 2015 at 1:07 PM, Richard Smith wrote: > On Sun, Nov 1, 2015 at 10:20 AM, Manuel Klimek wrote: > >> On Fri, Oct 23, 2015 at 9:31 PM Sean Silva wrote: >> >>> On Tue, Oct 20, 2015 at 1:52 AM, Manuel Klimek

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Thu, Nov 5, 2015 at 1:12 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > On Wed, Nov 4, 2015 at 7:14 PM, Sean Silva via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> On Wed, Nov 4, 2015 at 1:07 PM, Richard Smith <rich...@metafoo.co.uk> >> wro

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

On Tue, Oct 13, 2015 at 11:33 PM, Eric Christopher wrote: > It was already reverted, but I agree, let's get this fixed first. > It was reintroduced in r250252. It is breaking http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1362 -- Sean

Re: r250262 - I took care of the build problem in the commit 250252.

This is breaking llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1358/steps/test/logs/stdio On Tue, Oct 13, 2015 at 6:09 PM, Ekaterina Romanova via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: kromanova

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

On Tue, Oct 13, 2015 at 7:51 PM, Katya Romanova < katya_roman...@playstation.sony.com> wrote: > kromanova added a comment. > > Hi, > > The initial PS4 patch caused a test failure (debug-options.c) on the PS4 > bot. I suspect that I know why the problem happens, but I'm not sure what > will be the

r250273 - Revert-to-green r250262 (PS4 toolchain patch)

Author: silvas Date: Wed Oct 14 01:45:07 2015 New Revision: 250273 URL: http://llvm.org/viewvc/llvm-project?rev=250273=rev Log: Revert-to-green r250262 (PS4 toolchain patch) It is breaking llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast e.g.

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Tue, Oct 20, 2015 at 1:38 AM, Manuel Klimek wrote: > On Tue, Oct 20, 2015 at 5:52 AM Sean Silva wrote: > >> On Mon, Oct 19, 2015 at 2:10 AM, Manuel Klimek wrote: >> >>> On Sat, Oct 17, 2015 at 3:41 AM Richard Smith via cfe-commits

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Fri, Oct 16, 2015 at 6:41 PM, Richard Smith wrote: > On Fri, Oct 16, 2015 at 6:30 PM, Sean Silva wrote: > >> On Fri, Oct 16, 2015 at 6:26 PM, Richard Smith >> wrote: >> >>> On Fri, Oct 16, 2015 at 6:25 PM, Sean Silva

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Mon, Oct 19, 2015 at 2:10 AM, Manuel Klimek wrote: > On Sat, Oct 17, 2015 at 3:41 AM Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> On Fri, Oct 16, 2015 at 6:30 PM, Sean Silva >> wrote: >> >>> On Fri, Oct 16, 2015 at 6:26

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Tue, Oct 20, 2015 at 6:45 AM, Manuel Klimek wrote: > On Tue, Oct 20, 2015 at 3:38 PM Brad King wrote: > >> On 10/20/2015 04:38 AM, Manuel Klimek wrote: >> > On Tue, Oct 20, 2015 at 5:52 AM Sean Silva wrote: >> >> get cmake to generate clang module

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Fri, Oct 16, 2015 at 4:20 PM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Fri Oct 16 18:20:19 2015 > New Revision: 250577 > > URL: http://llvm.org/viewvc/llvm-project?rev=250577=rev > Log: > [modules] Allow the error when explicitly loading an

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Fri, Oct 16, 2015 at 6:12 PM, Richard Smith wrote: > On Fri, Oct 16, 2015 at 4:43 PM, Sean Silva wrote: > >> On Fri, Oct 16, 2015 at 4:20 PM, Richard Smith via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Author: rsmith >>> Date:

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

On Fri, Oct 16, 2015 at 6:26 PM, Richard Smith wrote: > On Fri, Oct 16, 2015 at 6:25 PM, Sean Silva wrote: > >> On Fri, Oct 16, 2015 at 6:12 PM, Richard Smith >> wrote: >> >>> On Fri, Oct 16, 2015 at 4:43 PM, Sean Silva

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

On Wed, Oct 14, 2015 at 12:54 AM, Romanova, Katya < katya_roman...@playstation.sony.com> wrote: > Hi Sean, > > > > I don’t think that the whole patch should have been reverted. There were > much easier way to make the PS4 bot green. A single test could have been > marked as “XFAIL: ps4” for a few

Re: [libcxx] r249738 - Split out of .

On Wed, Oct 14, 2015 at 3:32 PM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Wed, Oct 14, 2015 at 2:03 PM, Adrian Prantl via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> On Oct 14, 2015, at 11:37 AM, Adrian Prantl via cfe-commits < >>

Re: [PATCH] D12747: Implement [depr.c.headers]

On Tue, Oct 6, 2015 at 4:13 PM, Richard Smith wrote: > On Tue, Oct 6, 2015 at 4:11 PM, Sean Silva wrote: > >> +extern "C++" { >> +#include <__nullptr> >> +using std::nullptr_t; >> +} >> >> Does this even compile with modules? >> > > Yes. You're

Re: r250252 - This patch adds missing pieces to clang, including the PS4 toolchain

Looks like this doesn't even compile: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/32311/steps/compile/logs/stdio Can you revert? On Tue, Oct 13, 2015 at 4:40 PM, Ekaterina Romanova via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: kromanova > Date: Tue Oct 13

Re: [libcxx] r249738 - Split out of .

On Tue, Oct 13, 2015 at 6:54 PM, Sean Silva wrote: > > > On Tue, Oct 13, 2015 at 6:14 PM, Richard Smith > wrote: > >> On Tue, Oct 13, 2015 at 5:31 PM, Sean Silva >> wrote: >> >>> On Tue, Oct 13, 2015 at 3:17 PM, Richard Smith

Re: [PATCH] D12271: [X86] Expose the various _rot intrinsics on non-MS platforms

silvas added a subscriber: silvas. silvas added a comment. In http://reviews.llvm.org/D12271#231204, @rnk wrote: This looks good. As a larger issue, LLVM fast isel definitely won't pattern match this series of shifts and selects to rotl at -O0. There are some users who want branchless

Re: r245821 - [modules] Stop updating all identifiers when writing a module. This is

On Sun, Aug 23, 2015 at 8:33 PM, Richard Smith via cfe-commits cfe-commits@lists.llvm.org wrote: Author: rsmith Date: Sun Aug 23 22:33:22 2015 New Revision: 245821 URL: http://llvm.org/viewvc/llvm-project?rev=245821view=rev Log: [modules] Stop updating all identifiers when writing a

Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

silvas added a comment. ping? http://reviews.llvm.org/D11844 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r246702 - [modules] Tighten up this test a bit.

Author: silvas Date: Wed Sep 2 15:16:09 2015 New Revision: 246702 URL: http://llvm.org/viewvc/llvm-project?rev=246702=rev Log: [modules] Tighten up this test a bit. Modified: cfe/trunk/test/Modules/module-map-path-hash.cpp Modified: cfe/trunk/test/Modules/module-map-path-hash.cpp URL:

Re: r247765 - [modules] Fix a corner case in the macro override rules: properly handle overridden leaf module macros.

On Tue, Sep 15, 2015 at 5:55 PM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Tue Sep 15 19:55:50 2015 > New Revision: 247765 > > URL: http://llvm.org/viewvc/llvm-project?rev=247765=rev > Log: > [modules] Fix a corner case in the macro override

r247498 - Clean up trailing whitespace in the builtin headers

Author: silvas Date: Fri Sep 11 21:55:19 2015 New Revision: 247498 URL: http://llvm.org/viewvc/llvm-project?rev=247498=rev Log: Clean up trailing whitespace in the builtin headers Modified: cfe/trunk/lib/Headers/altivec.h cfe/trunk/lib/Headers/ammintrin.h

Re: [PATCH] D11844: [Modules] More descriptive diagnostics for misplaced import directive

silvas requested changes to this revision. silvas added a reviewer: silvas. This revision now requires changes to proceed. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7774 @@ -7771,1 +7773,3 @@ "@import of module '%0' in implementation of '%1'; use #import">; +def

Re: r248953 - [Sema] Don't crash when friending an unqualified templated constructor

On Wed, Sep 30, 2015 at 3:07 PM, David Majnemer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: majnemer > Date: Wed Sep 30 17:07:43 2015 > New Revision: 248953 > > URL: http://llvm.org/viewvc/llvm-project?rev=248953=rev > Log: > [Sema] Don't crash when friending an unqualified

Re: [PATCH] D11279: Initial patch for PS4 toolchain

silvas added a subscriber: silvas. silvas added a comment. Once this lands, I think we can revert r247977 http://reviews.llvm.org/D11279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [clang-tools-extra] r248080 - clang-tools-extra: Appease PR24881. [-Wdocumentation]

Thanks! -- Sean Silva On Fri, Sep 18, 2015 at 7:21 PM, NAKAMURA Takumi via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: chapuni > Date: Fri Sep 18 21:21:28 2015 > New Revision: 248080 > > URL: http://llvm.org/viewvc/llvm-project?rev=248080=rev > Log: > clang-tools-extra: Appease

Re: [clang-tools-extra] r254785 - Added coverage check for extensionless headers, and exclude hidden dot directoryies.

On Fri, Dec 4, 2015 at 2:42 PM, John Thompson via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: jtsoftware > Date: Fri Dec 4 16:42:18 2015 > New Revision: 254785 > > URL: http://llvm.org/viewvc/llvm-project?rev=254785=rev > Log: > Added coverage check for extensionless headers, and

Re: [PATCH] D15217: Clang documentation for UBSan.

silvas accepted this revision. silvas added a comment. Thanks for working on this! A tiny nit, but otherwise this LGTM. Comment at: docs/UndefinedBehaviorSanitizer.rst:33 @@ +32,3 @@ + +Use ``clang++`` to compile and link your program with ``-fsanitize=undefined`` +flag. Make

Re: [PATCH] D15462: [CMake] Add support for generating profdata for clang from training files

silvas added a comment. Can you elaborate on why this patch is restricted to unix? Also, this probably requires some documentation in http://llvm.org/docs/CMake.html (we don't really have an analogous page just for clang currently, so the llvm one is probably the best place right now)

Re: r255377 - Reapply "[Modules] Fix regression when an elaborated-type-specifier mentions a hidden tag"

On Fri, Dec 11, 2015 at 2:05 PM, Ben Langmuir via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: benlangmuir > Date: Fri Dec 11 16:05:13 2015 > New Revision: 255377 > > URL: http://llvm.org/viewvc/llvm-project?rev=255377=rev > Log: > Reapply "[Modules] Fix regression when an

Re: r255366 - [PGO] Stop using invalid char in instr variable names.

On Fri, Dec 11, 2015 at 11:53 AM, Xinliang David Li via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: davidxl > Date: Fri Dec 11 13:53:35 2015 > New Revision: 255366 > > URL: http://llvm.org/viewvc/llvm-project?rev=255366=rev > Log: > [PGO] Stop using invalid char in instr variable

Re: [PATCH] D15462: [CMake] Add support for generating profdata for clang from training files

silvas added a comment. Thanks. This LGTM but I'd wait for Duncan or Justin to sign off on it; they're likely to have more higher-level thoughts. Comment at: utils/perf-training/perf-helper.py:36 @@ +35,3 @@ + cmd.extend(findProfrawFiles(args[2])) +

Re: [PATCH] D15462: [CMake] Add support for generating profdata for clang from training files

silvas added a comment. In http://reviews.llvm.org/D15462#309889, @beanz wrote: > Sean, > > The reason for restricting to Unix is two fold. (1) the shell script goop, > which I can replace with python and (2) I don't have a windows box to test > on, so I didn't want people to think it worked.

Re: r253721 - [CMake] Add support for specifying arguments to the bootstrap build.

On Fri, Nov 20, 2015 at 2:09 PM, Chris Bieneman via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: cbieneman > Date: Fri Nov 20 16:09:06 2015 > New Revision: 253721 > > URL: http://llvm.org/viewvc/llvm-project?rev=253721=rev > Log: > [CMake] Add support for specifying arguments to the

Re: r271544 - [docs] Add a limitations section to SourceBasedCodeCoverage.rst

On Thu, Jun 2, 2016 at 10:19 AM, Vedant Kumar via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: vedantk > Date: Thu Jun 2 12:19:45 2016 > New Revision: 271544 > > URL: http://llvm.org/viewvc/llvm-project?rev=271544=rev > Log: > [docs] Add a limitations section to

Re: r271918 - Add a release note about the --build-id change.

On Mon, Jun 6, 2016 at 11:23 AM, Rafael Espindola via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rafael > Date: Mon Jun 6 13:23:11 2016 > New Revision: 271918 > > URL: http://llvm.org/viewvc/llvm-project?rev=271918=rev > Log: > Add a release note about the --build-id change. > >

Re: [PATCH] D21186: clang-rename: fix the DynamicCastExpr test on ps4

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

Re: [PATCH] D21186: clang-rename: mark the DynamicCastExpr test unsupported on ps4

silvas added a subscriber: silvas. silvas added a comment. > It has no RTTI, so the test would always fail in a non-interesting way. It does. There is a call to dynamic_cast on lines 20 and 21 of this test. You can fix this by explicitly adding -frtti I believe.

Re: [PATCH] D21186: clang-rename: mark the DynamicCastExpr test unsupported on ps4

silvas added a comment. In http://reviews.llvm.org/D21186#454497, @silvas wrote: > > It has no RTTI, so the test would always fail in a non-interesting way. > > > It does. There is a call to dynamic_cast on lines 20 and 21 of this test. You > can fix this by explicitly adding -frtti I believe.

Re: Builder llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast is back to normal

What was the issue? On Wed, May 25, 2016 at 12:17 PM, Galina Kistanova via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > Hello, > > Builder > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast > is back to normal. > Sorry for the noise. > > Thanks > >

Re: r272604 - [docs] Coverage: Document the profile merge pool specifier

Thanks! On Tue, Jun 14, 2016 at 10:30 AM, Vedant Kumar wrote: > I think David got rid of that limitation. > > The related warning in compiler-rt does not reflect this: > > PROF_WARN("%%m specifier can only be specified once at the end of > %s.\n", ...); > > Fixed the warning

Re: Buildbot numbers for the last week of 6/05/2016 - 6/11/2016

Some of these tables are getting quite mangled somewhere along the line (email client? mailing list handling? idk). Could you re-send with the tables also attached or something else that avoids that problem? -- Sean Silva On Tue, Jun 14, 2016 at 4:23 PM, Galina Kistanova via cfe-commits <

Re: r272604 - [docs] Coverage: Document the profile merge pool specifier

On Mon, Jun 13, 2016 at 5:42 PM, Vedant Kumar via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: vedantk > Date: Mon Jun 13 19:42:12 2016 > New Revision: 272604 > > URL: http://llvm.org/viewvc/llvm-project?rev=272604=rev > Log: > [docs] Coverage: Document the profile merge pool

Re: [PATCH] D21047: [clang] Documentation fixes for LibASTMatchersTutorial

silvas accepted this revision. silvas added a comment. This revision is now accepted and ready to land. Overall this LGTM, but let Manuel or one of the other libTooling folks sign off. Comment at: docs/LibASTMatchersTutorial.rst:46 @@ -45,3 +45,3 @@ cd ~/clang-llvm

Re: [PATCH] D20715: [docs] Document the source-based code coverage feature

silvas added subscribers: MaggieYi, phillip.power. silvas added a comment. This is looking really good. So based on reading this, a rough sketch of how we can expand this to PGO would be something like: - there is a separate PGO page that has similar structure, but is more tuned for the PGO

Re: [PATCH] Restructure the propagation of -fPIC/-fPIE.

-def fpie : Flag<["-"], "fpie">, Group; +def fpie : Flag<["-"], "fpie">, Group, Flags<[CC1Option]>; I don't think this is the right solution as "-fpie" doesn't mean the same thing to the driver and CC1 is sort of confusing (in CC1 it doesn't mean anything by itself). Can you change the existing

Re: r260201 - [CMake] Providing a CMake cache for 3-stage builds

Neat! Besides the awesomeness of the bootstrap, I didn't know about the -C option! (well, except that you told me about it at the social last week, but this is the first time I saw the actual usage). btw, the README.txt in cmake/caches says "The first two cache files in the directory" -- but it's

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:483 @@ +482,3 @@ +Opts.ProfileIRInstr = true; + else +// Opts.ClangProfInstrGen will be used for Clang instrumentation only. davidxl wrote: > silvas wrote: > > This still isn't

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.def:106 @@ -105,3 +105,3 @@ -CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate -///< execution counts to use with PGO.

Re: r259507 - Make the remaining headers self-contained.

On Tue, Feb 2, 2016 at 11:15 AM, David Blaikie via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > > On Tue, Feb 2, 2016 at 11:11 AM, Rafael Espíndola < > cfe-commits@lists.llvm.org> wrote: > >> Out of curiosity, what technique were you using to find out if the >> headers were

Re: [PATCH] D17622: [PGO] clang cc1 option change to enable IR level instrumenation

silvas added a comment. Thanks for splitting this out. I think the `-fprofile-instrument=llvm` is straightforward and uncontroversial. Let's focus this review on that and split the others into a separate review. But a quick preview of my thoughs about `-fprofile-instrument={llvm,clang}-use`: I

Re: [clang-tools-extra] r261737 - [clang-tidy] Added a check for forward declaration in the potentially wrong namespace

This or r261737 seem to be causing a bot failure: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/636 -- Sean Silva On Wed, Feb 24, 2016 at 5:35 AM, Alexander Kornienko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: alexfh > Date: Wed

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added a comment. In http://reviews.llvm.org/D15829#360006, @xur wrote: > Here is the new patch that removes the auto detection of profile kind. > > In this patch, I replace the CC1 option of -fprofile-instr-use=<> with > -fprofile-instrument={llvm-use|clang-use}. For the use compilation,

Re: [PATCH] D17737: [PGO] change profile use cc1 option

silvas added a comment. Thanks, this is looking good. I have some naming suggestions and nits, but the overall content of the patch LGTM. Comment at: include/clang/Driver/CC1Options.td:286 @@ +285,3 @@ +def fprofile_instrument_usepath_EQ : +Joined<["-"],

Re: [PATCH] D17622: [PGO] clang cc1 option change to enable IR level instrumenation

silvas accepted this revision. silvas added a comment. This revision is now accepted and ready to land. Thanks. LGTM. http://reviews.llvm.org/D17622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: r261552 - [VFS] Add 'overlay-relative' field to YAML files

This or r261551 seem to be causing a build failure: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/527 -- Sean SIlva On Mon, Feb 22, 2016 at 10:41 AM, Bruno Cardoso Lopes via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: bruno > Date: Mon

Re: r261552 - [VFS] Add 'overlay-relative' field to YAML files

The bot is still red: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast On Mon, Feb 22, 2016 at 6:48 PM, Bruno Cardoso Lopes < bruno.card...@gmail.com> wrote: > Hi Sean, > > This is hopefully fixed in r261556. > > On Mon, Feb 22, 2016 at 5:45 PM, Sean Silva

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added a comment. This needs tests showing that the IR gen/use passes get run. Maybe use -debug-pass=Structure like test/CodeGen/thinlto_backend.c? My biggest concern is the naming and user visible parts. I can't come up with anything better than `-fprofile-ir-instr` TBH. Overall, from a

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

silvas added a comment. For the preview of all the changes, can you please put that in a separate patch from this one? Repository: rL LLVM http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added a comment. @slingn and I had a discussion offline about the potential names and came up with some ideas, but none is a clear winner. Overall, my feeling is that from a user's perspective, the frontend stuff is probably best referred to as "coverage-based". It's not as clear for

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.def:108 @@ -107,2 +107,3 @@ ///< execution counts to use with PGO. +CODEGENOPT(ProfileIRInstr, 1, 0) ///< IR level code PGO instrumentation and use.

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added a comment. @bogner btw, did you say that at Apple you guys have a requirement of supporting existing profdata? I.e. users can pass older profdata to a newer compiler? Realistically, it would be nice if our PGO offering defaulted to the IR stuff (since it seems like it is going to

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added inline comments. Comment at: lib/Driver/Tools.cpp:3279 @@ -3278,1 +3278,3 @@ + + Args.AddAllArgs(CmdArgs, options::OPT_fprofile_ir_instr); } xur wrote: > mcrosier wrote: > > I don't think AddAllArgs is what you really want. What if the user > >

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added a comment. In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > For the longer term, one possible solution is to make FE based > instrumentation only used for coverage testing which can be turned on > with -fcoverage-mapping option (currently, -fcoverage-mapping can not >

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:440 @@ +439,3 @@ + if (CodeGenOpts.ProfileIRInstr) { +// Should not have ProfileInstrGenerate set -- it is for clang +// instrumentation only. Then change the existing references in

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:487 @@ +486,3 @@ + // Opts.ProfileInstrGenerate will be used for Clang instrumentation only. + if (!Opts.ProfileIRInstr) +Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) ||

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

On Mon, Jan 25, 2016 at 12:52 PM, Rong Xu wrote: > OK. Now I understand what you meant. I was trying to avoid driver changes. > It seems you are saying I can change the driver to support cc1 options more > cleanly. This can be done. > Great! Sorry for the confusion. -- Sean

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

On Mon, Jan 25, 2016 at 11:00 AM, Xinliang David Li wrote: > On Fri, Jan 22, 2016 at 1:43 PM, Sean Silva wrote: > > silvas added a comment. > > > > In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > > > >> For the longer term, one possible

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

silvas added a subscriber: silvas. silvas added a comment. This may sound stupid, but: can you benchmark the time it takes to build some project (that actually uses intrinsics in most translation units, e.g. a game) with the headers w/ and w/o the doxygen comments to check that all the extra

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

silvas added a comment. In http://reviews.llvm.org/D15999#331601, @kromanova wrote: > In http://reviews.llvm.org/D15999#330794, @silvas wrote: > > > This may sound stupid, but: can you benchmark the time it takes to build > > some project (that actually uses intrinsics in most translation

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

silvas added a comment. In http://reviews.llvm.org/D15999#331512, @kromanova wrote: > In http://reviews.llvm.org/D15999#330794, @silvas wrote: > > > This may sound stupid, but: can you benchmark the time it takes to build > > some project (that actually uses intrinsics in most translation

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

On Wed, Jan 20, 2016 at 3:14 PM, Eric Christopher wrote: > > > On Wed, Jan 20, 2016 at 3:12 PM Sean Silva wrote: > >> silvas added a comment. >> >> In http://reviews.llvm.org/D15999#331601, @kromanova wrote: >> >> > In

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added inline comments. Comment at: lib/Driver/Tools.cpp:5520 @@ +5519,3 @@ +A->claim(); +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && +(Args.hasArg(options::OPT_fprofile_instr_generate) || This is definitely not the right

r261053 - [AttrDocs.td] Fix up some reST syntax.

Author: silvas Date: Tue Feb 16 20:08:19 2016 New Revision: 261053 URL: http://llvm.org/viewvc/llvm-project?rev=261053=rev Log: [AttrDocs.td] Fix up some reST syntax. Modified: cfe/trunk/include/clang/Basic/AttrDocs.td Modified: cfe/trunk/include/clang/Basic/AttrDocs.td URL:

Re: r261053 - [AttrDocs.td] Fix up some reST syntax.

Btw, thanks to nlewycky to pointing this out. -- Sean Silva On Tue, Feb 16, 2016 at 6:08 PM, Sean Silva via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: silvas > Date: Tue Feb 16 20:08:19 2016 > New Revision: 261053 > > URL: http://llvm.org/viewvc/llvm-project

Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation

silvas added a comment. Apologies for the delay. At this point, I think that this patch has evolved enough that it is best to start a new patch. I think the steps forward are: - Have cc1 accept -fprofile-instrument=llvm (requires no driver changes, but is enough for developers to test by being

Re: [PATCH] D16851: Update of "GCC extensions not implemented yet" in Clang User's Manual

silvas added a subscriber: silvas. silvas added a comment. Assuming the features are implemented this seems fine. LGTM. Comment at: docs/UsersManual.rst:1698 @@ -1697,3 @@ -- clang does not support #pragma weak (`bug - 3679 `_). Due

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

On Tue, Mar 1, 2016 at 10:54 AM, Nico Weber via cfe-commits < cfe-commits@lists.llvm.org> wrote: > thakis added a comment. > > > I think we can do this separately. A "basename" macro is easier for > programmers to use and no build system change needed. > > > Hm, I would think that adding a flag

Re: [PATCH] D17775: Add a few dllimport/dllexport tests. NFC

silvas added a subscriber: silvas. silvas added a comment. Random question: > We plan to enable dllimport/dllexport support for the PS4 How are you planning on codegenning them? The same way as we do in our private branch? http://reviews.llvm.org/D17775

Re: [PATCH] D17737: [PGO] change profile use cc1 option

silvas accepted this revision. silvas added a comment. This revision is now accepted and ready to land. Thanks! http://reviews.llvm.org/D17737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: r262275 - Infrastructure improvements to Clang attribute TableGen.

All green on the two latest commits. Thanks! http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/961 -- Sean Silva On Mon, Feb 29, 2016 at 6:14 PM, John McCall wrote: > On Feb 29, 2016, at 5:44 PM, Sean Silva

r262285 - Make test more robust.

Author: silvas Date: Mon Feb 29 19:20:15 2016 New Revision: 262285 URL: http://llvm.org/viewvc/llvm-project?rev=262285=rev Log: Make test more robust. Really all these tests are checking is that we find a file path. The behavior when we don't find one will have `"ps4-ld"` in it. We just need a

Re: [PATCH] D17787: [Modules] Don't swallow errors when parsing optional attributes

silvas added a comment. makes sense. can you include a testcase? I assume we are forgetting some sort of diagnostic without this patch. Also, if learning about Clang's modules, I would start with `addHeaderInclude` in lib/Frontend/FrontendActions.cpp. The module map stuff ultimately boils down

  1   2   >