[PATCH] D51380: Fix incorrect usage of std::error_category

2018-08-28 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: ilya-biryukov, zturner. Herald added a subscriber: cfe-commits. As mentionned here , this change prevents a dangling pointer in `std:error_code` by using a singleton. Repository: rC Clang htt

[PATCH] D51380: Fix incorrect usage of std::error_category

2018-08-29 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Thank you! Repository: rC Clang https://reviews.llvm.org/D51380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51380: Fix incorrect usage of std::error_category

2018-08-29 Thread Alexandre Ganea via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC340929: [Preamble] Fix incorrect usage of std::error_category (authored by aganea, committed by ). Repository: rC Clang https://reviews.llvm.org/D51380 Files: lib/Frontend/PrecompiledPreamble.cpp

[PATCH] D51806: [clang-cl] Enable -march

2018-09-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: thakis, ddunbar, hans. Herald added a subscriber: cfe-commits. Currently, `-march` does not work in the `clang-cl` driver. We are currently migrating from MSVC. The project size makes that we have to stick with `clang-cl` for a little while.

[PATCH] D51806: [clang-cl] Enable -march

2018-09-10 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. @hans I want to target precisely a given CPU when using `clang-cl`. The options in the `m_x86_Features_Group` are too granular. It is easier to just add `-march=skylake` on the cmd-line. Is there any other way? Repository: rC Clang https://reviews.llvm.org/D51806

[PATCH] D51806: [clang-cl] Enable -march

2018-09-10 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 164666. aganea added a comment. Updated diff with coverage test as requested. Repository: rC Clang https://reviews.llvm.org/D51806 Files: include/clang/Driver/Options.td test/Driver/cl-options.c Index: test/Driver/cl-options.c ==

[PATCH] D51806: [clang-cl] Enable -march

2018-09-10 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Thank you! Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51806: [clang-cl] Enable -march

2018-09-10 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. @hans Just an after thought: maybe we should prevent usage of `-march=` and `/arch:` at the same time. What do you think? I can add another patch for that purpose. Repository: rC Clang https://reviews.llvm.org/D51806 ___

[PATCH] D51806: [clang-cl] Enable -march

2018-09-10 Thread Alexandre Ganea via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC341847: [clang-cl] Enable -march option (authored by aganea, committed by ). Repository: rC Clang https://reviews.llvm.org/D51806 Files: include/clang/Driver/Options.td test/Driver/cl-options.c

[PATCH] D43700: Emit proper CodeView even when not using the cl driver.

2018-02-26 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Thanks for fixing this @zturner. I simply want to back-up what @probinson says above - most of the games we do at Ubisoft are currently using a different compilation toolchains for each platform (we ship at least 4-5 platforms for each top game). It can be clang or it ca

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-10-10 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Updated tests again. This time I've closed all applications, all unused services, and tried to have a "clean" machine. New timings without the child `clang-cl.exe` being invoked (hacked from https://reviews.llvm.org/D52411). The test consists in building Clang+LLVM+LLD

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-10-10 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In https://reviews.llvm.org/D52193#1260862, @zturner wrote: > I can try to get some timings from my machine. How do we handle crash > recovery in the case where we don't spawn a child process? I thought the > whole reason for spawning the cc1 driver as a separate proce

[PATCH] D60018: [codeview] Remove Type member from CVRecord

2019-04-01 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: llvm/include/llvm/DebugInfo/CodeView/CVRecord.h:29 +/// Carrying the size separately instead of trusting the size stored in the +/// record prefix provides some extra safety and flexibility. template class CVRecord { To

[PATCH] D60018: [codeview] Remove Type member from CVRecord

2019-04-01 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done. aganea added inline comments. Comment at: llvm/include/llvm/DebugInfo/CodeView/CVRecord.h:29 +/// Carrying the size separately instead of trusting the size stored in the +/// record prefix provides some extra safety and flexibility. templ

[PATCH] D60018: [codeview] Remove Type member from CVRecord

