rnk (Reid Kleckner)
User

Projects

User Details

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

Recent Activity

Today

rnk added inline comments to D54425: [AArch64] Add aarch64_vector_pcs function attribute to Clang.
Fri, Nov 16, 3:44 PM
rnk added a comment to D54554: [PDB] Add symbol records in bulk.

The main reservation I have about landing this is whether to make it more generic and abstracted. I kind of feel like the main SymbolSerializer class should be powerful enough to do all these optimizations for us, or maybe some replacement for it.

Fri, Nov 16, 3:33 PM
rnk committed rL347111: Add missing test for r347072 -gcodeview-ghash.
Add missing test for r347072 -gcodeview-ghash
Fri, Nov 16, 3:19 PM
rnk committed rC347111: Add missing test for r347072 -gcodeview-ghash.
Add missing test for r347072 -gcodeview-ghash
Fri, Nov 16, 3:19 PM
rnk added inline comments to D54554: [PDB] Add symbol records in bulk.
Fri, Nov 16, 3:18 PM
rnk added inline comments to D54554: [PDB] Add symbol records in bulk.
Fri, Nov 16, 2:47 PM
rnk committed rC347072: [codeview] Expose -gcodeview-ghash for global type hashing.
[codeview] Expose -gcodeview-ghash for global type hashing
Fri, Nov 16, 10:50 AM
rnk committed rL347072: [codeview] Expose -gcodeview-ghash for global type hashing.
[codeview] Expose -gcodeview-ghash for global type hashing
Fri, Nov 16, 10:50 AM
rnk closed D54370: [codeview] Expose -gcodeview-ghash for global type hashing.
Fri, Nov 16, 10:50 AM

Yesterday

rnk added a comment to D54370: [codeview] Expose -gcodeview-ghash for global type hashing.

I collected statistics:
https://bugs.chromium.org/p/chromium/issues/detail?id=904324#c4
The summary is that links in Chromium get 32% faster, but object files grow by 7%. There is no compile time impact. Check it out for more details.

Thu, Nov 15, 4:40 PM
rnk accepted D54587: Fix DynamicLibraryTests build on Windows when LLVM_EXPORT_SYMBOLS_FOR_PLUGINS is ON.

lgtm

Thu, Nov 15, 1:32 PM
rnk added a comment to D54554: [PDB] Add symbol records in bulk.

Very nice! You're talking about /DEBUG:GHASH, but your optimisation is also beneficial when GHASH isn't used?

Thu, Nov 15, 10:30 AM

Wed, Nov 14

rnk created D54554: [PDB] Add symbol records in bulk.
Wed, Nov 14, 4:34 PM
rnk added a comment to D53414: Add instructions for migrating branches from one git repository to another..

I ended up finding these instructions accidentally when searching for other monorepo related stuff, and I followed these instructions, and they worked. Thanks for writing them up!

Wed, Nov 14, 3:39 PM
rnk committed rL346907: [codeview] Make "clang -g" emit codeview by default when targetting MSVC.
[codeview] Make "clang -g" emit codeview by default when targetting MSVC
Wed, Nov 14, 3:02 PM
rnk committed rC346907: [codeview] Make "clang -g" emit codeview by default when targetting MSVC.
[codeview] Make "clang -g" emit codeview by default when targetting MSVC
Wed, Nov 14, 3:02 PM
rnk closed D54499: [codeview] Make "clang -g" emit codeview by default when targetting MSVC.
Wed, Nov 14, 3:02 PM
rnk accepted D54536: [AST] Fix typo in MicrosoftMangle.

I'd say feel free to commit typo fixes like this in the future. I'd avoid fixing typos where there is spelling disagreement, like "prolog vs prologue", so just use your best judgement as always.
lgtm

Wed, Nov 14, 11:15 AM
rnk added a comment to D53876: Preserve loop metadata when splitting exit blocks.

It went green with my change here:
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/1463

Wed, Nov 14, 9:42 AM

Tue, Nov 13

rnk committed rL346823: Revert r346810 "Preserve loop metadata when splitting exit blocks".
Revert r346810 "Preserve loop metadata when splitting exit blocks"
Tue, Nov 13, 5:50 PM
rnk added a comment to D53876: Preserve loop metadata when splitting exit blocks.

