rnk (Reid Kleckner)
User

Projects

User Details

User Since
Jan 2 2013, 4:34 PM (298 w, 4 d)

Recent Activity

Wed, Sep 19

rnk added a comment to D52283: [PDB] Add the ability to resolve forward references.

It’s true that nested containers don’t usually perform well, but in this
case the outer vector *never* resizes. So it seems like it should be ok?

Wed, Sep 19, 4:23 PM
rnk accepted D52279: [sanitizer] Make __sanitizer::CheckFailed not public.

I ran the tests with your patch and everything passes, but of course, Timur mentioned that nothing in our test suite finds this issue. He probably encountered it in Chrome and it was hard to reduce into something we could add to the test suite.

Wed, Sep 19, 4:12 PM
rnk added inline comments to D52283: [PDB] Add the ability to resolve forward references.
Wed, Sep 19, 4:08 PM
rnk accepted D51700: [fuzzer] Replace FuzzerExtFunctionsDlsymWin.cpp with FuzzerExtFunctionsWeakAlias.cpp.

lgtm

Wed, Sep 19, 1:42 PM
rnk added a comment to D51700: [fuzzer] Replace FuzzerExtFunctionsDlsymWin.cpp with FuzzerExtFunctionsWeakAlias.cpp.

Should I remove FuzzerExtFunctionsDlsymWin.cpp?
I'm unsure about doing so because in Marcos's patch he didn't remove FuzzerExtFunctionsWeakAlias.cpp and because I may find how use it properly later, in which case removing it might make the history uglier?

Wed, Sep 19, 1:42 PM
rnk accepted D52091: [winasan] Unpoison the stack in NtTerminateThread.

lgtm

Wed, Sep 19, 1:40 PM
rnk committed rL342587: Replace clang-x86-windows-msvc2015 with an x64 VS 2017 build script.
Replace clang-x86-windows-msvc2015 with an x64 VS 2017 build script
Wed, Sep 19, 1:39 PM
rnk closed D51746: Replace clang-x86-windows-msvc2015 with an x64 VS 2017 build script.
Wed, Sep 19, 1:39 PM
rnk added a comment to D43881: Add CMake option for using /DEBUG:GHASH.

Let's land this, I want to try it out. :)

Wed, Sep 19, 1:33 PM

Tue, Sep 18

rnk updated the diff for D51664: [IR] Lazily number instructions for local dominance queries.
  • Fix one last use of setValueSubclassData
Tue, Sep 18, 6:23 PM
rnk added inline comments to D51664: [IR] Lazily number instructions for local dominance queries.
Tue, Sep 18, 6:22 PM
rnk updated the diff for D51664: [IR] Lazily number instructions for local dominance queries.
  • Use private bitfield for subclass data
  • Change cached order invalidation
  • Add tests for removal and erasure
Tue, Sep 18, 6:18 PM
rnk added a comment to D52193: RFC: [clang] Multithreaded compilation support.

This is pretty cool. The process launching APIs in LLVM were pretty basic, left a lot to be desired, returned ints, etc etc. This addresses a lot of that.

Tue, Sep 18, 5:18 PM
rnk committed rC342516: [MS] Defer dllexport inline friend functions like other inline methods.
[MS] Defer dllexport inline friend functions like other inline methods
Tue, Sep 18, 4:20 PM
rnk committed rL342516: [MS] Defer dllexport inline friend functions like other inline methods.
[MS] Defer dllexport inline friend functions like other inline methods
Tue, Sep 18, 4:20 PM
rnk accepted D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.

Thanks, I think this looks good with a minor tweak to the test.

Tue, Sep 18, 11:26 AM · debug-info
rnk added inline comments to D52217: [codeview] Emit S_FRAMEPROC and use S_DEFRANGE_FRAMEPOINTER_REL.
Tue, Sep 18, 11:19 AM
rnk added inline comments to D52217: [codeview] Emit S_FRAMEPROC and use S_DEFRANGE_FRAMEPOINTER_REL.
Tue, Sep 18, 10:46 AM
rnk accepted D52235: [COFF] Emit @feat.00 on 64-bit and set the CFG bit when emitting guardcf tables.

lgtm, thanks!

Tue, Sep 18, 9:25 AM

Mon, Sep 17

