Page MenuHomePhabricator

rnk (Reid Kleckner)
User

Projects

User Details

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

Recent Activity

Yesterday

rnk added a comment to D62063: CMake changes to get Windows self-host with PGO working.

Seems reasonable to me. I'm surprised CMake doesn't have a general facility for escaping paths used in cflags. I suppose users generally don't add paths, just flags.

Mon, May 20, 4:40 PM · Restricted Project
rnk committed rG1a5cc629debd: [COFF] Store alignment in log2 form, NFC (authored by rnk).
[COFF] Store alignment in log2 form, NFC
Mon, May 20, 3:56 PM
rnk committed rLLD361206: [COFF] Store alignment in log2 form, NFC.
[COFF] Store alignment in log2 form, NFC
Mon, May 20, 3:55 PM
rnk committed rL361206: [COFF] Store alignment in log2 form, NFC.
[COFF] Store alignment in log2 form, NFC
Mon, May 20, 3:55 PM
rnk closed D61698: [COFF] Store alignment in log2 form, NFC.
Mon, May 20, 3:55 PM · Restricted Project
rnk added inline comments to D61698: [COFF] Store alignment in log2 form, NFC.
Mon, May 20, 3:54 PM · Restricted Project
rnk updated the diff for D61698: [COFF] Store alignment in log2 form, NFC.
  • assert
Mon, May 20, 3:54 PM · Restricted Project
rnk accepted D62111: Tweaks for setting CMAKE_LINKER to lld-link.

That's old code... I guess nobody has tried building LLVM with asan on Windows in a while.

Mon, May 20, 2:27 PM · Restricted Project

Thu, May 16

rnk committed rG08c15df29f8b: [X86] Deduplicate symbol lowering logic, NFC (authored by rnk).
[X86] Deduplicate symbol lowering logic, NFC
Thu, May 16, 4:14 PM
rnk committed rL360952: [X86] Deduplicate symbol lowering logic, NFC.
[X86] Deduplicate symbol lowering logic, NFC
Thu, May 16, 4:12 PM
rnk closed D61690: [X86] Deduplicate symbol lowering logic, NFC.
Thu, May 16, 4:12 PM · Restricted Project
rnk added a comment to D54747: Discard debuginfo for object files empty after GC.

There's another use case worth considering here, which is -fmodules-debuginfo. In that situation, a standalone object file is emitted that contains nothing but DWARF .debug_info sections, and the object is fed to the linker. If the linker drops it with --gc-sections, it's not going to work. Here's a transcript of me trying to use this feature:
https://reviews.llvm.org/P8150

Thu, May 16, 1:58 PM · Restricted Project, lld
rnk accepted D61926: Emit global variables as S_CONSTANT records for codeview debug info..

lgtm with some nits

Thu, May 16, 12:56 PM · Restricted Project
rnk accepted D62014: [X86] Support .reloc *, R_{386,X86_64}_NONE, *.

This morning I didn't know why this was interesting, but then @inglorion told me about the new .init_array + reloc-none GC sections strategy. I still have concerns about dropping debug info as LLD is doing now, but I'll pass them along separately.

Thu, May 16, 12:51 PM · Restricted Project
rnk updated subscribers of D62014: [X86] Support .reloc *, R_{386,X86_64}_NONE, *.
Thu, May 16, 12:48 PM · Restricted Project
rnk added inline comments to D61690: [X86] Deduplicate symbol lowering logic, NFC.
Thu, May 16, 11:40 AM · Restricted Project

Wed, May 15

rnk added a comment to D61698: [COFF] Store alignment in log2 form, NFC.

By the way, the other idea I had in mind for this problem is to define a new integer-ish class that can hold only a 2^n value, by overriding cast from/to an integer. I don't know if that is a good idea or a disgusting idea, though.

Wed, May 15, 4:38 PM · Restricted Project
rnk removed a reviewer for D61687: Update Python tests for lldb-server on Windows: rnk.
Wed, May 15, 4:37 PM · Restricted Project
rnk added a comment to D61871: [asan] Fix debug asan build link error.

Sorry for the broken patch, I forgot to be extra careful on the codepaths I can't test, clearly. Does this work?

Wed, May 15, 4:29 PM · Restricted Project, Restricted Project
rnk updated the diff for D61871: [asan] Fix debug asan build link error.
  • various fixes to RTEMS / Myriad codepath
