thakis (Nico Weber)Email Not Verified
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 10 2013, 2:43 PM (305 w, 3 d)

Recent Activity

Yesterday

thakis updated the diff for D54678: [gn build] Create abi-breaking.h, config.h, llvm-config.h, and add a build file for llvm/lib/Support..

fix a comment

Sun, Nov 18, 3:17 PM
thakis created D54678: [gn build] Create abi-breaking.h, config.h, llvm-config.h, and add a build file for llvm/lib/Support..
Sun, Nov 18, 11:16 AM

Fri, Nov 16

thakis added inline comments to D54345: Add initial scaffolding for the GN build..
Fri, Nov 16, 8:39 AM

Thu, Nov 15

thakis added a comment to D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.

Thanks!

Thu, Nov 15, 2:50 PM
thakis updated the diff for D54345: Add initial scaffolding for the GN build..

rename toolchain to "unix"

Thu, Nov 15, 1:24 PM
thakis added inline comments to D54345: Add initial scaffolding for the GN build..
Thu, Nov 15, 11:00 AM
thakis added inline comments to D54345: Add initial scaffolding for the GN build..
Thu, Nov 15, 7:19 AM
thakis updated the diff for D54345: Add initial scaffolding for the GN build..

address comments

Thu, Nov 15, 7:19 AM

Tue, Nov 13

thakis added a comment to D54345: Add initial scaffolding for the GN build..

Thanks for the thorough review! I replied "not needed yet" to several items, I hope that's fine.

Tue, Nov 13, 7:10 AM
thakis updated the diff for D54345: Add initial scaffolding for the GN build..

address comments

Tue, Nov 13, 7:07 AM

Mon, Nov 12

thakis added a comment to D54319: clang-cl: Add documentation for /Zc:dllexportInlines-.

Nice!

Mon, Nov 12, 10:40 AM
thakis created D54434: original /Zc:dllexportInlines- prototype.
Mon, Nov 12, 10:11 AM

Sun, Nov 11

thakis updated the diff for D54396: [MS Demangler] Print public:, protected:, private: if set in FunctionClass or a variable's StorageClass..

variables too

Sun, Nov 11, 1:01 PM
thakis added inline comments to D54396: [MS Demangler] Print public:, protected:, private: if set in FunctionClass or a variable's StorageClass..
Sun, Nov 11, 12:24 PM
thakis created D54396: [MS Demangler] Print public:, protected:, private: if set in FunctionClass or a variable's StorageClass..
Sun, Nov 11, 12:24 PM

Fri, Nov 9

thakis updated the diff for D54345: Add initial scaffolding for the GN build..

zero symlinks, in exchange for longer gen command

Fri, Nov 9, 1:04 PM
thakis updated the diff for D54345: Add initial scaffolding for the GN build..

fix bad exec_script_whitelist default

Fri, Nov 9, 12:45 PM
thakis updated the diff for D54345: Add initial scaffolding for the GN build..

just one symlink

Fri, Nov 9, 12:41 PM
thakis created D54345: Add initial scaffolding for the GN build..
Fri, Nov 9, 12:31 PM
thakis created D54294: [MS demangler] Use a slightly shorter unmangling for mangled strings..
Fri, Nov 9, 12:38 AM

Wed, Oct 31

thakis created D53944: Initial documentation for the GN build..
Wed, Oct 31, 11:02 AM

Thu, Oct 25

thakis added a comment to D53457: clang-cl: Add "/clang:" pass-through arg support..

Comment from the peanut gallery:

  • I like the whitelist model for gcc-style flags. It allows us to curate them (and /? output) since many don't make much sense in the cl world.
  • I like the idea behind this patch (and /clang: seems like a good spelling)
Thu, Oct 25, 2:45 PM

Wed, Oct 24

thakis added a comment to D52411: Not for review -- test for not spawning subprocess for -cc1.

This patch doesn't link on Windows/VS2017 as main() is not in the same library as clangDriver

Wed, Oct 24, 10:44 AM

Oct 19 2018

thakis accepted D53434: Java annotation declaration being handled correctly.

