Page MenuHomePhabricator

rnk (Reid Kleckner)
User

Projects

User Details

User Since
Jan 2 2013, 4:34 PM (324 w, 21 h)

Recent Activity

Today

rnk committed rG05ea3a6be3ba: Fix lld wasm tests after r356610 (authored by rnk).
Fix lld wasm tests after r356610
Thu, Mar 21, 11:24 AM
rnk committed rLLD356694: Fix lld wasm tests after r356610.
Fix lld wasm tests after r356610
Thu, Mar 21, 11:24 AM
rnk committed rL356694: Fix lld wasm tests after r356610.
Fix lld wasm tests after r356610
Thu, Mar 21, 11:24 AM
rnk added a comment to D59620: [llvm-pdbutil] Add -type-ref-stats to help find unused type info.

@rnk At a later stage, would it be possible to add an option to only print the referenced/unreferenced types? ie. llvm-pdbutil dump -types=[all(default),ref,unref]

Thu, Mar 21, 11:03 AM · Restricted Project
rnk committed rGcda7ff9ddcef: [llvm-pdbutil] Add -type-ref-stats to help find unused type info (authored by rnk).
[llvm-pdbutil] Add -type-ref-stats to help find unused type info
Thu, Mar 21, 11:02 AM
rnk committed rL356692: [llvm-pdbutil] Add -type-ref-stats to help find unused type info.
[llvm-pdbutil] Add -type-ref-stats to help find unused type info
Thu, Mar 21, 11:01 AM
rnk closed D59620: [llvm-pdbutil] Add -type-ref-stats to help find unused type info.
Thu, Mar 21, 11:01 AM · Restricted Project
rnk accepted D59624: [Driver] Pass -malign-double from the driver to the cc1 command line.

lgtm

Thu, Mar 21, 10:57 AM · Restricted Project
rnk added a comment to D59620: [llvm-pdbutil] Add -type-ref-stats to help find unused type info.

The reason I put the variable with the decltype is because it's the easiest way to force the debugger to deal with the cycle. Without that, sure you can generate a cycle in the debug info, but it's not clear under what conditions the debugger would actually require that record.

This way, it's easy to add a watch in the debugger for foo.node and the debugger is forced to resolve the cycle.

Thu, Mar 21, 10:51 AM · Restricted Project
rnk added a comment to D59620: [llvm-pdbutil] Add -type-ref-stats to help find unused type info.

I don't think you can have a cycle to an unnamed type though because it's impossible to reference it.

Thu, Mar 21, 10:29 AM · Restricted Project
rnk added a comment to D59620: [llvm-pdbutil] Add -type-ref-stats to help find unused type info.

Another question - do you think we could replace forward references by the the concrete ones?

I don't think we could replace forward references with the full declarations. Forward references exist to deal with cycles, as well as to make sure the entire type stream can be topologically sorted.

Thu, Mar 21, 10:14 AM · Restricted Project

Yesterday

rnk created D59620: [llvm-pdbutil] Add -type-ref-stats to help find unused type info.
Wed, Mar 20, 4:22 PM · Restricted Project
rnk accepted D59560: Permit redeclarations of a builtin to specify calling convention. .

lgtm

Wed, Mar 20, 3:07 PM · Restricted Project

Tue, Mar 19

rnk added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

I see! I only took a quick look, but this looks exactly like what @rsmith has been asking for in discussions for a long time now: a more explicit AST representation of uuid of uuidof in template arguments. I'll make an effort to get his attention and see if this addresses his concerns.

Tue, Mar 19, 2:49 PM
rnk committed rGe7effeed76e7: Remove MSVC compat hack since the inline keyword was added in 2015 (authored by rnk).
Remove MSVC compat hack since the inline keyword was added in 2015
Tue, Mar 19, 2:41 PM
rnk committed rL356524: Remove MSVC compat hack since the inline keyword was added in 2015.
Remove MSVC compat hack since the inline keyword was added in 2015
Tue, Mar 19, 2:40 PM
rnk added inline comments to D59560: Permit redeclarations of a builtin to specify calling convention. .
Tue, Mar 19, 2:14 PM · Restricted Project
rnk added a comment to D59419: [XCOFF] Add functionality for parsing AIX XCOFF object files header . .

The skeleton of the dumper seems fine.

Tue, Mar 19, 10:09 AM · Restricted Project

Mon, Mar 18