I think this broke the windows selfhost:
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/1457/steps/stage%202%20build/logs/stdio

Tue, Nov 13, 5:46 PM
rnk added inline comments to D54499: [codeview] Make "clang -g" emit codeview by default when targetting MSVC.
Tue, Nov 13, 5:27 PM
rnk committed rLLD346817: [PDB] Simplify symbol handling code, NFC.
[PDB] Simplify symbol handling code, NFC
Tue, Nov 13, 3:47 PM
rnk committed rL346817: [PDB] Simplify symbol handling code, NFC.
[PDB] Simplify symbol handling code, NFC
Tue, Nov 13, 3:47 PM
rnk created D54499: [codeview] Make "clang -g" emit codeview by default when targetting MSVC.
Tue, Nov 13, 3:23 PM
rnk added a comment to D54328: Added 'cd -' built in to lit.

As I said: because it makes it harder to rerun parts of the test interactively. Which is a good enough reason to forbid cd - as well as any directory stack operations, IMO.

Tue, Nov 13, 2:50 PM
rnk added a comment to D54328: Added 'cd -' built in to lit.

I'm quite against this. "cd -" is a non-portable extension to shell and should never be used in first place.

Tue, Nov 13, 1:34 PM
rnk added a comment to D53540: [COFF, ARM64] Implement support for SEH extensions __try/__except/__finally.

Sorry, I didn't realize this was waiting on my review. It looks like this is mostly about localescape, so let's change the patch description and code to be about that.

Tue, Nov 13, 10:46 AM
rnk committed rLLD346777: [COFF] Simplify relocation to discarded section diagnostic code, NFC.
[COFF] Simplify relocation to discarded section diagnostic code, NFC
Tue, Nov 13, 10:33 AM
rnk committed rL346777: [COFF] Simplify relocation to discarded section diagnostic code, NFC.
[COFF] Simplify relocation to discarded section diagnostic code, NFC
Tue, Nov 13, 10:33 AM
rnk accepted D54446: Document how to comment an actual parameter.

+1 for writing it as you have it, it's what clang-format does, and so far as I know it's the prevailing style.

Tue, Nov 13, 10:04 AM
rnk requested changes to D54470: Avoid g++ warning spam.

Adding -Wno-cast-function-type seems reasonable, but we should do it more carefully.

Tue, Nov 13, 9:59 AM

Mon, Nov 12

rnk added a comment to D54370: [codeview] Expose -gcodeview-ghash for global type hashing.

@rnk Could you possibly post build metrics for Chromium (for the whole build process, compilation and link) with and without ghash?

Mon, Nov 12, 2:43 PM

Fri, Nov 9

rnk accepted D54328: Added 'cd -' built in to lit.

lgtm

Fri, Nov 9, 5:53 PM
rnk committed rLLD346578: [PDB] Simplify some ghash code, NFC.
[PDB] Simplify some ghash code, NFC
Fri, Nov 9, 5:38 PM
rnk committed rL346578: [PDB] Simplify some ghash code, NFC.
[PDB] Simplify some ghash code, NFC
Fri, Nov 9, 5:38 PM
rnk created D54370: [codeview] Expose -gcodeview-ghash for global type hashing.
Fri, Nov 9, 5:19 PM
rnk committed rCRT346560: Re-land r343606 "[winasan] Unpoison the stack in NtTerminateThread".
Re-land r343606 "[winasan] Unpoison the stack in NtTerminateThread"
Fri, Nov 9, 2:09 PM
rnk committed rL346560: Re-land r343606 "[winasan] Unpoison the stack in NtTerminateThread".
Re-land r343606 "[winasan] Unpoison the stack in NtTerminateThread"
Fri, Nov 9, 2:09 PM
rnk added a comment to D54328: Added 'cd -' built in to lit.

Can you add a quick test to the lit test suite for this? Shouldn't be too hard, make some dirs, cd into and out, pwd, filecheck the output.

Fri, Nov 9, 1:21 PM
rnk accepted D51524: [ARM64] [Windows] Handle funclets.

