Page MenuHomePhabricator

rnk (Reid Kleckner)
User

Projects

User Details

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

Recent Activity

Yesterday

rnk added a comment to D67579: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups.

From what I understand, the comdat group isn't necessary, so I disabled it on COFF in r372182. Can we simplify ELF to match?

Tue, Sep 17, 3:20 PM · Restricted Project
rnk added inline comments to D67631: Add AutoUpgrade function to add new address space datalayout string to existing datalayouts..
Tue, Sep 17, 2:34 PM · Restricted Project
rnk committed rG23e872a3d054: [PGO] Don't use comdat groups for counters & data on COFF (authored by rnk).
[PGO] Don't use comdat groups for counters & data on COFF
Tue, Sep 17, 2:12 PM
rnk committed rL372182: [PGO] Don't use comdat groups for counters & data on COFF.
[PGO] Don't use comdat groups for counters & data on COFF
Tue, Sep 17, 2:11 PM
rnk committed rG6f1f3cfc5ac2: Ignore exception specifier mismatch when merging redeclarations (authored by rnk).
Ignore exception specifier mismatch when merging redeclarations
Tue, Sep 17, 1:29 PM
rnk committed rL372178: Ignore exception specifier mismatch when merging redeclarations.
Ignore exception specifier mismatch when merging redeclarations
Tue, Sep 17, 1:28 PM
rnk closed D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode..
Tue, Sep 17, 1:27 PM · Restricted Project, Restricted Project
rnk updated the diff for D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode..
  • simplify merging check
Tue, Sep 17, 1:25 PM · Restricted Project, Restricted Project
rnk added inline comments to D67643: [lit] Extend internal diff to support `-` argument.
Tue, Sep 17, 11:36 AM · Restricted Project
rnk added a comment to D62063: CMake changes to get Windows self-host with PGO working.

@hans has been trying to build clang with PGO on Windows, so I'll defer this to him.

Tue, Sep 17, 11:12 AM · Restricted Project
rnk edited reviewers for D62063: CMake changes to get Windows self-host with PGO working, added: hans; removed: rnk.
Tue, Sep 17, 11:11 AM · Restricted Project
rnk added a comment to D67579: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups.

Hm, this broke check-asan on Windows, which exercises some code coverage features:
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/51563

Tue, Sep 17, 11:03 AM · Restricted Project

Mon, Sep 16

rnk updated the diff for D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode..
  • move test
Mon, Sep 16, 12:54 PM · Restricted Project, Restricted Project
rnk commandeered D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode..

Taking this to move the test around and try the other version...