rnk added a comment to D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.

Needs tests, but I'm on board with this and the implementation.

Mon, Sep 17, 7:24 PM · debug-info
rnk added a comment to D52119: [SanitizerCoverage] Don't make sancov module constructor comdat.

I think we need to back up and think about what we're doing where and why.

Mon, Sep 17, 7:17 PM
rnk created D52217: [codeview] Emit S_FRAMEPROC and use S_DEFRANGE_FRAMEPOINTER_REL.
Mon, Sep 17, 6:35 PM
rnk committed rL342430: Work around grep vs. CRLF issue in Thumb2 test by matching excess whitespace.
Work around grep vs. CRLF issue in Thumb2 test by matching excess whitespace
Mon, Sep 17, 5:05 PM
rnk accepted D52184: Remove dead function user_cache_directory().

Thanks! It's very likely we can use getenv("USERPROFILE") or the unicode equivalent to eliminate the remaining one. It's also pretty reasonable to just hardcode %USERPROFILE%/AppData/Local if we ever need this functionality.

Mon, Sep 17, 1:13 PM
rnk accepted D52188: [MC] Avoid inlining constant symbols with variants..

lgtm

Mon, Sep 17, 1:10 PM

Fri, Sep 14

rnk added a comment to D51664: [IR] Lazily number instructions for local dominance queries.

@gbiv suggested that I test this on a large codebase, so I went ahead and built the Chrome unit_tests target with this, and the validation checks passed.

Fri, Sep 14, 2:44 PM
rnk committed rL342285: [codeview] Remove dead code.
[codeview] Remove dead code
Fri, Sep 14, 2:15 PM
rnk committed rL342282: Mark index-tools.test as REQUIRES: shell so that it does not run with the….
Mark index-tools.test as REQUIRES: shell so that it does not run with the…
Fri, Sep 14, 1:54 PM
rnk committed rCTE342282: Mark index-tools.test as REQUIRES: shell so that it does not run with the….
Mark index-tools.test as REQUIRES: shell so that it does not run with the…
Fri, Sep 14, 1:54 PM
rnk committed rC342281: Relax assumption about default method calling convention in new test.
Relax assumption about default method calling convention in new test
Fri, Sep 14, 1:54 PM
rnk committed rL342281: Relax assumption about default method calling convention in new test.
Relax assumption about default method calling convention in new test
Fri, Sep 14, 1:54 PM
rnk committed rL342272: Remove unused DIASession field.
Remove unused DIASession field
Fri, Sep 14, 1:17 PM
rnk committed rL342265: Revert r342183 "[DAGCombine] Fix crash when store merging created an….
Revert r342183 "[DAGCombine] Fix crash when store merging created an…
Fri, Sep 14, 12:41 PM
rnk committed rL342260: Revert r342210 "[ARM] bottom-top mul support in ARMParallelDSP".
Revert r342210 "[ARM] bottom-top mul support in ARMParallelDSP"
Fri, Sep 14, 11:47 AM

Thu, Sep 13

rnk added a comment to D51829: [MachineInstr] In addRegisterKilled and addRegisterDead, don't remove operands from inline assembly instructions if they have an associated flag operand..

Right, sorry about that. :)

Thu, Sep 13, 2:24 PM
rnk requested changes to D51829: [MachineInstr] In addRegisterKilled and addRegisterDead, don't remove operands from inline assembly instructions if they have an associated flag operand..

lgtm

Thu, Sep 13, 1:16 PM
rnk accepted D52051: [LLD] [COFF] Avoid copying of chunk vectors. NFC..

lgtm

Thu, Sep 13, 1:03 PM

Wed, Sep 12

rnk added a comment to D51664: [IR] Lazily number instructions for local dominance queries.

Any opinions on this? I'm eager to get it in so I can release it and get some faster builds, but I know it's a core data structure change.

Wed, Sep 12, 2:04 PM
rnk added a comment to D51882: [Sanitizers] [MinGW] Produce undecorated symbols for /export: directives when in MinGW mode.

I'm just waiting to hear from other sanitizer maintainers about their stance on adding the complexity of supporting mingw headers in the sanitizers.

Wed, Sep 12, 1:50 PM
rnk added a comment to D51958: [PDB] Emit old fpo data to the PDB.