2019-04-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM. With a few minor comments: Comment at: llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h:122 ~FieldListDeserializer() override { -CVType FieldList; -Fi

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

2019-04-04 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: rnk, scott.linder, uabelho, aprantl. Herald added a project: clang. Herald added a subscriber: cfe-commits. When compiling from an already preprocessed CPP, the checksums generated in the debug info would be those of the (input) preprocessed C

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

2019-04-05 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 193886. aganea marked 2 inline comments as done. aganea edited the summary of this revision. aganea added a comment. I made a more elegant change, where no additional lookups or string compares are required. I noted btw that clang /E does not generate `#line`

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

2019-04-05 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. I'll try profiling several solution on a large unity/jumbo file and get back with some results. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60283/new/ https://reviews.llvm.org/D60283 ___ cf

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

2019-04-05 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D60283#1456497 , @thakis wrote: > See > http://llvm-cs.pcc.me.uk/tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp#756 > for a "is same file" example. I'm not sure adding a bool to PresumedLoc is > worth it for this. Yea

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

2019-04-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. That is strange. As you can’t mix /Yc and /Yu on the command line, MSBuild should issue two different calls to clang-cl, one with /Yc and one /Yu /MP. Can you check with ProcMon that commands are issued in the right order? Repository: rC Clang CHANGES SINCE LAST ACTI

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

2019-04-23 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 196321. aganea retitled this revision from "[clang-cl] Don't emit checksums when compiling a preprocessed CPP" to "[DebugInfo] Don't emit checksums when compiling a preprocessed CPP". aganea added a comment. Herald added a project: LLVM. Herald added a subscri

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: spatel, rnk, nikic, chandlerc, MaskRay, martong, ioeric. aganea added projects: LLVM, clang, lld. Herald added subscribers: rnkovacs, arichardson, mgorny, nhaehnle, jvesely, mehdi_amini, emaste, arsenm. Herald added a reviewer: espindola. Heral

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked 5 inline comments as done. aganea added inline comments. Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054 +} } Fixes ``` [2097/2979] Building CXX object tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 196358. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61046/new/ https://reviews.llvm.org/D61046 Files: clang/trunk/unittests/AST/ASTImporterTest.cpp clang/trunk/unittests/Tooling/LookupTest.cpp lld/trunk/ELF/LinkerScript.cpp llvm/trunk/includ

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 196505. aganea marked 7 inline comments as done. aganea added a comment. Adressed some of the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61046/new/ https://reviews.llvm.org/D61046 Files: clang/trunk/unittests/AST/ASTImporterTest.cpp

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp:1717-1722 +// Fix spurious warning with gcc 7.3 -O3 for NewBldVec[i] below +// warning: array subscript is above array bounds [-Warray-bounds] +#if defined(__GNUC__) && __GNUC__ >=

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked 3 inline comments as done. aganea added inline comments. Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054 +} } rnk wrote: > nikic wrote: > > shafik wrote: > > > aganea wrote: > > > > Fixes > > > > ``` > > > > [2097/2979] Build

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-25 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17 +# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916. +if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0) + set_s

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-25 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 196655. aganea marked 3 inline comments as done. aganea added a comment. Please let me know if other changes are needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61046/new/ https://reviews.llvm.org/D61046 Files: clang/trunk/unittests/AST/ASTI

[PATCH] D42642: [CUDA] Detect installation in PATH

2019-04-25 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Herald added a project: LLVM. Just a quick heads-up - the `cuda-detect-path.cu` test fails on WSL/Ubuntu 18.04: aganea@MTL-BJ842:/mnt/f/svn/buildWSL$ /usr/bin/python3.6 bin/llvm-lit -vv ../clang/test/Driver/cuda-detect-path.cu llvm-lit: /mnt/f/svn/llvm/utils/lit/lit/

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