Sorry, I lost track of this.

Fri, Nov 9, 12:59 PM
rnk accepted D54341: Speed up git-llvm script by only svn up'ing affected directories..

I guess you should update the commit message to say "monorepo-root" instead of "monorepo-top". Otherwise, neat!

Fri, Nov 9, 12:53 PM
rnk added a comment to D52674: [AST] Add Obj-C discriminator to MS ABI RTTI.

Threading a new options argument through mangleType that includes QualifierMangleMode as well as these obj-c options seems reasonable.

Fri, Nov 9, 12:22 PM

Thu, Nov 8

rnk accepted D54248: [COFF, ARM64] Add support for MSVC buffer security check.

lgtm

Thu, Nov 8, 2:53 PM
rnk committed rLLD346431: Fix -Wextra-qualification warning.
Fix -Wextra-qualification warning
Thu, Nov 8, 10:57 AM
rnk committed rL346431: Fix -Wextra-qualification warning.
Fix -Wextra-qualification warning
Thu, Nov 8, 10:57 AM
rnk committed rL346427: [COFF] Improve relocation against discarded section error.
[COFF] Improve relocation against discarded section error
Thu, Nov 8, 10:42 AM
rnk committed rLLD346427: [COFF] Improve relocation against discarded section error.
[COFF] Improve relocation against discarded section error
Thu, Nov 8, 10:42 AM
rnk closed D54240: [COFF] Improve relocation against discarded section error.
Thu, Nov 8, 10:41 AM
rnk updated subscribers of D54153: Fix compilation issue in VS2017 with Clang-tablegen and LLVM-tablegen.

I managed to reproduce the issue with Ninja as well. It happens rarely, 1 out 5 runs.

Thu, Nov 8, 8:44 AM

Wed, Nov 7

rnk committed rL346381: [sancov] Put .SCOV* sections into the right comdat groups on COFF.
[sancov] Put .SCOV* sections into the right comdat groups on COFF
Wed, Nov 7, 5:00 PM
rnk closed D54232: [sancov] Put .SCOV* sections into the right comdat groups on COFF.
Wed, Nov 7, 5:00 PM
rnk created D54240: [COFF] Improve relocation against discarded section error.
Wed, Nov 7, 4:50 PM
rnk updated the diff for D54232: [sancov] Put .SCOV* sections into the right comdat groups on COFF.
  • Elaborate on comdats in a comment
Wed, Nov 7, 4:29 PM
rnk added inline comments to D54232: [sancov] Put .SCOV* sections into the right comdat groups on COFF.
Wed, Nov 7, 4:18 PM
rnk created D54232: [sancov] Put .SCOV* sections into the right comdat groups on COFF.
Wed, Nov 7, 2:43 PM
rnk accepted D52763: Reorder FindPythonInterp so that config-ix can use PYTHON_EXECUTABLE.

I was going to consult @zturner about the state of Python 3 in LLDB, but that's not really related to your change. I think it's fine. lgtm

Wed, Nov 7, 1:10 PM
rnk edited reviewers for D52763: Reorder FindPythonInterp so that config-ix can use PYTHON_EXECUTABLE, added: zturner; removed: rnk.
Wed, Nov 7, 1:08 PM
rnk added a comment to D54187: Add debuginfo-tests that use cdb on Windows.

I hadn't realized that Dexter knew how to drive VS tools. I'll have to go read more and get back to you all.

Wed, Nov 7, 10:48 AM · debug-info

Tue, Nov 6

rnk added a comment to D54182: [Windows] Sink function bodies to .cpp file.

Traditionally, LLDB has resisted efforts to more tightly couple it with LLVM. Many of the contributors at Apple seem to prefer to build LLVM separately and then build against that with the LLDB xcode project. You can see how that would disincentivize you from contributing to LLVM first. And in fairness, sometimes LLVM people object to adding extra complexity to Support when it's only used in LLDB. And, as with lib/DebugInfo/DWARF, sometimes code gets hoisted to LLVM without refactoring LLDB to use it. So, there's plenty of work to go around. :)

Tue, Nov 6, 5:14 PM
rnk added a comment to D54187: Add debuginfo-tests that use cdb on Windows.