Needs tests? Maybe they just aren't in the diff?

Wed, Sep 12, 1:08 PM

Tue, Sep 11

rnk added inline comments to D51958: [PDB] Emit old fpo data to the PDB.
Tue, Sep 11, 5:14 PM
rnk committed rL342002: [cmake] Speed up check-llvm 5x by delay loading shell32 and ole32.
[cmake] Speed up check-llvm 5x by delay loading shell32 and ole32
Tue, Sep 11, 3:28 PM
rnk closed D51952: [cmake] Speed up check-llvm 5x by delay loading shell32 and ole32.
Tue, Sep 11, 3:27 PM
rnk accepted D51951: [PDB] Write FPO Data to the PDB.

We should add old FPO data dumping next.

Tue, Sep 11, 3:21 PM
rnk committed rL342000: Apply local fixes intended to be part of r341999.'.
Apply local fixes intended to be part of r341999.'
Tue, Sep 11, 3:03 PM
rnk committed rLLD341999: [codeview] Decode and dump FP regs from S_FRAMEPROC records.
[codeview] Decode and dump FP regs from S_FRAMEPROC records
Tue, Sep 11, 3:03 PM
rnk committed rL341999: [codeview] Decode and dump FP regs from S_FRAMEPROC records.
[codeview] Decode and dump FP regs from S_FRAMEPROC records
Tue, Sep 11, 3:03 PM
rnk closed D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records.
Tue, Sep 11, 3:03 PM
rnk created D51952: [cmake] Speed up check-llvm 5x by delay loading shell32 and ole32.
Tue, Sep 11, 2:50 PM
rnk updated the diff for D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records.
  • use less ambiguous name
  • pick a default CPU type for simplicity
Tue, Sep 11, 2:43 PM
rnk added inline comments to D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records.
Tue, Sep 11, 2:43 PM
rnk added a comment to D51941: [Support] Avoid calling CommandLineToArgvW from shell32.dll.

Unfortunately, it looks like this changed the behavior of newlines embedded in arguments. When parsing response files, we want to treat \n as whitespace. When parsing a command line, CommandLineToArgvW apparently does not. The documentation says it separates arguments on "whitespace characters", but it doesn't say what it considers to be whitespace. Apparently \n is not whitespace.

Tue, Sep 11, 2:31 PM
rnk updated subscribers of D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records.
Tue, Sep 11, 2:13 PM
rnk committed rL341992: [Support] Quote arguments containing \n on Windows.
[Support] Quote arguments containing \n on Windows
Tue, Sep 11, 2:06 PM
rnk committed rL341988: [Support] Avoid calling CommandLineToArgvW from shell32.dll.
[Support] Avoid calling CommandLineToArgvW from shell32.dll
Tue, Sep 11, 1:23 PM
rnk closed D51941: [Support] Avoid calling CommandLineToArgvW from shell32.dll.
Tue, Sep 11, 1:23 PM
rnk accepted D51914: [ASan] [Windows] Avoid including windows.h in asan_malloc_win.cc.

lgtm

Tue, Sep 11, 1:16 PM
rnk added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Tue, Sep 11, 1:16 PM
rnk updated the diff for D51941: [Support] Avoid calling CommandLineToArgvW from shell32.dll.
  • Be more string-y
Tue, Sep 11, 1:09 PM
rnk added a comment to D51941: [Support] Avoid calling CommandLineToArgvW from shell32.dll.

When you commit this, make sure to include in the commit message how much speedup you're seeing as a result of this.

Tue, Sep 11, 1:03 PM
rnk accepted D51940: [X86] Teach X86FastISel::X86SelectRet to use EAX for the sret pointer in GNUX32.

lgtm

Tue, Sep 11, 10:44 AM
rnk created D51941: [Support] Avoid calling CommandLineToArgvW from shell32.dll.
Tue, Sep 11, 10:38 AM

Mon, Sep 10

rnk added a comment to D51746: Replace clang-x86-windows-msvc2015 with an x64 VS 2017 build script.

I don't really know how any of this stuff works, but do we need the LLVM_ENABLE_PDB=ON?

Mon, Sep 10, 5:04 PM
rnk added a comment to D50166: [ARM64] [Windows] MCLayer support for exception handling.