Wed, May 15, 4:28 PM · Restricted Project, Restricted Project
rnk added a comment to D61690: [X86] Deduplicate symbol lowering logic, NFC.

Ping, seems like a good simplification.

Wed, May 15, 4:20 PM · Restricted Project
rnk added a comment to D61926: Emit global variables as S_CONSTANT records for codeview debug info..

I was going to ask for a test case that makes local constants, but I see clang generates pretty weird metadata for this test case:

int f(int x) {
  static const int MyConst = 42;
  return x + MyConst;
}

That's supposed to generate a local S_CONSTANT, but we don't populate the DIExpression properly. Hm.

Wed, May 15, 4:18 PM · Restricted Project
rnk added a comment to D54747: Discard debuginfo for object files empty after GC.

I think there's an issue with the whole idea of dropping .debug_info from objects without live sections. Consider:

// a.cpp
int main() {
  return f();
}
// b.cpp
struct Foo {
  int x, y;
};
int f() {
  volatile Foo var;
  var.x = 13;
  var.y = 42;
  return var.x + var.y;
}

When compiled and linked with thinlto, f is imported into the a.cpp TU, but the full definition of the Foo type remains in the b.cpp TU, because importing the full type would be expensive and wasteful.

Wed, May 15, 3:32 PM · Restricted Project, lld
rnk added a comment to D61960: [AArch64] only indicate CFI on Windows if we emitted CFI.
In D61960#1503681, @rnk wrote:

Normally I would go ahead and land this, but I think I should let somebody who has more ownership over the ARM64 backend take a look first.

Wed, May 15, 3:01 PM · Restricted Project
rnk added a comment to D61960: [AArch64] only indicate CFI on Windows if we emitted CFI.

Normally I would go ahead and land this, but I think I should let somebody who has more ownership over the ARM64 backend take a look first.

Wed, May 15, 2:46 PM · Restricted Project
rnk committed rG488249034990: [codeview] Fix SDNode representation of annotation labels (authored by rnk).
[codeview] Fix SDNode representation of annotation labels
Wed, May 15, 2:45 PM
rnk committed rL360818: [codeview] Fix SDNode representation of annotation labels.
[codeview] Fix SDNode representation of annotation labels
Wed, May 15, 2:44 PM
rnk committed rG7c438c5b07a3: [codeview] Finish support for reading and writing S_ANNOTATION records (authored by rnk).
[codeview] Finish support for reading and writing S_ANNOTATION records
Wed, May 15, 1:53 PM
rnk committed rL360813: [codeview] Finish support for reading and writing S_ANNOTATION records.
[codeview] Finish support for reading and writing S_ANNOTATION records
Wed, May 15, 1:51 PM
rnk added inline comments to D61871: [asan] Fix debug asan build link error.
Wed, May 15, 1:48 PM · Restricted Project, Restricted Project
rnk accepted D61960: [AArch64] only indicate CFI on Windows if we emitted CFI.

lgtm

Wed, May 15, 1:43 PM · Restricted Project
rnk added inline comments to D61926: Emit global variables as S_CONSTANT records for codeview debug info..
Wed, May 15, 11:44 AM · Restricted Project

Tue, May 14

rnk accepted D51103: [Support] Add a way to run a function on a detached thread.

Windows parts look good! Sorry for the delay.

Tue, May 14, 4:01 PM · Restricted Project
rnk accepted D61547: [IR] Disallow llvm.global_ctors and llvm.global_dtors of the 2-field form in textual format.

lgtm, thanks for adding the diagnostic!

Tue, May 14, 1:31 PM · Restricted Project
rnk added a comment to rC354839: Revert r354832 "[ASTImporter] Add support for importing ChooseExpr AST nodes.".

This is a destructive revert commit which does not simply revert the mentioned r354832 commit. Besides reverting that commit, it does remove a bunch of unrelated test files! E.g. cfe/trunk/test/ASTMerge/var/Inputs/var1.c. Could you please investigate and restore the unrelated test files?