Thanks for the fix! Do you happen to know what had regressed this?

Oct 19 2018, 8:47 AM

Oct 9 2018

thakis accepted D53052: [AST] Use -fvisibility value when ignoring -fv-i-h* inline static locals.
Oct 9 2018, 6:23 PM
thakis added a comment to D53021: lld-link: Use /pdbsourcepath: for more places when present..

Thanks!

Oct 9 2018, 10:58 AM
thakis updated the diff for D53021: lld-link: Use /pdbsourcepath: for more places when present..

add requested comment

Oct 9 2018, 10:26 AM
thakis updated the diff for D53021: lld-link: Use /pdbsourcepath: for more places when present..

update comments

Oct 9 2018, 7:49 AM
thakis added inline comments to D53021: lld-link: Use /pdbsourcepath: for more places when present..
Oct 9 2018, 7:37 AM
thakis added a comment to D53021: lld-link: Use /pdbsourcepath: for more places when present..

Thanks for the fast review! All done.

Oct 9 2018, 7:30 AM
thakis updated the diff for D53021: lld-link: Use /pdbsourcepath: for more places when present..

address comments

Oct 9 2018, 7:30 AM
thakis created D53021: lld-link: Use /pdbsourcepath: for more places when present..
Oct 9 2018, 6:49 AM

Oct 5 2018

thakis added a comment to D52949: [Diagnostics] Implement -Wsizeof-pointer-div .

It looks like you ran clang-format on all of lib/Sema/SemaExpr.cpp and changed many lines that are irrelevant to your patch. Can you undo that, please?

Oct 5 2018, 6:05 PM
thakis added a comment to D52942: lld-link: Implement support for %_PDB% and %_EXT% for /pdbaltpath:..

Thanks!

Oct 5 2018, 4:00 PM
thakis updated the diff for D52942: lld-link: Implement support for %_PDB% and %_EXT% for /pdbaltpath:..

forgot to regen diff after latest changes, sorry.

Oct 5 2018, 1:05 PM
thakis created D52942: lld-link: Implement support for %_PDB% and %_EXT% for /pdbaltpath:..
Oct 5 2018, 12:57 PM
thakis added a comment to D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References..

Thanks! Will land with tweaked comment.

Oct 5 2018, 11:15 AM

Oct 4 2018

thakis added a comment to D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References..

krasimir, do you do clang-format reviews these days? If not, do you know who does?

Oct 4 2018, 6:15 PM
thakis added a reviewer for D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.: krasimir.
Oct 4 2018, 6:15 PM
thakis added a comment to D52773: clang-cl: Add /showFilenames option (PR31957).

I agree magic environment variables are bad, but without it we don’t
address the only current actual use we have for this, which is making the
vs integration print filenames. Detecting compiler version from inside the
integration is hard

Oct 4 2018, 10:38 AM

Oct 3 2018

thakis updated the diff for D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References..
Oct 3 2018, 1:06 PM
thakis created D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References..
Oct 3 2018, 12:40 PM
thakis updated the diff for D52832: lld-link: Several tweaks to default entry point selection..

add warning test

Oct 3 2018, 9:30 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 9:22 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:55 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:55 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:55 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:55 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:55 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:55 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:55 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:52 AM
thakis updated the summary of D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:52 AM
thakis created D52832: lld-link: Several tweaks to default entry point selection..
Oct 3 2018, 8:52 AM

Oct 2 2018

thakis accepted D52773: clang-cl: Add /showFilenames option (PR31957).

Looks good. If this is passed and we invoke lld-link, should we give that a similar flag and pass that to lld-link as well? I think link.exe also prints its outputs.

Oct 2 2018, 6:42 AM

Sep 28 2018

thakis accepted D52666: [LLD][COFF] Fix pdb loading when the path points to a removable device.
Sep 28 2018, 12:31 PM

Sep 26 2018

thakis updated subscribers of D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074).

Sounds great, thanks!

Sep 26 2018, 10:19 AM
thakis added a comment to D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074).

Thanks! lgtm.

Sep 26 2018, 5:26 AM