I like the code, but I think this needs more tests, and better testing infrastructure before we can commit it. I would recommend beefing up COFFDumper::printUnwindInfo in llvm-readobj to print the ARM64 unwind opcodes in a human readable format, and then you won't need to write tests with hexadecimal CHECKs like you have now.

Mon, Sep 10, 5:01 PM
rnk added a comment to D51829: [MachineInstr] In addRegisterKilled and addRegisterDead, don't remove operands from inline assembly instructions if they have an associated flag operand..

Sounds good, it could use an IR test though.

Mon, Sep 10, 4:45 PM
rnk added a comment to D51879: [ASan] [MinGW] Avoid including headers for functions that will be redefined.

ಠ_ಠ mm_malloc.h, you are a source of much complexity.

Mon, Sep 10, 4:40 PM
rnk accepted D51883: [Sanitizers] [MinGW] Check for __i386__ in addition to _M_IX86 for i386 specific details.
Mon, Sep 10, 4:35 PM
rnk added a comment to D51885: [CMake] [MinGW] Build address sanitizer for MinGW.

I should add more reviewers, since adding a mingw port is not a small matter. I *think* that some of the sanitizer_common code already builds with mingw gcc for tsan-go, right? So, adding support for building with gcc on Windows should not be too onerous for the project as a whole. At least, that's what I think. Other folks can chime in.

Mon, Sep 10, 4:34 PM
rnk added reviewers for D51885: [CMake] [MinGW] Build address sanitizer for MinGW: morehouse, kcc, dvyukov.
Mon, Sep 10, 4:33 PM
rnk added a comment to D51887: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

Sequential file access might be fast enough, so I don't know which is better performance-wise. In such situation, I'd probably try to implement an easier one (hashing a mmap'ed buffer) to see if it is satisfactory, or try to implement both to compare if not, to keep it as simple as possible.

Mon, Sep 10, 4:33 PM
rnk accepted D51876: [ASan] [Windows] Remove const from _msize function declaration parameter.

lgtm

Mon, Sep 10, 4:23 PM
rnk added a comment to D51887: lld-link: Set PDB GUID to hash of PDB contents instead of to a random byte sequence..

I wonder if you can just compute a hash for a mmap'ed PDB file instead of defining a new type of Stream. That's what we do for ELF when computing a build-id, and I found that's easy to do. It is also easy to parallelize hash computation by making it a tree hash.

Mon, Sep 10, 4:22 PM
rnk added a comment to D51746: Replace clang-x86-windows-msvc2015 with an x64 VS 2017 build script.

Thoughts?

Mon, Sep 10, 4:10 PM
rnk added a comment to D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records.

Actually +@zturner this time.

Mon, Sep 10, 3:39 PM
rnk added a reviewer for D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records: zturner.
Mon, Sep 10, 3:39 PM
rnk created D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records.
Mon, Sep 10, 3:31 PM
rnk added a comment to D51885: [CMake] [MinGW] Build address sanitizer for MinGW.

Does it build with GCC, clang, or both?

Mon, Sep 10, 1:27 PM
rnk accepted D51877: [compiler-rt] [Windows] Include BaseTsd.h with lowercase.

Thanks!

Mon, Sep 10, 1:25 PM
rnk accepted D51842: [X86ISel] Implement byval lowering for Win64 calling convention.

lgtm with a tweaked comment. Thanks!

Mon, Sep 10, 11:50 AM
rnk added a comment to D51695: [MC] [AsmParser]: Ensure a new CFI frame is not opened within an existing one when using cc1as (causing a crash due to failed assert)..

This is a testcase for the crash, patch fixes it, though I'll amend it to work on MCStreamer level as per what @rnk suggested. I'm going to sound really stupid asking but where should the testcase be, is it a FileCheck based one? I just made two targets, one with pre-patch toolchain and one with current (my working tree) toolchain which handles this just fine. First one crashes, second one doesn't, not sure where tests like that go.

Mon, Sep 10, 11:43 AM

Fri, Sep 7

rnk committed rL341727: [COFF] Implement llvm.global_ctors priorities for MSVC COFF targets.
[COFF] Implement llvm.global_ctors priorities for MSVC COFF targets
Fri, Sep 7, 4:09 PM
rnk closed D51820: [COFF] Implement llvm.global_ctors priorities for MSVC COFF targets.
Fri, Sep 7, 4:09 PM
rnk updated the diff for D51820: [COFF] Implement llvm.global_ctors priorities for MSVC COFF targets.
  • beef up test
Fri, Sep 7, 4:08 PM
rnk updated the summary of D51820: [COFF] Implement llvm.global_ctors priorities for MSVC COFF targets.
Fri, Sep 7, 3:36 PM
rnk created D51820: [COFF] Implement llvm.global_ctors priorities for MSVC COFF targets.
Fri, Sep 7, 3:32 PM
rnk committed rL341717: [benchmark] Fix flags used to compile benchmark library with clang-cl.
[benchmark] Fix flags used to compile benchmark library with clang-cl
Fri, Sep 7, 2:48 PM
rnk committed rL341716: [benchmark] Re-enable benchmarks on all platforms including Windows.
[benchmark] Re-enable benchmarks on all platforms including Windows
Fri, Sep 7, 2:48 PM
rnk committed rL341712: [codeview] Add .cv_string directive for testing purposes.
[codeview] Add .cv_string directive for testing purposes
Fri, Sep 7, 2:34 PM
rnk added a comment to D51695: [MC] [AsmParser]: Ensure a new CFI frame is not opened within an existing one when using cc1as (causing a crash due to failed assert)..

Passing the source location through to MCStreamer seems like a good idea. Before rL315264, this code used to use report_fatal_error. My goal was to make it less crummy, and make sure errors get diagnosed when emitting assembly or emitting object files.

Fri, Sep 7, 2:03 PM
rnk accepted D51788: [winasan] Reduce hotpatch prefix check to 8 bytes.

I see that we've done this before in rL310419. This was originally added in rL275180. It'd be nice to figure out what tools generate this nop so we can enumerate all the patterns, but for now let's just do this.

Fri, Sep 7, 1:57 PM
rnk added a comment to D51552: Libraries added with add_llvm_loadable_module macro to have their component name.

I looked at the documentation for the cmake install command, but I still have no idea what a component is or what effect this option has. Can you elaborate on that?

Fri, Sep 7, 1:02 PM
rnk accepted D51784: ms: Insert $$Z in mangling between directly consecutive parameter packs..

lgtm, thanks! I thought that was going to be more of a pain. :)