It would be nice if instead of having a set of windows-only tests, we could wrap cdb similar to llgdb.py wraps LLDB, so these tests run on all platforms. If the Windows debugger is just too different for this to make sense, let me know.

Tue, Nov 6, 3:56 PM · debug-info
rnk committed rL346280: [Windows] Simplify WindowsSupport.h.
[Windows] Simplify WindowsSupport.h
Tue, Nov 6, 3:42 PM
rnk closed D54182: [Windows] Sink function bodies to .cpp file.
Tue, Nov 6, 3:42 PM
rnk created D54187: Add debuginfo-tests that use cdb on Windows.
Tue, Nov 6, 3:33 PM · debug-info
rnk updated the diff for D54182: [Windows] Sink function bodies to .cpp file.
  • Fix thinko
Tue, Nov 6, 3:17 PM
rnk accepted D54129: [AArch64] [Windows] Trap after noreturn calls..
Tue, Nov 6, 3:11 PM
rnk added a comment to D54129: [AArch64] [Windows] Trap after noreturn calls..

The case I'm trying to fix is functions/funclets which don't have an epilogue at all. As far as I can tell, the x86 only emits SEH_Epilogue before an epilogue; I don't see how it's related.

Tue, Nov 6, 3:11 PM
rnk updated the diff for D54182: [Windows] Sink function bodies to .cpp file.
  • simplify
Tue, Nov 6, 3:10 PM
rnk added inline comments to D54182: [Windows] Sink function bodies to .cpp file.
Tue, Nov 6, 3:04 PM
rnk committed rL346271: Set config.lit_tools_dir, which is needed by lit.llvm.initialize..
Set config.lit_tools_dir, which is needed by lit.llvm.initialize.
Tue, Nov 6, 1:57 PM
rnk accepted D54181: [LoopSink] Do not sink instructions into non-cold blocks.

lgtm with some minor suggestions

Tue, Nov 6, 1:51 PM
rnk created D54182: [Windows] Sink function bodies to .cpp file.
Tue, Nov 6, 1:48 PM
rnk committed rL346268: Silence deprecation warning for GetVersionEx with clang-cl.
Silence deprecation warning for GetVersionEx with clang-cl
Tue, Nov 6, 1:44 PM
rnk committed rC346265: [MS] Zero out ECX in __cpuid in intrin.h.
[MS] Zero out ECX in __cpuid in intrin.h
Tue, Nov 6, 12:49 PM
rnk committed rL346265: [MS] Zero out ECX in __cpuid in intrin.h.
[MS] Zero out ECX in __cpuid in intrin.h
Tue, Nov 6, 12:49 PM
rnk closed D54171: [MS] Zero out ECX in __cpuid in intrin.h.
Tue, Nov 6, 12:49 PM
rnk created D54171: [MS] Zero out ECX in __cpuid in intrin.h.
Tue, Nov 6, 11:41 AM
rnk added a comment to D51524: [ARM64] [Windows] Handle funclets.

It works for x64 and x86, so I guess it's arm-specific. The state table emission code is portable, so maybe the bug is in the arm ip2state mapping logic?

Looked a bit more; it looks like the problem is actually that the "throw" is the last instruction in the function, so unwinding gets confused. There's some code in X86TargetMachine::X86TargetMachine which turns on TrapUnreachable, which avoids the problem. It looks like MSVC does something similar (it inserts a nop instead of a trap, but same idea). I'll post a patch separately.

Tue, Nov 6, 10:12 AM
rnk added a comment to D54129: [AArch64] [Windows] Trap after noreturn calls..

We came up with a different solution for this called SEH_Epilogue. It basically inserts a nop if the last real instruction was a call. It's slightly complicated because it looks backwards across empty basic blocks and things. You can see it here:
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/X86/X86MCInstLower.cpp#L1800

Tue, Nov 6, 10:12 AM
rnk added a comment to D54153: Fix compilation issue in VS2017 with Clang-tablegen and LLVM-tablegen.

The issue is that XXX-tablegen-host .vcxprojects are explicitly calling cmake --build, thus not going through MSBuild's dependency graph. It looks like the Ninja generator doesn't do that.

