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 (275 w, 4 d)

Recent Activity

Yesterday

thakis updated subscribers of D45966: Remove LLVM_INSTALL_CCTOOLS_SYMLINKS.
Mon, Apr 23, 8:15 AM
thakis created D45966: Remove LLVM_INSTALL_CCTOOLS_SYMLINKS.
Mon, Apr 23, 7:54 AM
thakis updated subscribers of D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

In any case, when you see a test failing on bots and the fix isn't obvious,
revert first to get the bots back green.

Mon, Apr 23, 6:58 AM
thakis added a comment to D35209: [ARM] Unify handling of M-Class system registers.

Sorry about the belated review comment, but:

Mon, Apr 23, 6:25 AM

Fri, Apr 20

thakis added inline comments to D45899: TableGen: Add a --write-if-changed flag to tablegen directly instead of doing it in cmake..
Fri, Apr 20, 12:59 PM
thakis created D45899: TableGen: Add a --write-if-changed flag to tablegen directly instead of doing it in cmake..
Fri, Apr 20, 12:59 PM
thakis closed D44359: MC intel asm parser: Allow @ at the start of function names..
Fri, Apr 20, 10:25 AM
thakis closed D45165: Use llvm::sys::fs::real_path() in clang..
Fri, Apr 20, 10:25 AM
thakis closed D45262: Remove llvm-build's --configure-target-def-file..
Fri, Apr 20, 10:24 AM
thakis accepted D45262: Remove llvm-build's --configure-target-def-file..

r330455

Fri, Apr 20, 10:24 AM
thakis closed D45877: clang-cl: Accept (and ignore) /Zc:__cplusplus..

r330427, thanks!

Fri, Apr 20, 6:14 AM
thakis updated the summary of D45877: clang-cl: Accept (and ignore) /Zc:__cplusplus..
Fri, Apr 20, 6:13 AM
thakis created D45877: clang-cl: Accept (and ignore) /Zc:__cplusplus..
Fri, Apr 20, 6:13 AM

Thu, Apr 19

thakis accepted D45777: [UnitTests] NFC/build-perf: Break up nontrivial compile jobs.

Makes sense to me.

Thu, Apr 19, 10:25 AM

Tue, Apr 17

thakis added a comment to D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators..

+1 to just adding a dedicated Wno flag for the new warning instead of
coming up with this new spelling.

Tue, Apr 17, 8:08 PM
thakis added inline comments to D45601: Warn on bool* to bool conversion.
Tue, Apr 17, 5:03 AM

Thu, Apr 12

thakis added a comment to D44909: [DAGCombine] (float)((int) f) --> ftrunc (PR36617).

We think there's an UB check for this already, but v8 isn't UB clean.

Thu, Apr 12, 8:10 AM

Wed, Apr 11

thakis added a comment to D44909: [DAGCombine] (float)((int) f) --> ftrunc (PR36617).

This broke some large integration test on arm64 in Chromium (https://crbug.com/831145 – doesn't yet have any information other than "something is broken" and that we bisected it to this revision). It sounds like the patch got rewritten during review; does "formally allowed by the standard, but extremely likely to break things and surprise people" still apply? It sounds like this is supposed to be behavior-preserving for values that aren't NaN and Inf, but it does change behavior for those two? (If so, in what way?)

Wed, Apr 11, 6:49 PM

Tue, Apr 10

thakis added a comment to D43578: -ftime-report switch support in Clang.

$ svn merge -c -329714 .

  • Reverse-merging r329714 into '.':

A test/Frontend/ftime-report-template-decl.cpp

  • Recording mergeinfo for reverse merge of r329714 into '.': U .

$ svn merge -c -329693 .

  • Reverse-merging r329693 into '.':

U test/Frontend/ftime-report-template-decl.cpp

  • Recording mergeinfo for reverse merge of r329693 into '.': U .

$ svn merge -c -329684 .

  • Reverse-merging r329684 into '.':

D test/Frontend/ftime-report-template-decl.cpp
U include/clang/Frontend/FrontendAction.h
U lib/Parse/ParseTemplate.cpp
U lib/Frontend/FrontendAction.cpp
U lib/Lex/Pragma.cpp
U lib/CodeGen/CodeGenAction.cpp
U include/clang/Parse/Parser.h
U include/clang/Sema/Sema.h
U include/clang/Lex/HeaderSearch.h
U lib/Parse/Parser.cpp
U lib/Frontend/ASTMerge.cpp
U lib/Frontend/FrontendActions.cpp
U lib/Frontend/CompilerInstance.cpp
U lib/Sema/SemaChecking.cpp
U lib/Sema/SemaExpr.cpp
U lib/Sema/Sema.cpp
U lib/Sema/SemaDecl.cpp
U lib/Sema/SemaTemplate.cpp
U lib/Lex/PPMacroExpansion.cpp
U lib/Lex/HeaderSearch.cpp

  • Recording mergeinfo for reverse merge of r329684 into '.': U .

$ svn commit -m 'Revert r329684 (and follow-ups 329693, 329714). See discussion on https://reviews.llvm.org/D43578.'
Sending include/clang/Frontend/FrontendAction.h
Sending include/clang/Lex/HeaderSearch.h
Sending include/clang/Parse/Parser.h
Sending include/clang/Sema/Sema.h
Sending lib/CodeGen/CodeGenAction.cpp
Sending lib/Frontend/ASTMerge.cpp
Sending lib/Frontend/CompilerInstance.cpp
Sending lib/Frontend/FrontendAction.cpp
Sending lib/Frontend/FrontendActions.cpp
Sending lib/Lex/HeaderSearch.cpp
Sending lib/Lex/PPMacroExpansion.cpp
Sending lib/Lex/Pragma.cpp
Sending lib/Parse/ParseTemplate.cpp
Sending lib/Parse/Parser.cpp
Sending lib/Sema/Sema.cpp
Sending lib/Sema/SemaChecking.cpp
Sending lib/Sema/SemaDecl.cpp
Sending lib/Sema/SemaExpr.cpp
Sending lib/Sema/SemaTemplate.cpp
Transmitting file data ...................done
Committing transaction...
Committed revision 329739.

Tue, Apr 10, 11:56 AM
thakis updated subscribers of D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes).