Fri, Sep 7, 1:00 PM
rnk accepted D51749: [GlobalISel] Lower dbg.declare into indirect DBG_VALUE.

lgtm

Fri, Sep 7, 12:55 PM
rnk accepted D51804: [CMake] Fix LTO option on Windows.

Wow, three different ways to set the linker. I'm impressed with how many cooks there are in our kitchen.

Fri, Sep 7, 12:54 PM
rnk added a comment to D51695: [MC] [AsmParser]: Ensure a new CFI frame is not opened within an existing one when using cc1as (causing a crash due to failed assert)..

I'd suggest fixing this in MCStreamer::EmitCFIStartProc by having it recover in some way that doesn't lead to future assertions. Also, it should be possible to write a standalone .s test case for this.

Fri, Sep 7, 12:54 PM
rnk committed rL341695: [codeview] Improve readobj FPO dumper and pdbutil register names.
[codeview] Improve readobj FPO dumper and pdbutil register names
Fri, Sep 7, 11:52 AM

Thu, Sep 6

rnk committed rL341604: Fix SampleProf code on LLP64 platforms with stoull.
Fix SampleProf code on LLP64 platforms with stoull
Thu, Sep 6, 4:37 PM
rnk added a comment to D51749: [GlobalISel] Lower dbg.declare into indirect DBG_VALUE.

Because of LiveDebugValues this patch might still work though.

Does GlobalISel support the MMI side table? Otherwise this seems a fine approach.

Thu, Sep 6, 2:07 PM
rnk added a comment to D51720: [COFF] don't mark lazy symbols as used in regular objects.

BTW, it's possible that this is also what's causing our size regression vs. non-LTO builds. If archives add lazy symbols which are marked as being used from a regular object, that might prevent us from discarding them.

Thu, Sep 6, 1:20 PM
rnk accepted D51720: [COFF] don't mark lazy symbols as used in regular objects.

lgtm, thanks!

Thu, Sep 6, 1:19 PM