rnk committed rGed350f73c1dc: [asan] Disable -Wfortify-source in intentional OOB tests (authored by rnk).
[asan] Disable -Wfortify-source in intentional OOB tests
Mon, Mar 18, 4:05 PM
rnk committed rL356426: [asan] Disable -Wfortify-source in intentional OOB tests.
[asan] Disable -Wfortify-source in intentional OOB tests
Mon, Mar 18, 4:05 PM
rnk committed rCRT356426: [asan] Disable -Wfortify-source in intentional OOB tests.
[asan] Disable -Wfortify-source in intentional OOB tests
Mon, Mar 18, 4:05 PM
rnk added a comment to D59440: add steps to preprocess file and reduce command line args.

Aside from the buglet, I'm happy with this, but I wanted to wait until @george.burgess.iv takes a look.

Mon, Mar 18, 3:53 PM · Restricted Project
rnk committed rG61c9b7cb9ffb: [MS] Skip vbase construction in abstract class ctors (authored by rnk).
[MS] Skip vbase construction in abstract class ctors
Mon, Mar 18, 3:43 PM
rnk committed rC356425: [MS] Skip vbase construction in abstract class ctors.
[MS] Skip vbase construction in abstract class ctors
Mon, Mar 18, 3:43 PM
rnk committed rL356425: [MS] Skip vbase construction in abstract class ctors.
[MS] Skip vbase construction in abstract class ctors
Mon, Mar 18, 3:43 PM
rnk accepted D58827: [Sema][NFCI] Don't allocate storage for the various CorrectionCandidateCallback unless we are going to do some typo correction.

I think this is worth the complexity of the repeated clone methods. lgtm

Mon, Mar 18, 1:58 PM · Restricted Project
rnk added inline comments to D59440: add steps to preprocess file and reduce command line args.
Mon, Mar 18, 11:57 AM · Restricted Project
rnk accepted D58160: MS ABI: adding template static member in the linker directive section to make sure init function can be called before main..

lgtm

Mon, Mar 18, 11:21 AM · Restricted Project
rnk added a comment to D58160: MS ABI: adding template static member in the linker directive section to make sure init function can be called before main..

Apparently I wrote this comment long ago and never hit send.

Mon, Mar 18, 11:03 AM · Restricted Project

Fri, Mar 15

rnk accepted D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h.

Looks good to me. Let's wait for @jyknight though.

Fri, Mar 15, 3:15 PM · Restricted Project
rnk added a comment to D59411: [LLD][COFF] Improve checkFailIfMismatch.

I think this is a good change. The map on the side is less string-ly typed now. We should wait for @ruiu since it was his review feedback.

Fri, Mar 15, 2:36 PM · Restricted Project
rnk added inline comments to D54802: [LLD][COFF] Generate import modules in PDB.
Fri, Mar 15, 2:11 PM · Restricted Project

Thu, Mar 14

rnk accepted D59261: [LLD][COFF] Separate module descriptors creation from type/symbol merging.

lgtm