"False positive" means "warning fires but didn't find anything
interesting", not "warning fires while being technically correct". So all
these instances do count as false positives.

Tue, Apr 10, 8:12 AM
thakis added a comment to D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes).

This landing made our clang trunk bots do an evaluation of this warning :-P It fired 8 times, all false positives, and all from unit tests testing that operator= works for self-assignment. (https://chromium-review.googlesource.com/c/chromium/src/+/1000856 has the exact details) It looks like the same issue exists in LLVM itself too, https://reviews.llvm.org/D45082

Tue, Apr 10, 7:10 AM
thakis added a comment to D45082: [RFC][unittests] ADT: silence -Wself-assign diagnostics.

Sorry, that comment was meant for the commit that tweaked the warning in clang. I'll repost there.

Tue, Apr 10, 7:10 AM
thakis added a comment to D45082: [RFC][unittests] ADT: silence -Wself-assign diagnostics.

This landing made our clang trunk bots do an evaluation of this warning :-P It fired 8 times, all false positives, and all from unit tests testing that operator= works for self-assignment. (https://chromium-review.googlesource.com/c/chromium/src/+/1000856 has the exact details)

Tue, Apr 10, 7:02 AM
thakis added a comment to D43578: -ftime-report switch support in Clang.

Also, please add cfe-commits to clang changes. Since this wasn't here and the patch wasn't seen by clang folks, since ftime-report-template-decl.cppwarnings is still failing on the bots (e.g. http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/16388) and since it breaks the shared bot, maybe we should revert for now and reland when the issues are addressed?

Tue, Apr 10, 6:43 AM
thakis updated subscribers of D43578: -ftime-report switch support in Clang.
Tue, Apr 10, 6:40 AM
thakis added a comment to D45165: Use llvm::sys::fs::real_path() in clang..

r329698, thanks!

Tue, Apr 10, 6:39 AM
thakis added a comment to D45485: [CMake] Unbreak BUILD_SHARED_LIBS workflow.

Thanks for the pointer! I commented on https://reviews.llvm.org/D43578; I'm hoping there's a fix that doesn't require making so many libs depend on clang's IR library.

Tue, Apr 10, 6:30 AM
thakis updated subscribers of D43578: -ftime-report switch support in Clang.

@davezarzycki remarks in https://reviews.llvm.org/D45485 that this breaks the shared build. The proposed fix there is to make several of clang's modules depend on LLVM's IR library ("Core"). This seems weird to me for two reasons, one architectural, one form a build point of view:

Tue, Apr 10, 6:29 AM
thakis added a comment to D45485: [CMake] Unbreak BUILD_SHARED_LIBS workflow.

Can you link to the recent timer changes?

Tue, Apr 10, 6:07 AM
thakis added a comment to D45262: Remove llvm-build's --configure-target-def-file..

Unless someone shouts, I'm going to submit this CL in the next few days. If someone does depend on this code, we can easily put it back.

Tue, Apr 10, 6:07 AM

Fri, Apr 6

thakis added a comment to D45262: Remove llvm-build's --configure-target-def-file..

ddunbar: ping

Fri, Apr 6, 8:34 PM

Wed, Apr 4

thakis created D45262: Remove llvm-build's --configure-target-def-file..
Wed, Apr 4, 8:23 AM
thakis accepted D45260: COFF: Layout sections in the same order as link.exe.

lgtm, but I don't know this code super well, so wait for ruiu too.

Wed, Apr 4, 8:18 AM
thakis added a comment to D43286: [CodeGen] Generate DWARF v5 Accelerator Tables.

It's not just MSVC, see the polly bot.

Wed, Apr 4, 6:11 AM
thakis added a comment to D43286: [CodeGen] Generate DWARF v5 Accelerator Tables.

This breaks compile on various bots:
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/26736/steps/build%20clang%20lld/logs/stdio
http://lab.llvm.org:8011/builders/lldb-x86-windows-msvc2015/builds/21016/steps/build/logs/stdio
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-parallel-fast/builds/12248/steps/build%20stage%201/logs/stdio
(and others)

Wed, Apr 4, 5:47 AM
thakis added a comment to D45165: Use llvm::sys::fs::real_path() in clang..

bruno: ping

Wed, Apr 4, 5:36 AM
thakis added a comment to D45167: Use sys::fs::real_path() instead of realpath() in Symbolize and remove HAVE_REALPATH..

kcc, if samsonov no longer does llvm reviews, who can review this?

Wed, Apr 4, 5:35 AM
thakis added a reviewer for D45167: Use sys::fs::real_path() instead of realpath() in Symbolize and remove HAVE_REALPATH.: kcc.
Wed, Apr 4, 5:34 AM

Tue, Apr 3

thakis added inline comments to D44452: [llvm-ar] Support multiple dashed options.
Tue, Apr 3, 7:54 PM

Mon, Apr 2

thakis accepted D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17.

I’d expect _LIBCPP_FORCE_NODISCARD to win over _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17, but /shrug. Thanks!

Mon, Apr 2, 1:24 PM
thakis added a comment to D45163: [Sema] -Wunused-value: diagnose unused std::move() call results..

(See also https://bugs.llvm.org/show_bug.cgi?id=10011 for a somewhat related discussion.)

Mon, Apr 2, 10:39 AM
thakis created D45167: Use sys::fs::real_path() instead of realpath() in Symbolize and remove HAVE_REALPATH..
Mon, Apr 2, 8:47 AM
thakis created D45165: Use llvm::sys::fs::real_path() in clang..
Mon, Apr 2, 8:35 AM
thakis added a comment to D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes).

Historically Clang's policy on warnings was, I think, much more
conservative than it seems to be today. There was a strong desire not to
implement off-by-default warnings, and to have warnings with an
exceptionally low false-positive rate - maybe the user-defined operator
detection was either assumed to, or demonstrated to, have a sufficiently
high false positive rate to not meet that high bar.

Mon, Apr 2, 7:54 AM
thakis added a comment to D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer.

Actually, it doesn't pass on non-Windows either: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/27665/steps/test/logs/stdio

Mon, Apr 2, 7:28 AM · Restricted Project
thakis added a comment to D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer.

The test added here doesn't pass on Windows, and the change breaks another test on Windows: http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9794

Mon, Apr 2, 7:27 AM · Restricted Project
thakis added a comment to D45085: [AMDGPU][MC][GFX9] Added s_atomic_* and s_buffer_atomic_* instructions.

Reverted in r328978 to green up the bots.

Mon, Apr 2, 7:23 AM
thakis added a comment to D45085: [AMDGPU][MC][GFX9] Added s_atomic_* and s_buffer_atomic_* instructions.

Looks like this made tablegen crash: http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-buildserver/builds/21791/steps/ninja%20build%20local/logs/stdio

Mon, Apr 2, 7:14 AM
thakis closed D45155: Assume existence of inttypes.h and stdint.h in DataTypes.h..

r328970, thanks!

Mon, Apr 2, 6:25 AM

Sun, Apr 1

thakis added inline comments to D30261: [Support] Provide linux/magic.h fallback for older kernels.
Sun, Apr 1, 6:56 PM
thakis updated the diff for D45155: Assume existence of inttypes.h and stdint.h in DataTypes.h..
Sun, Apr 1, 6:08 PM
thakis created D45155: Assume existence of inttypes.h and stdint.h in DataTypes.h..
Sun, Apr 1, 5:31 PM

Fri, Mar 30

thakis closed D45087: [lld-link] Add comment explaining that /FIXED behavior is correct despite contradicting MSDN..

r328879

Fri, Mar 30, 10:20 AM
thakis closed D45091: [lld-link] Let /PROFILE imply /OPT:REF /OPT:NOICF /INCREMENTAL:NO /FIXED:NO.

r328877, thanks!

Fri, Mar 30, 10:18 AM
thakis created D45091: [lld-link] Let /PROFILE imply /OPT:REF /OPT:NOICF /INCREMENTAL:NO /FIXED:NO.
Fri, Mar 30, 7:36 AM
thakis created D45087: [lld-link] Add comment explaining that /FIXED behavior is correct despite contradicting MSDN..
Fri, Mar 30, 6:36 AM

Mar 19 2018

thakis accepted D44630: [ms] Parse #pragma optimize and ignore it behind its own flag.

Awesome, thanks! One nit below:

Mar 19 2018, 12:41 PM

Mar 12 2018

thakis closed D44307: Make lld-link shout at me less..

r327261, thanks!

Mar 12 2018, 5:50 AM
thakis added a comment to D44359: MC intel asm parser: Allow @ at the start of function names..

r327262, thanks!

Mar 12 2018, 5:50 AM
thakis closed D44297: [lld-link] For suppressible warnings, print the suppressing flag..

Maybe we should also print something like

Mar 12 2018, 5:08 AM

Mar 11 2018

thakis accepted D44371: [Driver] Update the comment about incompatible sanitizers.
Mar 11 2018, 4:46 PM

Mar 10 2018

thakis created D44359: MC intel asm parser: Allow @ at the start of function names..
Mar 10 2018, 6:59 PM
thakis added inline comments to D44297: [lld-link] For suppressible warnings, print the suppressing flag..
Mar 10 2018, 11:26 AM

Mar 9 2018

thakis accepted D44320: [sanitizer] Revert rCRT327145.

Thanks!

Mar 9 2018, 12:18 PM
thakis added inline comments to D44261: [sanitizer] Align & pad the allocator structures to the cacheline size.
Mar 9 2018, 11:47 AM
thakis added inline comments to D44297: [lld-link] For suppressible warnings, print the suppressing flag..
Mar 9 2018, 9:56 AM
thakis created D44307: Make lld-link shout at me less..
Mar 9 2018, 8:03 AM
thakis accepted D44302: CMake: Make libxml2 show up in --system-libs (PR36660).
Mar 9 2018, 6:46 AM
thakis created D44297: [lld-link] For suppressible warnings, print the suppressing flag..
Mar 9 2018, 4:52 AM
thakis closed D44286: [lld-link] Add support for /ignore:4037..

r327124, thanks!

Mar 9 2018, 4:43 AM
thakis added inline comments to D44286: [lld-link] Add support for /ignore:4037..
Mar 9 2018, 3:35 AM

Mar 8 2018

thakis created D44286: [lld-link] Add support for /ignore:4037..
Mar 8 2018, 9:05 PM
thakis added a comment to D16403: Add scope information to CFG.

Since this affects analysis-based warnings, have you checked if this patch has any effect on compile times?

Mar 8 2018, 9:50 AM · Restricted Project

Mar 7 2018

thakis closed D44223: [ms] Emit vtordisp initializers in a deterministic order..

r326960, thanks!

Mar 7 2018, 3:21 PM
thakis added inline comments to D44223: [ms] Emit vtordisp initializers in a deterministic order..
Mar 7 2018, 1:53 PM
thakis updated the diff for D44223: [ms] Emit vtordisp initializers in a deterministic order..

rnk comment

Mar 7 2018, 1:50 PM
thakis updated the diff for D44223: [ms] Emit vtordisp initializers in a deterministic order..

sort unstably

Mar 7 2018, 1:14 PM
thakis added inline comments to D44223: [ms] Emit vtordisp initializers in a deterministic order..
Mar 7 2018, 12:22 PM
thakis created D44223: [ms] Emit vtordisp initializers in a deterministic order..
Mar 7 2018, 12:13 PM
thakis accepted D44039: [Sema] Make getCurFunction() return null outside function parsing.

Good luck!

Mar 7 2018, 9:27 AM
thakis accepted D43980: Push a function scope when parsing function bodies without a declaration.

Fix LGTM, one optional comment below.

Mar 7 2018, 9:10 AM

Mar 2 2018

thakis added a comment to D43978: Write a hash of the binary as the PE Debug Directory Timestamp.

Apparently this *still* isn't quite right. The scenario presented to me was this:

  1. User does a clean build, hash X gets written into PDB, hash Y gets written into EXE.
  2. User archives EXE and PDB on symbol store. EXE is in some directory X, PDB is in some directory Y.
  3. User adds some blank lines and rebuilds. EXE doesn't change but PDB does.
  4. User re-archives. EXE is unchanged so doesn't get updated on the symbol store, but PDB does. PDB now gets archived in directory Z.

    So now there is only one EXE, Y, but there are two PDBs, X and Z. If you have the new source code, you will run the EXE, it will find it under directory Y in the sym store, map to the old PDB X, and the source won't match.

I don't understand this scenario. If the pdb changes, doesn't the RSDS header in the exe linking to the binary have to change as well?

Mar 2 2018, 7:26 AM
thakis added a comment to D43978: Write a hash of the binary as the PE Debug Directory Timestamp.

Apparently this *still* isn't quite right. The scenario presented to me was this:

  1. User does a clean build, hash X gets written into PDB, hash Y gets written into EXE.
  2. User archives EXE and PDB on symbol store. EXE is in some directory X, PDB is in some directory Y.
  3. User adds some blank lines and rebuilds. EXE doesn't change but PDB does.
  4. User re-archives. EXE is unchanged so doesn't get updated on the symbol store, but PDB does. PDB now gets archived in directory Z.

    So now there is only one EXE, Y, but there are two PDBs, X and Z. If you have the new source code, you will run the EXE, it will find it under directory Y in the sym store, map to the old PDB X, and the source won't match.
Mar 2 2018, 7:23 AM

Feb 28 2018

thakis closed D43888: [clang-cl] Implement /X.

r326357, thanks!

Feb 28 2018, 11:51 AM
thakis created D43888: [clang-cl] Implement /X.
Feb 28 2018, 11:17 AM

Feb 15 2018

thakis added a comment to D43156: Allow disabling PDB generation in Release build.

So should we revert https://reviews.llvm.org/D42632 for now?

Feb 15 2018, 7:13 AM
thakis added a comment to D42925: Call FlushFileBuffers on readwrite file mappings..

Did this land?

Feb 15 2018, 7:07 AM

Feb 13 2018

thakis closed D43221: Teach Wreturn-type, Wunreachable-code, and alpha.deadcode.UnreachableCode to treat __assume(0) like __builtin_unreachable..

r325052, thanks!

Feb 13 2018, 1:33 PM

Feb 12 2018

thakis created D43221: Teach Wreturn-type, Wunreachable-code, and alpha.deadcode.UnreachableCode to treat __assume(0) like __builtin_unreachable..
Feb 12 2018, 9:11 PM
thakis updated the diff for D42910: not for review, just prototyping some msxml stuff.

some attr merging

Feb 12 2018, 11:56 AM

Feb 9 2018

thakis added a comment to D43110: [Sema] Don't mark plain MS enums as fixed.

Nice!

Feb 9 2018, 5:26 AM

Feb 8 2018

thakis added a comment to D43002: Emit S_OBJNAME symbol in CodeView.

I was about to say "please make sure this honors -fdebug-prefix-map" but I suppose codeview probably contains so many absolute paths at the moment that we should instead have a concerted effort to make debug info cwd-independent at some later point instead.

Feb 8 2018, 6:58 AM
thakis added inline comments to D42632: Generate PDB files for profiling even in Release build.
Feb 8 2018, 6:47 AM

Feb 7 2018

thakis updated the diff for D42910: not for review, just prototyping some msxml stuff.
Feb 7 2018, 10:42 AM
thakis updated the diff for D42910: not for review, just prototyping some msxml stuff.
Feb 7 2018, 9:21 AM
thakis added a comment to D36191: [CodeGen] Don't make availability attributes imply default visibility on macos.

Err sorry, landed in rL310382.

Feb 7 2018, 7:33 AM
thakis added a comment to D36191: [CodeGen] Don't make availability attributes imply default visibility on macos.

What's the status here?

Feb 7 2018, 7:32 AM

Feb 6 2018

thakis added a comment to D34249: [libc++] Don't use UTIME_OMIT to detect utimensat on Apple.

I went ahead and landed this with the comments requested by Duncan in r324385. (http://llvm.org/viewvc/llvm-project?view=revision&revision=324385)

Feb 6 2018, 11:20 AM

Feb 5 2018

thakis added a comment to D41102: Setup clang-doc frontend framework.

This should be in clang-tools-extra next to clang-tidy, clang-include-fixer, clangd etc, not in the main compiler repo, right?

Feb 5 2018, 12:44 PM · Restricted Project
thakis created D42910: not for review, just prototyping some msxml stuff.
Feb 5 2018, 7:55 AM