Tue, Nov 6, 10:06 AM

Mon, Nov 5

rnk added a comment to D51524: [ARM64] [Windows] Handle funclets.

The following testcase is also broken, for reasons I don't understand. I don't think it's a problem with any of the code in this patch, though; the throw out of the "catch" block somehow picks the wrong catch block, but this patch doesn't touch that code.

void a() { try { try { throw 1; } catch (...) { throw 1; } } catch (...) {}}
Mon, Nov 5, 3:53 PM
rnk accepted D54122: [LLD] Fix precompiled headers build break on Linux.

lgtm

Mon, Nov 5, 3:14 PM
rnk added inline comments to D54062: [COFF, ARM64] Implement InterlockedCompareExchange*_* builtins.
Mon, Nov 5, 2:19 PM
rnk added inline comments to D54062: [COFF, ARM64] Implement InterlockedCompareExchange*_* builtins.
Mon, Nov 5, 11:40 AM

Fri, Nov 2

rnk committed rL346062: [codeview] Let the X86 backend tell us the VFRAME offset adjustment.
[codeview] Let the X86 backend tell us the VFRAME offset adjustment
Fri, Nov 2, 5:44 PM
rnk committed rL346060: Update debuginfo tests lit for r341135.
Update debuginfo tests lit for r341135
Fri, Nov 2, 5:25 PM
rnk committed rL346059: [debuginfo-tests] Avoid "import commands" which was deprecated in Py3.
[debuginfo-tests] Avoid "import commands" which was deprecated in Py3
Fri, Nov 2, 5:18 PM
rnk accepted D53727: Only call FlushFileBuffers when writing executables.

Looks good, but when you commit it, you probably will want to keep an eye out for warnings or build breakages in other configurations. This code will be compiled with GCC, MSVC, and clang, and I can imagine a few ways the WindowsSupport.h code could raise warnings.

Fri, Nov 2, 2:27 PM
rnk added inline comments to D51524: [ARM64] [Windows] Handle funclets.
Fri, Nov 2, 2:24 PM
rnk added a comment to D51524: [ARM64] [Windows] Handle funclets.

Reid, do you have an opinion on what test coverage is necessary here?

Fri, Nov 2, 2:16 PM
rnk updated subscribers of D53912: [Headers] [MS] Add intrin0.h.

I agree. This currently resolves issues with building for ARM64 target using Visual Studio 2017. The missing intrinsics it complains about are already present in intrin.h. We could add those to intrin0.h too but that would duplicate. Please let me know what you think.

Fri, Nov 2, 2:04 PM
rnk accepted D54046: [COFF, ARM64] Implement InterlockedExchange*_* builtins.

lgtm

Fri, Nov 2, 1:57 PM

Thu, Nov 1

rnk committed rL345894: Silence -Wimplicit-fallthrough in gold plugin.
Silence -Wimplicit-fallthrough in gold plugin
Thu, Nov 1, 2:27 PM
rnk added inline comments to D53992: [DebugInfo] Correctly sink DBG_VALUEs in postra-machine-sink.
Thu, Nov 1, 1:54 PM
rnk committed rL345887: Enable -Wimplicit-fallthrough for clang as well as GCC.
Enable -Wimplicit-fallthrough for clang as well as GCC
Thu, Nov 1, 1:34 PM
rnk committed rL345883: [Hexagon] Remove unintended fallthrough from MC duplex code.
[Hexagon] Remove unintended fallthrough from MC duplex code
Thu, Nov 1, 1:01 PM
rnk closed D53991: [Hexagon] Remove unintended fallthrough from MC duplex code.
Thu, Nov 1, 1:01 PM
rnk committed rLLD345882: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC.
Fix clang -Wimplicit-fallthrough warnings across llvm, NFC
Thu, Nov 1, 12:58 PM
rnk committed rLLDB345882: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC.
Fix clang -Wimplicit-fallthrough warnings across llvm, NFC
Thu, Nov 1, 12:58 PM
rnk committed rCTE345882: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC.
Fix clang -Wimplicit-fallthrough warnings across llvm, NFC
Thu, Nov 1, 12:58 PM