Mon, Sep 16, 12:53 PM · Restricted Project, Restricted Project
rnk committed rG32837a0c93ec: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups (authored by rnk).
[PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups
Mon, Sep 16, 11:49 AM
rnk committed rL372020: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups.
[PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups
Mon, Sep 16, 11:49 AM
rnk closed D67579: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups.
Mon, Sep 16, 11:49 AM · Restricted Project
rnk added inline comments to D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode..
Mon, Sep 16, 11:43 AM · Restricted Project, Restricted Project

Fri, Sep 13

rnk added a comment to D67579: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups.
In D67579#1670170, @xur wrote:

LGTM. Thanks for the fix!

I wasn't sure why profd was set to internal when looking at PR41380 (as for ELF, all are linkonce_oda). But your description explained that.

BTW, the intrinsic lowering happens before inline (I suppose you are talking the main IPA inline).

Fri, Sep 13, 4:18 PM · Restricted Project
rnk created D67579: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups.
Fri, Sep 13, 3:42 PM · Restricted Project
rnk added a comment to D66843: Change datalayout compatibility check for X86 to allow datalayouts without the new address spaces..

If this is only being used for clang-cl (or -fms-compatibility) mode only, is it possible to limit it to just that rather than it affecting pretty much everything?

Fri, Sep 13, 10:51 AM · Restricted Project

Thu, Sep 12

rnk committed rG0e88ebe11d93: Use host's executable suffix for clang when cross-compiling compiler-rt (authored by rnk).
Use host's executable suffix for clang when cross-compiling compiler-rt
Thu, Sep 12, 11:44 AM
rnk committed rL371754: Use host's executable suffix for clang when cross-compiling compiler-rt.
Use host's executable suffix for clang when cross-compiling compiler-rt
Thu, Sep 12, 11:44 AM
rnk closed D67401: Use host's executable suffix for clang when cross-compiling compiler-rt.
Thu, Sep 12, 11:44 AM · Restricted Project, Restricted Project
rnk added a comment to D67401: Use host's executable suffix for clang when cross-compiling compiler-rt.
In D67401#1665548, @rnk wrote:

lgtm

I don't have commit access, can I ask you to commit this for me? Or is there anything else that needs to be done?

Thu, Sep 12, 11:44 AM · Restricted Project, Restricted Project
rnk committed rGb6a8152b8bf7: [MS] Warn when shadowing template parameters under -fms-compatibility (authored by rnk).
[MS] Warn when shadowing template parameters under -fms-compatibility
Thu, Sep 12, 11:26 AM
rnk committed rL371753: [MS] Warn when shadowing template parameters under -fms-compatibility.
[MS] Warn when shadowing template parameters under -fms-compatibility
Thu, Sep 12, 11:26 AM
rnk closed D67463: [MS] Warn when shadowing template parameters under -fms-compatibility.
Thu, Sep 12, 11:26 AM · Restricted Project, Restricted Project
rnk committed rGb00a49d1b3a1: Don't warn about selectany on implicitly inline variables (authored by rnk).
Don't warn about selectany on implicitly inline variables
Thu, Sep 12, 10:58 AM
rnk committed rL371749: Don't warn about selectany on implicitly inline variables.
Don't warn about selectany on implicitly inline variables
Thu, Sep 12, 10:58 AM
rnk closed D67426: Don't warn about selectany on implicitly inline variables.
Thu, Sep 12, 10:58 AM · Restricted Project

Wed, Sep 11

rnk committed rL371686: Run svn cleanup before svn up on Windows annotated build bots.
Run svn cleanup before svn up on Windows annotated build bots
Wed, Sep 11, 4:18 PM
rnk committed rGe78a7a0ecddc: [TableGen] Skip CRLF conversion when writing output (authored by rnk).
[TableGen] Skip CRLF conversion when writing output
Wed, Sep 11, 3:34 PM
rnk committed rL371683: [TableGen] Skip CRLF conversion when writing output.
[TableGen] Skip CRLF conversion when writing output
Wed, Sep 11, 3:34 PM
rnk committed rGff45955fc868: [X86] Fix latent bugs in 32-bit CMPXCHG8B inserter (authored by rnk).
[X86] Fix latent bugs in 32-bit CMPXCHG8B inserter
Wed, Sep 11, 2:59 PM
rnk committed rL371678: [X86] Fix latent bugs in 32-bit CMPXCHG8B inserter.
[X86] Fix latent bugs in 32-bit CMPXCHG8B inserter
Wed, Sep 11, 2:54 PM
rnk committed rGa685f5161db9: Start porting ivfsoverlay tests to Windows (authored by rnk).
Start porting ivfsoverlay tests to Windows
Wed, Sep 11, 1:57 PM
rnk committed rL371663: Start porting ivfsoverlay tests to Windows.
Start porting ivfsoverlay tests to Windows
Wed, Sep 11, 1:55 PM
rnk closed D67454: Start porting ivfsoverlay tests to Windows.
Wed, Sep 11, 1:55 PM · Restricted Project, Restricted Project
rnk committed rG6d5f0029fc06: [llvm-reduce] Fix a bug, improve error handling when running test (authored by rnk).
[llvm-reduce] Fix a bug, improve error handling when running test
Wed, Sep 11, 1:31 PM
rnk committed rL371653: [llvm-reduce] Fix a bug, improve error handling when running test.
[llvm-reduce] Fix a bug, improve error handling when running test
Wed, Sep 11, 1:27 PM
rnk created D67463: [MS] Warn when shadowing template parameters under -fms-compatibility.
Wed, Sep 11, 1:09 PM · Restricted Project, Restricted Project
rnk created D67454: Start porting ivfsoverlay tests to Windows.
Wed, Sep 11, 11:39 AM · Restricted Project, Restricted Project
rnk committed rGabcc2a879c95: [MS] Consder constexpr globals to be inline, as in C++17 (authored by rnk).
[MS] Consder constexpr globals to be inline, as in C++17
Wed, Sep 11, 11:09 AM
rnk committed rL371642: [MS] Consder constexpr globals to be inline, as in C++17.
[MS] Consder constexpr globals to be inline, as in C++17
Wed, Sep 11, 11:07 AM
rnk closed D47956: [MS] Consder constexpr globals to be inline, as in C++17.
Wed, Sep 11, 11:07 AM · Restricted Project, Restricted Project
rnk added a comment to D66843: Change datalayout compatibility check for X86 to allow datalayouts without the new address spaces..

Still really not a fan of this way. I think you can change the incoming DL in compatibility cases to add an address space when none exists on x86.

-eric

Wed, Sep 11, 9:23 AM · Restricted Project

Tue, Sep 10

rnk accepted D67401: Use host's executable suffix for clang when cross-compiling compiler-rt.

lgtm

Tue, Sep 10, 6:10 PM · Restricted Project, Restricted Project
rnk committed rG7b4237d3ccb6: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI (authored by rnk).
Emit -Wmicrosoft-enum-value warning instead of error in MS ABI
Tue, Sep 10, 6:01 PM
rnk committed rL371581: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.
Emit -Wmicrosoft-enum-value warning instead of error in MS ABI
Tue, Sep 10, 6:00 PM
rnk closed D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.
Tue, Sep 10, 6:00 PM · Restricted Project, Restricted Project
rnk created D67426: Don't warn about selectany on implicitly inline variables.
Tue, Sep 10, 5:47 PM · Restricted Project
rnk added a comment to D47956: [MS] Consder constexpr globals to be inline, as in C++17.
In D47956#1138555, @rnk wrote:

Can we now remove the corresponding MSVC-specific hacks elsewhere (eg, ASTContext::isMSStaticDataMemberInlineDefinition), or do we still need those for const-but-not-constexpr static data members?

We should be able to do that, but unfortunately it drastically changes the diagnostics we emit, as you can see from the tortured ifdefs in my test case updates. I gave up before attempting it.

Tue, Sep 10, 5:26 PM · Restricted Project, Restricted Project
rnk updated the diff for D47956: [MS] Consder constexpr globals to be inline, as in C++17.
  • rebase
Tue, Sep 10, 5:23 PM · Restricted Project, Restricted Project
rnk added a comment to D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.

We have the old TODO of changing this warning to be emitted at enum use time (e.g. when Foo_Val is compared to 0) instead of declaration time. Maybe that's a better fix. Or is implementing that very involved?

Tue, Sep 10, 4:34 PM · Restricted Project, Restricted Project
rnk committed rGc9f5aa99acbb: Actually reorder not and env in crash-recovery-modules.m (authored by rnk).
Actually reorder not and env in crash-recovery-modules.m
Tue, Sep 10, 2:57 PM
rnk committed rL371559: Actually reorder not and env in crash-recovery-modules.m.
Actually reorder not and env in crash-recovery-modules.m
Tue, Sep 10, 2:57 PM
rnk committed rG38e033bf33e8: Re-land Remove REQUIRES:shell from tests that pass for me on Windows (authored by rnk).
Re-land Remove REQUIRES:shell from tests that pass for me on Windows
Tue, Sep 10, 1:15 PM
rnk committed rL371552: Re-land Remove REQUIRES:shell from tests that pass for me on Windows.
Re-land Remove REQUIRES:shell from tests that pass for me on Windows
Tue, Sep 10, 1:15 PM
rnk added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

I hope at least for ELF, you can keep the "_Z" prefix check. It doesn't need the ObjC "___Z" extension.

Tue, Sep 10, 1:07 PM · Restricted Project
rnk added a comment to D66531: [lit] Fix `not` calling internal commands.

I managed to run into this again:
https://reviews.llvm.org/rL371478#687359

Tue, Sep 10, 1:05 PM · Restricted Project
rnk requested changes to D67394: [NFC] Explicitly state that llvm/test/MC/AsmParser/preserve-comments-crlf.s must be CRLF.

Hm, see rL371513. I'm not sure what the correct fix is.

Tue, Sep 10, 10:40 AM · Restricted Project
rnk accepted D67394: [NFC] Explicitly state that llvm/test/MC/AsmParser/preserve-comments-crlf.s must be CRLF.

I see that llvm/.gitattributes already exists, and I assumed that was enough, but I see that it is not. lgtm

Tue, Sep 10, 10:31 AM · Restricted Project

Mon, Sep 9

rnk committed rGa9980f60ce08: Remove REQUIRES:shell from tests that pass for me on Windows (authored by rnk).
Remove REQUIRES:shell from tests that pass for me on Windows
Mon, Sep 9, 5:50 PM
rnk committed rL371478: Remove REQUIRES:shell from tests that pass for me on Windows.
Remove REQUIRES:shell from tests that pass for me on Windows
Mon, Sep 9, 5:50 PM
rnk committed rG87d47cb7c479: Remove some unnecessary REQUIRES: shell lines (authored by rnk).
Remove some unnecessary REQUIRES: shell lines
Mon, Sep 9, 5:07 PM
rnk committed rL371473: Remove some unnecessary REQUIRES: shell lines.
Remove some unnecessary REQUIRES: shell lines
Mon, Sep 9, 5:06 PM
rnk committed rGbf02399a852e: [Windows] Replace TrapUnreachable with an int3 insertion pass (authored by rnk).
[Windows] Replace TrapUnreachable with an int3 insertion pass
Mon, Sep 9, 4:05 PM
rnk committed rL371466: [Windows] Replace TrapUnreachable with an int3 insertion pass.
[Windows] Replace TrapUnreachable with an int3 insertion pass
Mon, Sep 9, 4:05 PM
rnk closed D67201: [Windows] Replace TrapUnreachable with an int3 insertion pass.
Mon, Sep 9, 4:05 PM · Restricted Project
rnk added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
In D67301#1661748, @rnk wrote:

Should the _ prefix (for itanium symbols in i386 mode) be stripped by the COFF wrapper function, or by the common demangle function (matching _Z or __Z)?

Maybe, I forget if the extra _ comes before the __imp_ prefix or after it when dllimported.

It's after the __imp_ prefix, so the demangler could handle it.

But there's another consideration, which is that this code could hypothetically be shared with a MachO linker implementation, where the extra underscore prefix is also present. IMO it's best for any generic demangler to match any number of underscores followed by Z.

I'm not entirely decided on whether I think this is a good thing or not.

The caller knows with certainty whether there should be an extra underscore prefix or not, as it is well defined (always in MachO, never in ELF, plus on i386 in COFF). But if we make the demangler implicitly handle both cases, it can also accidentally e.g. try to demangle any symbol which just starts with a capital Z, without any real underscore prefix, on i386 in COFF or in MachO. (I realize the current code in LLD I just committed also has this issue though.) So if we make it a requirement for the caller to compensate for the prefix, it is less convenient, but also less ambiguous.

WDYT?

Mon, Sep 9, 3:53 PM · Restricted Project
rnk accepted D58694: LLVM: Optimization Pass: Remove conflicting attribute, if any, before adding new read attribute to an argument.

lgtm

Mon, Sep 9, 2:07 PM · Restricted Project
rnk added a comment to D67201: [Windows] Replace TrapUnreachable with an int3 insertion pass.

Thanks!

Mon, Sep 9, 2:05 PM · Restricted Project
rnk accepted D66843: Change datalayout compatibility check for X86 to allow datalayouts without the new address spaces..

Sorry this got lost for two weeks. :(

Mon, Sep 9, 1:51 PM · Restricted Project
rnk requested review of D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.

Ptal, new patch

Mon, Sep 9, 1:41 PM · Restricted Project, Restricted Project
rnk retitled D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI from Unify checking enumerator values in ObjC, C, and MSVC modes to Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.
Mon, Sep 9, 1:40 PM · Restricted Project, Restricted Project
rnk added a comment to D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.

Should there be a test exercising this part? The updated tests both have -fms-compatibility, so were already just warning.

Mon, Sep 9, 1:40 PM · Restricted Project, Restricted Project
rnk updated the diff for D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.
  • rewrite, abandon unification
Mon, Sep 9, 1:39 PM · Restricted Project, Restricted Project

Fri, Sep 6

rnk committed rGa8d3771a318f: Fix thunks.cpp test, don't FileCheck for anon namespace id (authored by rnk).
Fix thunks.cpp test, don't FileCheck for anon namespace id
Fri, Sep 6, 5:41 PM
rnk committed rL371277: Fix thunks.cpp test, don't FileCheck for anon namespace id.
Fix thunks.cpp test, don't FileCheck for anon namespace id
Fri, Sep 6, 5:40 PM
rnk added a comment to D67295: [COFF] Fix to not add archive name to buffer identifiers when they come from thin archives..

Looks right to me, but let's wait for @inlgorion to take a look on Monday.

Fri, Sep 6, 4:12 PM · Restricted Project
rnk added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

Hit send too soon...

Fri, Sep 6, 4:12 PM · Restricted Project
rnk added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

Should the _ prefix (for itanium symbols in i386 mode) be stripped by the COFF wrapper function, or by the common demangle function (matching _Z or __Z)?

Fri, Sep 6, 4:12 PM · Restricted Project
rnk committed rG28328c3771e4: Use musttail for variadic method thunks when possible (authored by rnk).
Use musttail for variadic method thunks when possible
Fri, Sep 6, 3:56 PM
rnk committed rL371269: Use musttail for variadic method thunks when possible.
Use musttail for variadic method thunks when possible
Fri, Sep 6, 3:54 PM
rnk closed D67028: Use musttail for variadic method thunks when possible.
Fri, Sep 6, 3:54 PM · Restricted Project, Restricted Project
rnk updated subscribers of D67283: [GCOV] Skip artificial functions from being emitted.
Fri, Sep 6, 3:45 PM · Restricted Project
rnk added inline comments to D67028: Use musttail for variadic method thunks when possible.
Fri, Sep 6, 3:37 PM · Restricted Project, Restricted Project
rnk created D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI.
Fri, Sep 6, 3:33 PM · Restricted Project, Restricted Project
rnk committed rGe0df2dce4cf6: Remove dead .seh_stackalloc parsing method in X86AsmParser (authored by rnk).
Remove dead .seh_stackalloc parsing method in X86AsmParser
Fri, Sep 6, 1:18 PM
rnk committed rL371251: Remove dead .seh_stackalloc parsing method in X86AsmParser.
Remove dead .seh_stackalloc parsing method in X86AsmParser
Fri, Sep 6, 1:18 PM
rnk added inline comments to D65761: Add Windows Control Flow Guard checks (/guard:cf)..
Fri, Sep 6, 11:28 AM · Restricted Project, Restricted Project
rnk added inline comments to D66625: [X86] Print register names in .seh_* directives.
Fri, Sep 6, 11:06 AM · Restricted Project

Thu, Sep 5

rnk updated subscribers of D65761: Add Windows Control Flow Guard checks (/guard:cf)..

Here is some feedback, I apologize for dragging my feet.

Thu, Sep 5, 5:50 PM · Restricted Project, Restricted Project
rnk accepted D67175: [llvm] [cmake] Add possibility to use ChooseMSVCCRT.cmake when include LLVM library.

lgtm, it's in the LLVM_ namespace, so it couldn't hurt to record it.

Thu, Sep 5, 3:32 PM · Restricted Project
rnk updated the diff for D67028: Use musttail for variadic method thunks when possible.
  • emit an error if we try to clone without a definition
Thu, Sep 5, 3:32 PM · Restricted Project, Restricted Project
rnk added a comment to D67028: Use musttail for variadic method thunks when possible.

In your test case, we hit the early return that I linked to, so we don't try to clone, and we don't need to emit an error.

Yes, that's what happens with the Itanium ABI; what happens with the MS ABI?

Thu, Sep 5, 3:29 PM · Restricted Project, Restricted Project
rnk updated the diff for D67028: Use musttail for variadic method thunks when possible.
  • merge into thunks.cpp
Thu, Sep 5, 3:11 PM · Restricted Project, Restricted Project
rnk added a comment to D67028: Use musttail for variadic method thunks when possible.

I think what I said applies to your test case. Basically, in the Itanium C++ ABI, virtual method definitions provide all their thunks as weak_odr. We only emit thunks referenced by vtables as an optimization, and they are marked available_externally. In your test case, we hit the early return that I linked to, so we don't try to clone, and we don't need to emit an error.

Thu, Sep 5, 2:47 PM · Restricted Project, Restricted Project
rnk added a comment to D67028: Use musttail for variadic method thunks when possible.

Do we have test coverage for a variadic, covariant thunk for a function without a definition? I don't think there's any way for us to actually emit that, but we should make sure the error message is right.

Thu, Sep 5, 1:38 PM · Restricted Project, Restricted Project