Sep 25 2018

thakis added a comment to D52266: [clang-cl] Provide separate flags for all the /O variants.

Actually, trying this out with MSVC, I don't see any __chkstk calls with /O1, or with eg. /Gs1 for that matter:

Sep 25 2018, 6:42 AM
thakis accepted D52266: [clang-cl] Provide separate flags for all the /O variants.

Maybe file a bug on figuring out the /Gs story and add a FIXME linking to it. Weird.

Sep 25 2018, 6:36 AM

Sep 24 2018

thakis added a comment to D52193: RFC: [clang] Multithreaded compilation support.

...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 :-)

Sep 24 2018, 7:37 AM
thakis added a comment to D52193: RFC: [clang] Multithreaded compilation support.

@thakis > 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:

- the child clang-cl -cc1 takes about ~117ms until it gets into the global initializers. This is on my Haswell PC. On the Skylake, this takes "only" ~60ms.
This probably explains why Ninja is slower on the Skylake when using clang-cl as a compiler. There should be a shorter codepath maybe when only a single .cpp is being compiled, and avoid running the child process.

Sep 24 2018, 7:20 AM
thakis created D52411: Not for review -- test for not spawning subprocess for -cc1.
Sep 24 2018, 7:20 AM

Sep 20 2018

thakis added inline comments to D52266: [clang-cl] Provide separate flags for all the /O variants.
Sep 20 2018, 11:09 AM
thakis added a comment to D52266: [clang-cl] Provide separate flags for all the /O variants.

Sorry, I didn't realize we both set off to do this in parallel. I've incorporated your changes into my patch.

Sep 20 2018, 8:46 AM

Sep 19 2018

thakis added a comment to D52193: RFC: [clang] Multithreaded compilation support.

Since it helps existing msbuild configs, adding this seems like a good thing to me.

Sep 19 2018, 8:54 AM
thakis added a comment to D52266: [clang-cl] Provide separate flags for all the /O variants.

FWIW the recommendation against /Ox in my version is because of https://github.com/ulfjack/ryu/pull/70#issuecomment-412168459

Sep 19 2018, 8:14 AM
thakis added inline comments to D52266: [clang-cl] Provide separate flags for all the /O variants.
Sep 19 2018, 8:10 AM

Sep 17 2018

thakis created D52184: Remove dead function user_cache_directory().
Sep 17 2018, 12:24 PM
thakis added a comment to D52143: Make initializeOutputStream() return false on error and true on success..

Not formally in any kind of mailing list discussion

Sep 17 2018, 10:59 AM
thakis added a comment to D52145: lld-link: Also demangle undefined dllimported symbols..

Thanks! Changed the comment; landing.

Sep 17 2018, 9:32 AM

Sep 15 2018

thakis added a comment to D52145: lld-link: Also demangle undefined dllimported symbols..

Thanks!

Sep 15 2018, 6:39 PM
thakis updated the diff for D52145: lld-link: Also demangle undefined dllimported symbols..

fix case on variable

Sep 15 2018, 5:40 PM
thakis created D52145: lld-link: Also demangle undefined dllimported symbols..
Sep 15 2018, 5:39 PM
thakis abandoned D51887: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..
Sep 15 2018, 4:42 PM
thakis abandoned D51957: https://reviews.llvm.org/D51951, naive parallel hash variant.
Sep 15 2018, 4:42 PM
thakis created D52143: Make initializeOutputStream() return false on error and true on success..
Sep 15 2018, 4:37 PM
thakis added a comment to rL342148: Renovate CMake files in the `llvm-(cfi-verify|exegesis|mca)` tools..

The revert helped, see e.g. http://lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/25686 and http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/23378 (compare to previous build)

Sep 15 2018, 12:33 PM
thakis added a comment to rL342148: Renovate CMake files in the `llvm-(cfi-verify|exegesis|mca)` tools..

I reverted this (and follow-on fix attempts r342154 r342180 r342182 r342193) in r342336 in an attempt to heal the lld bots.