Tue, May 14, 1:08 PM
rnk committed rGdc2f5f9ff81d: Fix ASTMerge/namespace/test.cpp after r360701 (authored by rnk).
Fix ASTMerge/namespace/test.cpp after r360701
Tue, May 14, 1:00 PM
rnk committed rL360705: Fix ASTMerge/namespace/test.cpp after r360701.
Fix ASTMerge/namespace/test.cpp after r360701
Tue, May 14, 1:00 PM
rnk committed rC360705: Fix ASTMerge/namespace/test.cpp after r360701.
Fix ASTMerge/namespace/test.cpp after r360701
Tue, May 14, 1:00 PM
rnk committed rG2423b7dfd3d2: Update ASTMerge FileCheck test expectations (authored by rnk).
Update ASTMerge FileCheck test expectations
Tue, May 14, 12:01 PM
rnk committed rL360701: Update ASTMerge FileCheck test expectations.
Update ASTMerge FileCheck test expectations
Tue, May 14, 12:00 PM
rnk committed rC360701: Update ASTMerge FileCheck test expectations.
Update ASTMerge FileCheck test expectations
Tue, May 14, 12:00 PM
rnk committed rC360699: Restore test files accidentally deleted in r354839.
Restore test files accidentally deleted in r354839
Tue, May 14, 11:50 AM
rnk committed rG0333dd95636d: Restore test files accidentally deleted in r354839 (authored by rnk).
Restore test files accidentally deleted in r354839
Tue, May 14, 11:50 AM
rnk committed rL360699: Restore test files accidentally deleted in r354839.
Restore test files accidentally deleted in r354839
Tue, May 14, 11:50 AM
rnk removed a reviewer for D60386: FileCheck [6/12]: Introduce numeric variable definition: rnk.
Tue, May 14, 11:30 AM · Restricted Project
rnk removed a reviewer for D60387: FileCheck [7/12]: Arbitrary long numeric expressions: rnk.
Tue, May 14, 11:30 AM · Restricted Project
rnk removed a reviewer for D60385: FileCheck [5/12]: Introduce regular numeric variables: rnk.
Tue, May 14, 11:30 AM · Restricted Project
rnk removed a reviewer for D60390: FileCheck [10/12]: Add support for signed numeric values: rnk.
Tue, May 14, 11:30 AM · Restricted Project
rnk removed a reviewer for D60388: FileCheck [8/12]: Define numeric var from expr: rnk.
Tue, May 14, 11:30 AM · Restricted Project
rnk removed a reviewer for D60389: FileCheck [9/12]: Add support for matching formats: rnk.
Tue, May 14, 11:30 AM · Restricted Project
rnk removed a reviewer for D60391: FileCheck [11/12]: Add matching constraint specification: rnk.
Tue, May 14, 11:30 AM · Restricted Project
rnk removed a reviewer for D60392: FileCheck [12/12]: Support use of var defined on same line: rnk.
Tue, May 14, 11:30 AM · Restricted Project

Mon, May 13

rnk removed a reviewer for D61686: Enable lldb-server on Windows: rnk.
Mon, May 13, 1:39 PM · Restricted Project
rnk accepted D61742: [Driver][Windows] Add dependent lib argument for profile instr generate.

lgtm

Mon, May 13, 1:38 PM · Restricted Project
rnk created D61871: [asan] Fix debug asan build link error.
Mon, May 13, 1:35 PM · Restricted Project, Restricted Project
rnk added a comment to D61698: [COFF] Store alignment in log2 form, NFC.

ptal

Mon, May 13, 1:27 PM · Restricted Project
rnk added inline comments to D47916: [asan, myriad] Use local pool for new/delete when ASan run-time is not up.
Mon, May 13, 11:14 AM · Restricted Project

Fri, May 10

rnk committed rGc10f80eb7b4a: [COFF] Update LLD yaml test cases to include .bss size (authored by rnk).
[COFF] Update LLD yaml test cases to include .bss size
Fri, May 10, 3:11 PM
rnk committed rLLD360476: [COFF] Update LLD yaml test cases to include .bss size.
[COFF] Update LLD yaml test cases to include .bss size
Fri, May 10, 3:10 PM
rnk committed rL360476: [COFF] Update LLD yaml test cases to include .bss size.
[COFF] Update LLD yaml test cases to include .bss size
Fri, May 10, 3:10 PM
rnk committed rG7eb6b5ffc3c7: [COFF] Fix .bss section size bug in obj2yaml / yaml2obj (authored by rnk).
[COFF] Fix .bss section size bug in obj2yaml / yaml2obj
Fri, May 10, 2:52 PM
rnk committed rL360473: [COFF] Fix .bss section size bug in obj2yaml / yaml2obj.
[COFF] Fix .bss section size bug in obj2yaml / yaml2obj
Fri, May 10, 2:51 PM

Thu, May 9