Thu, Mar 14, 1:01 PM · Restricted Project
rnk added inline comments to D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.
Thu, Mar 14, 12:48 PM · Restricted Project, Restricted Project
rnk added inline comments to D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.
Thu, Mar 14, 11:16 AM · Restricted Project, Restricted Project
rnk committed rG0f56b22614c8: Add PragmaHandler for MSVC pragma execution_character_set (authored by rnk).
Add PragmaHandler for MSVC pragma execution_character_set
Thu, Mar 14, 11:12 AM
rnk committed rCTE356185: Add PragmaHandler for MSVC pragma execution_character_set.
Add PragmaHandler for MSVC pragma execution_character_set
Thu, Mar 14, 11:12 AM
rnk committed rC356185: Add PragmaHandler for MSVC pragma execution_character_set.
Add PragmaHandler for MSVC pragma execution_character_set
Thu, Mar 14, 11:12 AM
rnk committed rL356185: Add PragmaHandler for MSVC pragma execution_character_set.
Add PragmaHandler for MSVC pragma execution_character_set
Thu, Mar 14, 11:12 AM
rnk closed D58530: Add PragmaHandler for MSVC pragma execution_character_set.
Thu, Mar 14, 11:11 AM · Restricted Project, Restricted Project
rnk added a comment to D59272: [DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare.

Thanks! I haven't read the code in detail, but this all sounds good to me. (/me goes back to cave).

Thu, Mar 14, 9:57 AM · Restricted Project
rnk added a comment to D58662: Handle consecutive-double-quotes in Windows argument parsing.

I agree with your analysis, thanks!

Thu, Mar 14, 9:29 AM · Restricted Project

Wed, Mar 13

rnk added a comment to D59272: [DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare.

@aprantl, I think you are correct for all the code that I can find today. The only thing that matters is the inlinedAt part of the location. But I did see this code that handles dbg.declare, which uses the scope from the location:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L1042

Wed, Mar 13, 4:20 PM · Restricted Project
rnk added inline comments to D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.
Wed, Mar 13, 3:13 PM · Restricted Project, Restricted Project
rnk added a comment to D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.

Very cool! I'll take a look, I wasn't aware this had been rebased and uploaded, I was thinking about doing it myself yesterday as a side project.

Wed, Mar 13, 2:37 PM · Restricted Project, Restricted Project

Tue, Mar 12

rnk added a comment to D59272: [DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare.

This makes me nervous. The pair of the DIVariable and DILocation is used to uniquely identify the variable that a dbg.value applies to. In practice, inlining is usually what creates multiple instances of the same variable that get updated by dbg.values. You can see how the location is used in DwarfDebug::collectVariableInfoFromMFTable.

Tue, Mar 12, 3:15 PM · Restricted Project
rnk added a comment to D59273: [compiler-rt] Windows: fix crt_initializer.cc test uses wrong pragma to declare crt initializer..

I don't think this change is correct, the test is creating a global variable, run_on_startup, and allocating it into .CRT$XIB. There are no initializers involved, which is what #pragma init_seg controls. We'd have to restructure the test like so for that to make sense:

#pragma init_seg(".crt$XIB")
struct AutoIniti { AutoInit() { call_me_maybe(); } } run_on_startup;

This test is essentially rolling its own initializer, so data_seg is the right pragma. Another way to do this would be to use #pragma section / __declspec(allocate).

Tue, Mar 12, 3:05 PM · Restricted Project, Restricted Project
rnk accepted D58544: [AST] Improve support of external layouts in `MicrosoftRecordLayoutBuilder`.

lgtm

Tue, Mar 12, 2:09 PM · Restricted Project, Restricted Project
rnk accepted D57982: [SanitizierCoverage] Avoid splitting critical edges when destination is a basic block containing unreachable.

Looks good, I see why this is needed now.

Tue, Mar 12, 11:11 AM · Restricted Project

Mon, Mar 11

rnk added inline comments to D59118: creduce script for clang crashes.
Mon, Mar 11, 4:59 PM · Restricted Project
rnk updated subscribers of D59118: creduce script for clang crashes.

@gbiv already got all my shell quoting comments.

Mon, Mar 11, 4:09 PM · Restricted Project
rnk added a comment to D59226: [LLD][COFF] DebugTypes prototype -- NOT FOR SUBMIT.

Yep, as a direction, this makes sense to me.

Mon, Mar 11, 3:59 PM · Restricted Project
rnk accepted D59229: Add Swift enumerator value for CodeView::SourceLanguage.

Lgtm!

Mon, Mar 11, 3:31 PM · Restricted Project

Thu, Mar 7

rnk committed rG15846bb5acdc: Fix some clang analysis tests passing arguments incorrectly (authored by rnk).
Fix some clang analysis tests passing arguments incorrectly
Thu, Mar 7, 11:02 AM
rnk committed rL355625: Fix some clang analysis tests passing arguments incorrectly.
Fix some clang analysis tests passing arguments incorrectly
Thu, Mar 7, 10:56 AM
rnk committed rC355625: Fix some clang analysis tests passing arguments incorrectly.
Fix some clang analysis tests passing arguments incorrectly
Thu, Mar 7, 10:56 AM
rnk accepted D58530: Add PragmaHandler for MSVC pragma execution_character_set.

lgtm, thanks! Would you like someone to land this for you?

Thu, Mar 7, 10:45 AM · Restricted Project, Restricted Project

Wed, Mar 6

rnk added inline comments to D59053: [LLD][COFF] Early dependency detection.
Wed, Mar 6, 2:49 PM · Restricted Project

Tue, Mar 5

rnk accepted D58910: [LLD][COFF] More detailed information for /failifmismatch.

lgtm

Tue, Mar 5, 3:10 PM · Restricted Project
rnk accepted D58925: gn build: Port r342002.

lgtm

Tue, Mar 5, 11:46 AM · Restricted Project

Fri, Mar 1

rnk updated the diff for D51664: [IR] Lazily number instructions for local dominance queries.
  • rebase
Fri, Mar 1, 4:11 PM · Restricted Project
rnk accepted D58844: Give builtins and alloc/dealloc operators the default calling convention..

lgtm

Fri, Mar 1, 1:39 PM · Restricted Project, Restricted Project
rnk accepted D58767: [compiler-rt] [CMake] Don't use llvm_replace_compiler_option.

lgtm

Fri, Mar 1, 11:44 AM · Restricted Project, Restricted Project

Thu, Feb 28

rnk added a comment to D58765: [sanitizers] Don't use Windows Trace Logging on MinGW.

Should this be factored into some general define in some header (which one?) like SANITIZER_WIN_TRACE or so, to avoid duplicating the condition all over the place?

Thu, Feb 28, 3:21 PM · Restricted Project, Restricted Project
rnk committed rG701593f1dbff: [sancov] Instrument reachable blocks that end in unreachable (authored by rnk).
[sancov] Instrument reachable blocks that end in unreachable
Thu, Feb 28, 2:55 PM
rnk committed rL355152: [sancov] Instrument reachable blocks that end in unreachable.
[sancov] Instrument reachable blocks that end in unreachable
Thu, Feb 28, 2:53 PM
rnk closed D58740: [sancov] Instrument reachable blocks that end in unreachable.
Thu, Feb 28, 2:53 PM · Restricted Project
rnk committed rG7818144ff344: [COFF] Add address-taken import thunks to the fid table (authored by rnk).
[COFF] Add address-taken import thunks to the fid table
Thu, Feb 28, 1:05 PM
rnk committed rL355141: [COFF] Add address-taken import thunks to the fid table.
[COFF] Add address-taken import thunks to the fid table
Thu, Feb 28, 1:05 PM
rnk committed rLLD355141: [COFF] Add address-taken import thunks to the fid table.
[COFF] Add address-taken import thunks to the fid table
Thu, Feb 28, 1:05 PM
rnk closed D58739: [COFF] Add address-taken import thunks to the fid table.
Thu, Feb 28, 1:05 PM · Restricted Project
rnk accepted D58788: Revert "Revert "[sanitizers] Restore internal_readlink for x32"".

lgtm

Thu, Feb 28, 11:21 AM · Restricted Project, Restricted Project
rnk added a comment to D58651: [NFC][Sanitizer] Pull up GetStackTrace into sanitizer_common.
In D58651#1413884, @yln wrote:

Is there a way to emulate weak linkage on windows?
Can you offer guidance on how to resolve this?

Thu, Feb 28, 9:40 AM · Restricted Project, Restricted Project
rnk accepted D58752: [AArch64] [Windows] Don't skip constructing UnwindHelp..

lgtm

Thu, Feb 28, 9:34 AM · Restricted Project
rnk accepted D58766: [sanitizers] Explicitly use GetModuleFileNameW with wchar_t.

lgtm

Thu, Feb 28, 9:17 AM · Restricted Project, Restricted Project
rnk added a comment to D58767: [compiler-rt] [CMake] Don't use llvm_replace_compiler_option.

I'd prefer to keep the same /Zi vs. /Z7 setting in the standalone build. Does the standalone build not have access to the installed LLVM cmake modules? If so, I would think we could include them. Or, we could put in a basic regex here like we used to have for -DNDEBUG.

Thu, Feb 28, 9:17 AM · Restricted Project, Restricted Project

Wed, Feb 27

rnk added inline comments to D58739: [COFF] Add address-taken import thunks to the fid table.
Wed, Feb 27, 4:38 PM · Restricted Project
rnk added inline comments to D58739: [COFF] Add address-taken import thunks to the fid table.
Wed, Feb 27, 4:29 PM · Restricted Project
rnk accepted D58662: Handle consecutive-double-quotes in Windows argument parsing.

I experimented with this program to confirm the behavior matches MSVCRT:

#include <stdio.h>
extern "C" const char *GetCommandLineA(void);
int main(int argc, char **argv) {
  puts(GetCommandLineA());
  for (int i = 0; i < argc; ++i) {
    puts(argv[i]);
  }
}
Wed, Feb 27, 4:09 PM · Restricted Project
rnk committed rG4fb3502bc9fa: [InstrProf] Use separate comdat group for data and counters (authored by rnk).
[InstrProf] Use separate comdat group for data and counters
Wed, Feb 27, 3:39 PM
rnk committed rC355044: [InstrProf] Use separate comdat group for data and counters.
[InstrProf] Use separate comdat group for data and counters
Wed, Feb 27, 3:39 PM
rnk committed rCRT355044: [InstrProf] Use separate comdat group for data and counters.
[InstrProf] Use separate comdat group for data and counters
Wed, Feb 27, 3:39 PM
rnk committed rL355044: [InstrProf] Use separate comdat group for data and counters.
[InstrProf] Use separate comdat group for data and counters
Wed, Feb 27, 3:39 PM
rnk closed D58737: [InstrProf] Use separate comdat group for data and counters.
Wed, Feb 27, 3:39 PM · Restricted Project, Restricted Project, Restricted Project
rnk added a comment to D58737: [InstrProf] Use separate comdat group for data and counters.
In D58737#1412847, @xur wrote:

lgtm.

BTW, I'm in the process of committing D54175. After that patch, PGO instrumentation can be called after the main inlining. I don't think it will conflict anything in this patch

Wed, Feb 27, 3:27 PM · Restricted Project, Restricted Project, Restricted Project
rnk added inline comments to D58739: [COFF] Add address-taken import thunks to the fid table.
Wed, Feb 27, 3:19 PM · Restricted Project
rnk added a comment to D58737: [InstrProf] Use separate comdat group for data and counters.

Oops, forgot to respond to this...

Wed, Feb 27, 3:18 PM · Restricted Project, Restricted Project, Restricted Project
rnk updated the diff for D58737: [InstrProf] Use separate comdat group for data and counters.
  • share code
Wed, Feb 27, 3:08 PM · Restricted Project, Restricted Project, Restricted Project
rnk added a comment to D58737: [InstrProf] Use separate comdat group for data and counters.
In D58737#1412734, @vsk wrote:

I'm confused by this wording re: comdats in the LangRef: "All global objects that specify this key will only end up in the final object file if the linker chooses that key over some other key". Why can multiple global objects with the same comdat key end up in the final object file?

Also, why is the selection kind here 'any' instead of 'exactmatch'? If the counter array sizes legitimately can vary (why?), then wouldn't we want 'largestsize'?

Wed, Feb 27, 3:04 PM · Restricted Project, Restricted Project, Restricted Project
rnk added a comment to D57982: [SanitizierCoverage] Avoid splitting critical edges when destination is a basic block containing unreachable.
In D57982#1412610, @kcc wrote:

So, apparently, checking for isa<UnreachableInst>(DestBB->getTerminator()) is not the right way to check if the block entry is unreachable.

Thoughts?

Wed, Feb 27, 2:51 PM · Restricted Project
rnk created D58740: [sancov] Instrument reachable blocks that end in unreachable.
Wed, Feb 27, 2:51 PM · Restricted Project
rnk created D58739: [COFF] Add address-taken import thunks to the fid table.
Wed, Feb 27, 2:25 PM · Restricted Project
rnk added a comment to D58599: [LLD] Add summary support.
In D58599#1410086, @aganea wrote:> The following does not produce the expected result - instead the text is half-printed on the console, and the dump file only contains partial information (Discarded symbols and the cmd-line):
lld-link.exe ... /verbose >dump.txt
Wed, Feb 27, 1:56 PM · Restricted Project
rnk created D58737: [InstrProf] Use separate comdat group for data and counters.
Wed, Feb 27, 1:44 PM · Restricted Project, Restricted Project, Restricted Project
rnk accepted D58718: [Memory] Add basic support for large/huge memory pages.

lgtm

Wed, Feb 27, 11:26 AM · Restricted Project
rnk added a comment to D57982: [SanitizierCoverage] Avoid splitting critical edges when destination is a basic block containing unreachable.

Sure, but that block is generally still not useful to instrument (for fuzzing). When fuzzing, we save all inputs that either (1) increase coverage as measured by SanitizerCoverage, or (2) crash. So if case 2 happens every time we touch a block that ends in unreachable, there's no point in instrumenting it so that case 1 happens too.

This explanation looks reasonable to me.

Wed, Feb 27, 11:12 AM · Restricted Project

Tue, Feb 26

rnk added a comment to D58694: LLVM: Optimization Pass: Function Attribute: nocapture should be added if the argument is WriteOnly.

Removing an attribute explicitly added by the user (writeonly) and replacing it with the attribute we deduced (readnone) ? That might not be the improvement we want.
A side note, but the snippet was reduced from an actual code, where the writeonly is required.

Tue, Feb 26, 2:22 PM · Restricted Project
rnk updated subscribers of D57982: [SanitizierCoverage] Avoid splitting critical edges when destination is a basic block containing unreachable.

@vitalybuka @morehouse, any thoughts on this comment:
https://github.com/llvm/llvm-project/blob/29ac3a5b822ba8c097a3ae78d983cdb94da43dd4/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp#L457

Tue, Feb 26, 2:18 PM · Restricted Project
rnk added a comment to D58694: LLVM: Optimization Pass: Function Attribute: nocapture should be added if the argument is WriteOnly.

This seems like the wrong fix, I would expect functionattrs to improve the deduction by removing the writeonly attribute.

Tue, Feb 26, 12:52 PM · Restricted Project
rnk committed rGf9ef9f868c82: [MS] Don't emit coverage for deleting dtors (authored by rnk).
[MS] Don't emit coverage for deleting dtors
Tue, Feb 26, 12:45 PM