Sep 15 2018, 12:07 PM
thakis added a comment to D52104: lld-link: print demangled symbol names for "undefined symbol" diagnostics.

Ah, looks like all changes are mentioned at the top, just not as comments further down.

Sep 15 2018, 11:41 AM
thakis added a comment to D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

https://reviews.llvm.org/rLLD342334 too. Thanks!

Sep 15 2018, 11:41 AM
thakis added a comment to D52104: lld-link: print demangled symbol names for "undefined symbol" diagnostics.

Also https://reviews.llvm.org/rL342331 and https://reviews.llvm.org/rLLD342332

Sep 15 2018, 11:35 AM
thakis added a comment to D52104: lld-link: print demangled symbol names for "undefined symbol" diagnostics.

Thanks! Submitting...

Sep 15 2018, 11:23 AM

Sep 14 2018

thakis added inline comments to D52104: lld-link: print demangled symbol names for "undefined symbol" diagnostics.
Sep 14 2018, 12:59 PM
thakis added a comment to D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

The symptoms of a collision are just going to be you can’t debug the
program, so not very severe imo, especially since it would almost certainly
be resolved on the next incremental build

Sep 14 2018, 12:06 PM
thakis added a comment to D52104: lld-link: print demangled symbol names for "undefined symbol" diagnostics.

Thanks!

Sep 14 2018, 11:40 AM
thakis updated the diff for D52104: lld-link: print demangled symbol names for "undefined symbol" diagnostics.

ruiu comments

Sep 14 2018, 11:40 AM
thakis added inline comments to D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..
Sep 14 2018, 11:25 AM
thakis updated the diff for D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

improve comment

Sep 14 2018, 10:38 AM
thakis closed D52095: Introduce explicit add_file_reading_unittest target for tests that use llvm::getInputFileDirectory().

r342248, thanks!

Sep 14 2018, 10:37 AM
thakis created D52104: lld-link: print demangled symbol names for "undefined symbol" diagnostics.
Sep 14 2018, 10:31 AM
thakis added inline comments to D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..
Sep 14 2018, 10:27 AM
thakis updated the diff for D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

rebase, minor cleanups

Sep 14 2018, 8:13 AM
thakis added a comment to D51561: [CMake] Add support for unittests that have input files.

All of the lit tests have this restriction, is this something you would do
with unit tests but not lit tests?

Sep 14 2018, 8:00 AM
thakis created D52095: Introduce explicit add_file_reading_unittest target for tests that use llvm::getInputFileDirectory().
Sep 14 2018, 7:54 AM
thakis added a comment to D51561: [CMake] Add support for unittests that have input files.

Unrelatedly, another problem with the current design is that llvm.srcdir.txt must contain an absolute path, which makes it impossible to build on one machine and then copy artifacts and test inputs to another machine and run tests on another machine. DebugInfoPDBTests is the only test in all llvm, lld, clang tests that has this restriction. Requiring some defined fixed pwd for unit tests with inputs would address this issue as well. (As would finding a way to not require reading a file off disk for this test.)

Sep 14 2018, 7:41 AM
thakis added a comment to D51561: [CMake] Add support for unittests that have input files.

These aren't demangle tests fwiw. But that just complicates the cmake
logic, and I don't see the benefit of doing so. Writing 3k of data to the
disk is hardly going to be a noticeable part of the configure step.

The benefits are:

  1. Unit tests reading inputs are bad form; doing that should require an explicit opt-in.
  2. While writing 3k of data to disk isn't causing measurable perf issues, writing a text file next to every test binary just because one test binary needs it is still plain ugly.

    And not doing it is super easy to do. I don't understand the pushback here.
Sep 14 2018, 6:04 AM

Sep 13 2018

thakis added a comment to D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

Actually, this replaces https://reviews.llvm.org/D51887

Sep 13 2018, 11:17 AM
thakis added a comment to D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

This replaces https://reviews.llvm.org/D51957.

Sep 13 2018, 10:52 AM
thakis retitled D51956: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence. from https://reviews.llvm.org/D51951, naive sequential hash variant to lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..
Sep 13 2018, 10:52 AM