rnk updated subscribers of D61646: Include corecrt.h/vcruntime.h to improve MS compatibility.

@echristo mentioned that this patch was also causing issues in another freestanding environment where _MSC_VER was defined, but no corecrt.h header existed on the system. So, I think it's likely that we don't want to do this in the long run. The compiler-provided headers and system headers are already too entangled, and this is one more dependency between them.

Thu, May 9, 3:22 PM · Restricted Project
rnk updated the diff for D61698: [COFF] Store alignment in log2 form, NFC.
  • remove [gs]etLog2Alignment
Thu, May 9, 3:16 PM · Restricted Project
rnk added inline comments to D61696: [COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC.
Thu, May 9, 3:00 PM · Restricted Project
rnk committed rG4c64256b516b: [COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC (authored by rnk).
[COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC
Thu, May 9, 2:20 PM
rnk committed rLLD360376: [COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC.
[COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC
Thu, May 9, 2:20 PM
rnk committed rL360376: [COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC.
[COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC
Thu, May 9, 2:20 PM
rnk closed D61696: [COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC.
Thu, May 9, 2:20 PM · Restricted Project
rnk added a comment to D61698: [COFF] Store alignment in log2 form, NFC.

In the long term, I think we want to hoist section characteristics up into chunk, and that's going to create the 8192 max alignment constraint. This is just a first step to refactor the code to use the accessors.

Thu, May 9, 2:14 PM · Restricted Project
rnk added a comment to D61742: [Driver][Windows] Add dependent lib argument for profile instr generate.

Thanks, I would like to do this for the sanitizers as well, since this is a constant pain point for users, who have to come up with an architecture-dependent filename (clang_rt.asan-dynamic-$arch.lib or something).

Thu, May 9, 1:57 PM · Restricted Project
rnk accepted D61757: [cmake] Remove MSVC C4355 override.

I noticed you can see these warnings by compiling with /Wall, by the way. It's MSVC's equivalent of Clang's -Weverything. So I guess that's the new next level after /W4.

Thu, May 9, 1:46 PM · Restricted Project
rnk accepted D61730: [cmake] Remove MSVC C4800 override.
Thu, May 9, 1:45 PM · Restricted Project
rnk added a comment to D61730: [cmake] Remove MSVC C4800 override.

I confirmed that this hasn't been in the default warning set for 2017, which these days I guess is the oldest we support. Feel free to remove other off-by-default warnings, I don't think we need to proactively defend against users adding them to their own cflags, or using old msvc versions.

Thu, May 9, 1:35 PM · Restricted Project
rnk accepted D61621: [X86] Make `x86intrin.h`, `immintrin.h` includable with `-fno-gnu-inline-asm`..

Didn't use -fms-compatibility in the test and it seems to be working fine.
Don't know if it is accidental and if I should add the flag.

Thu, May 9, 11:54 AM · Restricted Project

Wed, May 8

rnk committed rG44dd05c31baf: Try to restore some clang test headers lost in r360291 (authored by rnk).
Try to restore some clang test headers lost in r360291
Wed, May 8, 3:29 PM
rnk committed rL360297: Try to restore some clang test headers lost in r360291.
Try to restore some clang test headers lost in r360291
Wed, May 8, 3:28 PM
rnk committed rC360297: Try to restore some clang test headers lost in r360291.
Try to restore some clang test headers lost in r360291
Wed, May 8, 3:28 PM
rnk committed rG55fab1ff4806: Revert Include corecrt.h in stddef.h and vcruntime.h in stdarg.h to improve MS… (authored by rnk).
Revert Include corecrt.h in stddef.h and vcruntime.h in stdarg.h to improve MS…
Wed, May 8, 3:00 PM
rnk committed rC360291: Revert Include corecrt.h in stddef.h and vcruntime.h in stdarg.h to improve MS….
Revert Include corecrt.h in stddef.h and vcruntime.h in stdarg.h to improve MS…
Wed, May 8, 2:59 PM
rnk committed rL360291: Revert Include corecrt.h in stddef.h and vcruntime.h in stdarg.h to improve MS….
Revert Include corecrt.h in stddef.h and vcruntime.h in stdarg.h to improve MS…
Wed, May 8, 2:59 PM
rnk added a comment to D61646: Include corecrt.h/vcruntime.h to improve MS compatibility.

This change broke our build which is using Clang and the x86_64-windows-msvc target to cross-compile our EFI bootloader. Now the compilation fails because it cannot find Windows headers (as expected). We're probably going to work around the issue by setting -U_MSC_VER, but I'm going to start a discussion whether we should maybe define a new target for EFI.

Wed, May 8, 2:59 PM · Restricted Project
rnk added a comment to D61699: [lld-link] initialize targets and asmparsers before invoking lib.

Nice.

Wed, May 8, 2:20 PM · Restricted Project
rnk created D61698: [COFF] Store alignment in log2 form, NFC.
Wed, May 8, 1:55 PM · Restricted Project
rnk created D61696: [COFF] Simplify Chunk::writeTo and remove OutputSectionOff, NFC.
Wed, May 8, 1:50 PM · Restricted Project
rnk added a comment to D61670: [RFC] [MinGW] Allow opting out from .refptr stubs.

Well, I'm curious what meaning GCC ascribes to a medium code model for COFF. Do they generate code to allow PE images larger than 2GB, or is it more like the ELF small code model, where they assume everything can be reached with RIP relative addressing?

Wed, May 8, 11:52 AM · Restricted Project
rnk added a comment to D61646: Include corecrt.h/vcruntime.h to improve MS compatibility.

Did I get the bug right that this adds almost 400kB to every file that includes stddef.h?

Wed, May 8, 11:48 AM · Restricted Project
rnk committed rG1558731607cb: Fix new reassociate-catchswitch.ll test (authored by rnk).
Fix new reassociate-catchswitch.ll test
Wed, May 8, 11:37 AM
rnk committed rL360279: Fix new reassociate-catchswitch.ll test.
Fix new reassociate-catchswitch.ll test
Wed, May 8, 11:37 AM
rnk added a comment to D61670: [RFC] [MinGW] Allow opting out from .refptr stubs.

I'm a little concerned about overloading the code model this way. Currently we have the levels tiny, small, medium, large. Default is the same as small in every way so far as I'm aware. On COFF, the code models are currently untested, so far as I know, and I've been assuming that any usage of the non-small code model more or less implies ELF, and that it would be reasonable to emit wrong code or a fatal error on such codepaths during LLVM codegen. Maybe we have some use cases for the other code models in MCJIT.

Wed, May 8, 11:37 AM · Restricted Project
rnk accepted D61671: X86WinAllocaExpander: Drop code looking through register copies (PR41786).

lgtm, too

Wed, May 8, 11:11 AM · Restricted Project
rnk created D61690: [X86] Deduplicate symbol lowering logic, NFC.
Wed, May 8, 10:54 AM · Restricted Project
rnk added a comment to D61547: [IR] Disallow llvm.global_ctors and llvm.global_dtors of the 2-field form in textual format.

Presumably test/Bitcode/upgrade-global-ctors.ll.bc should remain unchanged, i.e. not be regenerated? We want it to use the pre-upgraded, two field form.

Wed, May 8, 9:30 AM · Restricted Project

Tue, May 7

rnk accepted D61646: Include corecrt.h/vcruntime.h to improve MS compatibility.

lgtm, thanks

Tue, May 7, 4:21 PM · Restricted Project
rnk added a comment to D61621: [X86] Make `x86intrin.h`, `immintrin.h` includable with `-fno-gnu-inline-asm`..

I see one more instance of unprotected asm usage, but it's under #ifdef _MSC_VER, it's the HLE extensions. Can you protect that, and then add another RUN line with a windows triple and -fms-extensions to test it?

Tue, May 7, 4:21 PM · Restricted Project
rnk committed rG6bf108d77a3c: [COFF] Use COFF stubs for extern_weak functions (authored by rnk).
[COFF] Use COFF stubs for extern_weak functions
Tue, May 7, 4:05 PM
rnk committed rL360207: [COFF] Use COFF stubs for extern_weak functions.
[COFF] Use COFF stubs for extern_weak functions
Tue, May 7, 4:05 PM
rnk committed rC360207: [COFF] Use COFF stubs for extern_weak functions.
[COFF] Use COFF stubs for extern_weak functions
Tue, May 7, 4:05 PM
rnk closed D61615: [COFF] Use COFF stubs for extern_weak functions.
Tue, May 7, 4:05 PM · Restricted Project, Restricted Project
rnk added inline comments to D61646: Include corecrt.h/vcruntime.h to improve MS compatibility.
Tue, May 7, 3:46 PM · Restricted Project