2019-04-26 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Thanks Paul, your solution is even better. I'll apply rL11 locally - if everything's fine, do you mind if I re-land it again? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60283/new/ https://review

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-05-01 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Ping! Is this good to go? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61046/new/ https://reviews.llvm.org/D61046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D42642: [CUDA] Detect installation in PATH

2019-05-01 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a subscriber: rnk. aganea added a comment. So it turns out this is a symlink issue. The file `clang/trunk/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas` has been synchronized on my Windows 10 PC as a regular text file, not a symlink. It looks like TortoiseSVN doesn't implement sym

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

2019-05-03 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: rnk, jlebar. aganea added a project: clang. Herald added a subscriber: mstorsjo. aganea edited the summary of this revision. The files below have been synced by Tortoise as CRLF, and it looks like they are like that in the depot as well (I'm s

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

2019-05-03 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D61496#1489523 , @probinson wrote: > Checked-in files should not have CRLF endings, in general (there are a very > few exceptions, and these aren't among them). > If the checked-in files have LF endings but your tool checks the

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

2019-05-03 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 197990. aganea added a comment. Changed to use `FileCheck` instead of `grep` Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61496/new/ https://reviews.llvm.org/D61496 Files: clang/trunk/test/Preprocessor/indent_macro.c cla

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-05-06 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done. aganea added a comment. In D61046#1486850 , @rnk wrote: > The only remaining change I find questionable is the R600 backend change creduce'd and reported here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63477#c

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-05-06 Thread Alexandre Ganea via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL360044: Fix compilation warnings when compiling with GCC 7.3 (authored by aganea, committed by ). Changed prior to commit: https://reviews.llvm.org/D61046?vs=196655&id=198265#toc Repository: rL LLVM

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

2019-05-06 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/trunk/test/Preprocessor/indent_macro.c:1-2 -// RUN: %clang_cc1 -E %s | grep '^ zzap$' +// RUN: %clang_cc1 -E %s | FileCheck %s --match-full-lines --strict-whitespace +// CHECK: zzap ---

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-17 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: rnk, hans, zturner. Herald added a reviewer: JDevlieghere. Herald added a subscriber: cfe-commits. This is very preliminary change which adds support for (`clang-cl`) `/MP` (Build with multiple processes). Doc for `/MP` is here

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-18 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. The goal of this change is frictionless compilation into VS2017 when using `clang-cl` as a compiler. We've realized that compiling Clang+LLD with Clang generates a faster executable that with MSVC (even latest one). I currently can't see a good way of generating the Visua

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-18 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In https://reviews.llvm.org/D52193#1238944, @zturner wrote: > In https://reviews.llvm.org/D52193#1238923, @aganea wrote: > > > The goal of this change is frictionless compilation into VS2017 when using > > `clang-cl` as a compiler. We've realized that compiling Clang+LLD

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In https://reviews.llvm.org/D52193#1239171, @hans wrote: > Thanks for adding the Ninja numbers. It confirms that Ninja is significantly > faster than MSBuild + /MP. It is true for the 6-cores Xeon, but not for the dual-18-cores. I think there are two issues there, some

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. > clang-cl isn't supposed to do (explicit) registry accesses when you hold it > right (pass in -fms-compatibility-version etc). Have you seen registry access > costs, or is that speculation? No speculation, I will share the FileMon logs shortly. It is indirectly depende

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-20 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done. aganea added a comment. It seems Reid's change has done good: `cmake` is not as significant as before in the build process. See below the improved timings: The tests consist in a full cleanup (delete build folder), cmake to regenerate, then a full rebuil

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-20 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In https://reviews.llvm.org/D52193#1241056, @zturner wrote: > The process stuff looks cool and useful independently of `/MP`. Would it be > possible to break that into a separate patch, and add a unit test for the > behavior of the `WaitAll` flag? Of course, will do.

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-09-20 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. > clang-cl isn't supposed to do (explicit) registry accesses when you hold it > right (pass in -fms-compatibility-version etc). Have you seen registry access > costs, or is that speculation? Please see this log: F7268226: clang-cl-log.zip

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

2018-10-05 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In https://reviews.llvm.org/D52193#1243456, @thakis wrote: > ...and to reword this a bit: Clang taking a long time to start up in some > configurations is a bug we should profile and fix :-) This is time spent in `ntdll.dll` loading various low-level libraries like `ke

[PATCH] D61914: [Support][Test] Time profiler: add regression test

2019-06-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. Thanks Anton! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61914/new/ https://reviews.llvm.org/D61914 __

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

2019-06-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4 + "directory": "DIR", + "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d", + "file": "DIR/regular_cdb.cpp" Is `-MD -MF` required for clang-s

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

2019-06-10 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision. aganea added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4 + "directory": "DIR", + "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d", +

[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface

2019-06-17 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a reviewer: benlangmuir. aganea added inline comments. Comment at: clang/include/clang/Frontend/Utils.h:118 /// Builds a depdenency file when attached to a Preprocessor (for includes) and /// ASTReader (for module imports), and writes it out at the end of process

[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface

2019-06-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63290/new/ https://reviews.llvm.org/D63290 ___ cfe-commits mailing list cfe-commits@

[PATCH] D63579: [clang-scan-deps] print the dependencies to stdout and remove the need to use -MD options in the CDB

2019-06-20 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:32 +class DependencyCollectorFactory { +public: Do you envision future uses for this factory? Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:50

[PATCH] D63579: [clang-scan-deps] print the dependencies to stdout and remove the need to use -MD options in the CDB

2019-06-20 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:32 +class DependencyCollectorFactory { +public: arphaman wrote: > aganea wrote: > > Do you envision future uses for this factory? > Most likely, yes. > > I don't want to lo

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

2019-07-16 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Ping! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63648/new/ https://reviews.llvm.org/D63648 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

2019-07-17 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D52193#1589096 , @yurybura wrote: > Do you have any plans to make this PR compatible with trunk? Now MSVC with > /MP builds much faster than clang-cl (at least 2x faster for our project)... I'll get back to this after the vaca

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

2019-07-17 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Thanks for getting back Hans! In D63648#1589119 , @hans wrote: > Are you saying the diagnostics were not using absolute paths in those cases > and this patch fixes that? Yes. >> , or --show-includes, or -E > > Those aren't dia

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

2019-07-17 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. The problem is that it is not `cl.exe`'s behavior - it always makes paths absolute: F:\svn\test>cl /c test.cc /showIncludes Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64 Copyright (C) Microsoft Corporation. All rights reserved. test.cc

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

2019-07-18 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. It totally makes sense, thanks for the explanation Nico! Let's forget about `cl` compatibility, that wasn't my initial intent. We always pass //relative// paths on the cmd-line, for all the reasons you've mentioned. We also pass `-fdiagnostics-absolute-paths` hoping to f

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

2019-07-23 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. I will take a look next week when I get back! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63907/new/ https://reviews.llvm.org/D63907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

2019-07-30 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added subscribers: sammccall, JDevlieghere. aganea added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:1 +//===- DependencyScanningFilesystem.h - clang-scan-deps fs ===---*- C++ -*-===// +// Ge

[PATCH] D65511: Delay emitting dllexport explicitly defaulted members until the class is fully parsed (PR40006)

2019-07-31 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11545 +for (CXXMethodDecl *M : WorkList) { + DefineImplicitSpecialMember(*this, M, M->getLocation()); + ActOnFinishInlineFunctionDef(M); rnk wrote: > hans wrote: > > rnk wrote:

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

2019-08-01 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:84 +DependencyScanningFilesystemSharedCache() { + NumShards = 8; + CacheShards = llvm::make_unique(NumShards); arphaman wrote: > aganea wrote: >

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

2019-08-06 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Thanks for the update Alex! Just a few more comments and we should be good to go: Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:117 +std::mutex CacheLock; +llvm::StringMap> Cache; + }; --

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

2019-08-06 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM, thank you! In D63907#1617417 , @arphaman wrote: > Just for reference, this patch still doesn't reuse the FileManager across > invocations in a t

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

2019-08-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: arphaman, dexonsmith, Bigcheese. aganea added a project: clang. Herald added a subscriber: tschuett. This patch fixes: 1. Invisible characters that come just before #include, such as #ifndef. ( were hidden, depending on the display loca

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

2019-08-08 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D65906#1620034 , @arphaman wrote: > Regarding the invisible characters before `#ifdef`, are you sure that's the > right behavior? Should we then copy these invisible characters to the minimized output? Believe it or not, thes

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

2019-08-09 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D65906#1622209 , @arphaman wrote: > @aganea These are not just any invisible characters that you have, this is > the UTF8 BOM. Clang's Lexer skips over them if they're in the beginning of > the file (`Lexer::InitLexer`). The mi

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

2019-08-15 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: rnk, aprantl. aganea added a project: clang. Previously, when clang was compiled with -DLLVM_ENABLE_ASSERTIONS=ON, the attached test was yielding: inlinable function call in a function with debug info must have a !dbg location call voi

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

2019-08-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 216046. aganea marked 4 inline comments as done. aganea added a comment. As requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66328/new/ https://reviews.llvm.org/D66328 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDeclCXX.cpp test/C

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

2019-06-21 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: hans, thakis, rsmith. aganea added a project: clang. Previously, `-fdiagnostics-absolute-paths` did not have an effect in some cases, for example: when using relative include paths, or relative CPP paths, or `--show-includes`, or `-E` or disp

[PATCH] D63579: [clang-scan-deps] print the dependencies to stdout and remove the need to use -MD options in the CDB

2019-06-21 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM! Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63579/new/ https://reviews.llvm.org/D63579 ___ cfe-commits mailing list c

[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

2019-06-25 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. LGTM! Some quick stats on our end (running Windows 10 on a Intel W-2135, 6-core, 3.7 GHz, NVMe SSD): on a large .SLN compiling approx. 16,000 .CPP files through 600 unity .CPPs and 23,000 .H files, out of **86 secs** spent in `ClangScanDeps`, about **32 secs** are spent

[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

2019-06-26 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a subscriber: rnk. aganea added a comment. A bit more detail on what we're seeing on our end (specs in the post above). The 'Count' column represents the number of 1ms samples taken in that function. The 'Weight' column is cumulated times for all cores, for a given process, in ms.

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

2019-07-09 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D64301#1574304 , @rnk wrote: > I think @zturner wanted to teach lit to completely remove the Output/ > directory for every test so we don't have to deal with stale files from > previous tests hanging around. I remember objectin

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

2019-07-09 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D64301#1576615 , @aganea wrote: > In D64301#1574304 , @rnk wrote: > > > I think @zturner wanted to teach lit to completely remove the Output/ > > directory for every test so we don't have

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

2019-05-10 Thread Alexandre Ganea via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC360467: Fixed tests where grep was not matching the linefeed (authored by aganea, committed by ). Changed prior to commit: https://reviews.llvm.org/D61496?vs=197990&id=199069#toc Repository: rC Clang

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

2019-05-15 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 199599. aganea marked an inline comment as done. aganea added a comment. Updated again with a different solution. We can't just act on `Entry.getFile().hasLineDirectives()` because a directive such as `#line 100` simply overrides the `__LINE__`, not `__FILE__

[PATCH] D61945: ftime-trace as a CoreOption

2019-05-15 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: thakis, anton-afanasyev. aganea added a project: clang. As suggested here by @thakis , make so that `-ftime-trace` can be used directly in clang-cl without `-Xclang` Repository: rC Clang https://r

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

2019-05-15 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 199659. aganea marked 3 inline comments as done. aganea added a comment. In D60283#1503256 , @probinson wrote: > Minor stuff. This solution is surprisingly simple, the main question being > (and I don't have an answer

[PATCH] D61945: ftime-trace as a CoreOption

2019-05-16 Thread Alexandre Ganea via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL360907: ftime-trace as a CoreOption (authored by aganea, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D61945?v

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

2019-05-20 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Ping! Any further comments? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60283/new/ https://reviews.llvm.org/D60283 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

2019-05-21 Thread Alexandre Ganea via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC361296: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP (authored by aganea, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60283/new/ ht

[PATCH] D61914: [Support][Test] Time profiler: add regression test

2019-05-22 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Could you please move the test to a more approriate location? (ie. clang/trunk/test/Driver/) Comment at: clang/tools/driver/cc1_main.cpp:245 + +llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n"; +llvm::errs() --

[PATCH] D11850: Delay emitting members of dllexport classes until the class is fully parsed (PR23542)

2018-12-18 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added subscribers: belkiss, rnk, aganea. aganea added a comment. Herald added a subscriber: llvm-commits. Cross-referencing PR40006 . It seems to be a different manifestation of this same bug. @hans Could you possibly take a look at the bug abo

[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-17 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: lld/ELF/Driver.cpp:515 + + if (llvm::timeTraceProfilerEnabled()) { +SmallString<128> path(config->outputFile); Could you possibly move this block (and the init block above) into a new file in `lld/Common/` so we can

[PATCH] D80454: [Clang][test] fix tests when using external assembler

2020-05-22 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. Could you please attach a test to demonstrate the issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80454/new/ https://reviews.llvm.org/D80454 ___ cfe-commits mailing list cf

[PATCH] D80454: [Clang][test] fix tests when using external assembler

2020-05-22 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM. Are you planning on adding another patch for the change that you've sent initially? Ideally, I'd like to relax a bit that constraint in `Driver.cpp`, as in D74447

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

2020-05-26 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D69778#2035805 , @llunak wrote: > @aganea Have you tried how this version of the patch affects PR44953? If not, > could you please try? I've applied the patch, I don't see the issue raised in PR44953 anymore. However as discu

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

2020-05-28 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 266989. aganea added a comment. Herald added subscribers: cfe-commits, steven_wu, aheejin, arichardson, sbc100, emaste. Herald added a reviewer: espindola. Herald added a project: clang. I'm taking over this patch, as discussed offline with Zachary. I've re-i

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

2020-05-29 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D43002#2061162 , @echristo wrote: > First question: > > Since split dwarf has to do some similar things can we not use the same > support? This seems to be a lot of changes for this. > > Second question: > > and if we can't use

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

2020-05-29 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 267111. aganea added a comment. Simplified. Now using `CodeGenOptions` to pass the object filename around. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43002/new/ https://reviews.llvm.org/D43002 Files: clang

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-05-29 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: hans, amccarth, thakis, mstorsjo, akhuang. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. This patch adds some missing information to the `LF_BUILDINFO` which allows for rebuilding a .CPP wi

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 267880. aganea marked an inline comment as done. aganea added a comment. As requested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://reviews.llvm.org/D80833 Files: clang/include/clang/Basic

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:3 +// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s + +int main() { return 42; } Is there a way to launch an arbitrary pro

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked 2 inline comments as done. aganea added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:320 + /// Executable and command-line used to create a given CompilerInvocation. + const char *BuildTool = nullptr; + ArrayRef CommandLineArgs; ---

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done. aganea added a comment. In D80833#2069246 , @amccarth wrote: > Does the full path to the tool end up only in the object file, or does this > make it all the way to the final executable or library? The `LF_BUILDIN

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-03 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done. aganea added a comment. I didn't change the PWD/CWD stored in `LF_BUILDINFO`, it has already been done by Reid a while ago: D53179 (it already stores absolute paths) However absolute paths are stored by design, if we wan

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-03 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked 5 inline comments as done. aganea added a comment. In D80833#2071863 , @hans wrote: > In D80833#2071818 , @aganea wrote: > > > I didn't change the PWD/CWD stored in `LF_BUILDINFO`, it has already been

  1   2   3   4   5   >