rnk (Reid Kleckner)
User

Projects

User Details

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

Recent Activity

Fri, Apr 20

rnk accepted D45894: asan: Mark printf-4.c as unsupported on Windows..

lgtm Thanks for the investigation. I RDPd to the bot and then got distracted before debugging it.

Fri, Apr 20, 11:41 AM
rnk accepted D45857: [ObjCARC] Account for funclet token in storeStrong transform.

I'm unhappy with the need for instrumentation passes to "color" their inserted call instructions. It's a real drag. What we really want here are single-entry, multi-exit regions. If you ignore unreachable, then funclets are already single-entry, single-exit regions. Unfortunately, unreachable is very very common, because it comes after throw, which always calls a noreturn function. So this is common:

try { may_throw(); } catch (...) { do_something(); throw; /*unreachable*/ }
try { may_throw(); } catch (...) { do_something(); throw; /*unreachable*/ }

We needed to prevent simplifycfg from tail merging the two otherwise identical catch funclets that don't end in catchret.

Fri, Apr 20, 11:25 AM

Thu, Apr 19

rnk committed rL330372: Don't do aligned allocations on MSVCRT before 19.12 (update 15.3).
Don't do aligned allocations on MSVCRT before 19.12 (update 15.3)
Thu, Apr 19, 3:17 PM
rnk committed rCXX330372: Don't do aligned allocations on MSVCRT before 19.12 (update 15.3).
Don't do aligned allocations on MSVCRT before 19.12 (update 15.3)
Thu, Apr 19, 3:17 PM
rnk closed D45836: Don't do aligned allocations on MSVCRT before 19.12 (update 15.3).
Thu, Apr 19, 3:17 PM
rnk created D45836: Don't do aligned allocations on MSVCRT before 19.12 (update 15.3).
Thu, Apr 19, 2:31 PM
rnk committed rCXX330360: Remove impossible _MSC_VER check.
Remove impossible _MSC_VER check
Thu, Apr 19, 12:50 PM
rnk accepted D45832: [PDB] Output first section contribution for each module.

lgtm

Thu, Apr 19, 12:49 PM
rnk committed rL330360: Remove impossible _MSC_VER check.
Remove impossible _MSC_VER check
Thu, Apr 19, 12:43 PM
rnk closed D45829: Remove impossible _MSC_VER check.
Thu, Apr 19, 12:43 PM
rnk created D45829: Remove impossible _MSC_VER check.
Thu, Apr 19, 11:39 AM
rnk updated subscribers of rL328180: Fix PR22634 - std::allocator doesn't respect over-aligned types..

+@pcc

Thu, Apr 19, 11:35 AM
rnk added a comment to D45801: COFF: Use (name, output characteristics) as a key when grouping input sections into output sections..

@zturner: We need to add logic to this section merging code to make S_SECTION / S_COFFGROUP records.

Thu, Apr 19, 10:06 AM
rnk updated subscribers of D45801: COFF: Use (name, output characteristics) as a key when grouping input sections into output sections..
Thu, Apr 19, 10:00 AM
rnk added a comment to D45801: COFF: Use (name, output characteristics) as a key when grouping input sections into output sections..

because of the previous
permission merging semantics, the .CRT sections were causing the entire
.rdata section to become writable, which caused the SEH runtime to
crash because it apparently requires .xdata to be read-only. This
change also implements the same special case.

Thu, Apr 19, 9:38 AM

Wed, Apr 18

rnk committed rL330303: [MS] Fix unprototyped thunk emission for incomplete return types.
[MS] Fix unprototyped thunk emission for incomplete return types
Wed, Apr 18, 4:24 PM
rnk committed rC330303: [MS] Fix unprototyped thunk emission for incomplete return types.
[MS] Fix unprototyped thunk emission for incomplete return types
Wed, Apr 18, 4:24 PM
rnk committed rL330300: [COFF] Mark images with no exception handlers for /safeseh.
[COFF] Mark images with no exception handlers for /safeseh
Wed, Apr 18, 3:41 PM
rnk committed rLLD330300: [COFF] Mark images with no exception handlers for /safeseh.
[COFF] Mark images with no exception handlers for /safeseh
Wed, Apr 18, 3:41 PM
rnk closed D45778: [COFF] Mark images with no exception handlers for /safeseh.
Wed, Apr 18, 3:40 PM
rnk added inline comments to D45778: [COFF] Mark images with no exception handlers for /safeseh.
Wed, Apr 18, 3:40 PM
rnk updated the diff for D45778: [COFF] Mark images with no exception handlers for /safeseh.
  • fix boolean logic thinko
Wed, Apr 18, 2:56 PM
rnk updated the diff for D45778: [COFF] Mark images with no exception handlers for /safeseh.
  • fix hello32.test: default to marking image as "no seh", then remove it if we record handlers
Wed, Apr 18, 2:54 PM
rnk updated the diff for D45778: [COFF] Mark images with no exception handlers for /safeseh.
  • move logic to createSEHTable
Wed, Apr 18, 2:45 PM
rnk added inline comments to D45778: [COFF] Mark images with no exception handlers for /safeseh.
Wed, Apr 18, 2:42 PM
rnk accepted D45742: Fix data race in X86FloatingPoint.cpp ASSERT_SORTED.

lgtm

Wed, Apr 18, 1:16 PM
rnk updated the diff for D45778: [COFF] Mark images with no exception handlers for /safeseh.
  • remove tab
Wed, Apr 18, 1:06 PM
rnk added a comment to D45778: [COFF] Mark images with no exception handlers for /safeseh.

What effect does it have if a DLL is marked as not using safe SEH?

Wed, Apr 18, 1:05 PM
rnk updated the diff for D45778: [COFF] Mark images with no exception handlers for /safeseh.
  • Set the flag if there are handlers but no load config
Wed, Apr 18, 1:04 PM
rnk added a comment to D45778: [COFF] Mark images with no exception handlers for /safeseh.

@mstorsjo will this be a problem for mingw? Does the mingw CRT provide a load config for each image? This could be an undesirable behavior change if mingw does not provide a load config, and the user links in a 32-bit object file that has exception handlers that they don't actually need for program correctness, but now the DLL is marked as not using safe SEH.

Wed, Apr 18, 10:51 AM
rnk created D45778: [COFF] Mark images with no exception handlers for /safeseh.
Wed, Apr 18, 10:39 AM

Tue, Apr 17

rnk added a comment to D45504: [MinGW] Look for a cross sysroot relative to the clang binary.

Sorry, I skipped over the message and looked at the code, which seems pretty straightforward.

Tue, Apr 17, 1:51 PM
rnk accepted D45504: [MinGW] Look for a cross sysroot relative to the clang binary.

lgtm

Tue, Apr 17, 1:50 PM
rnk accepted D45673: [x86] Fix PR37100 by teaching the EFLAGS copy lowering to rewrite uses across basic blocks in the limited cases where it is very straight forward to do so..

lgtm

Tue, Apr 17, 9:45 AM

Mon, Apr 16

rnk added a comment to D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..

Could you please put a --reproduce package of those libraries in a bug report? Thanks!

Mon, Apr 16, 5:39 PM
rnk committed rLLD330164: Revert r329960 "Do not keep shared symbols created from garbage-collected….
Revert r329960 "Do not keep shared symbols created from garbage-collected…
Mon, Apr 16, 3:48 PM
rnk committed rL330164: Revert r329960 "Do not keep shared symbols created from garbage-collected….
Revert r329960 "Do not keep shared symbols created from garbage-collected…
Mon, Apr 16, 3:48 PM
rnk added a comment to D45536: Do not keep shared symbols to garbage-collected eliminated DSOs..

This is causing LLD to drop __cxa_finalize from the symbol tables of some Chromium binaries, and that causes them to crash on shutdown. See the test failures here:
https://ci.chromium.org/buildbot/chromium.clang/ToTLinux/2152

Mon, Apr 16, 3:47 PM
rnk accepted D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy.

Mostly looks good.

Mon, Apr 16, 2:32 PM
rnk added inline comments to D45673: [x86] Fix PR37100 by teaching the EFLAGS copy lowering to rewrite uses across basic blocks in the limited cases where it is very straight forward to do so..
Mon, Apr 16, 2:29 PM
rnk added a comment to D45604: Support for multiarch runtimes layout.

The overall goal makes sense to me, but I feel like this patch adds many CMake variables to accomplish it, which are only set from the Fuchsia CMake cache. I feel like it would be simpler to have one CMake flag that clang and the runtime libraries look at to decide if they want to use the old libclang_rt.$arch.$lib paths, or the new clang/$version/$triple/lib/$lib paths.

Mon, Apr 16, 9:54 AM
rnk accepted D45632: [Attributes] Fix a bug in AttributeList::get so it can handle a mix FunctionIndex and ReturnIndex/arg indices at the same time.

Yikes. Thanks for the fix, looks good.

Mon, Apr 16, 9:36 AM

Wed, Apr 11

rnk accepted D45523: [CodeGen] Handle __func__ inside __finally.

lgtm, thanks!

Wed, Apr 11, 10:03 AM
rnk added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Wed, Apr 11, 10:02 AM
rnk accepted D45473: CodeGen: Don't try to canonicalize Unix-style paths in CodeView debug info..

OK, here are the two competing use cases that I am imagining:

  1. (yours) Full cross-compilation from Unix, with linker diagnostics referring to files that exist locally on Unix.
  2. Distributed cross-compilation on Unix, local linking on Windows with the VC linker.
Wed, Apr 11, 9:52 AM
rnk added a comment to D45164: [MC] Change AsmParser to leverage Assembler during evaluation.

I have not found any documentation giving guarantees about the correspondence between output types, but it seem natural to me that direct object generation may be more permissive than compilation through assembly hence this patch. There already appear to be additional restrictions in the AsmStreamer (e.g. dwarf CUID) over the ObjectStreamer so this isn't a new thing.

Wed, Apr 11, 9:36 AM
rnk added a comment to D45122: [DebugInfo] Add a new DI flag to record if a C++ record is a trivial type.

Only considering DIFlagPassByReference for CxxReturnUdt will not cover all
cases. There are times when DIFlagPassByValue is also CxxReturnUdt.

Wed, Apr 11, 9:30 AM
rnk added a comment to D45123: [CodeView] Emit function options for subprogram and member functions.

You're right, this patch does get the constructor member LF right, but it would be easier and more direct if the constructor itself were marked trivial or just omitted by clang.

Wed, Apr 11, 9:24 AM
rnk committed rL329822: [FastISel] Disable local value sinking by default.
[FastISel] Disable local value sinking by default
Wed, Apr 11, 9:06 AM

Tue, Apr 10

rnk added a comment to D45473: CodeGen: Don't try to canonicalize Unix-style paths in CodeView debug info..
In D45473#1063531, @pcc wrote:
In D45473#1063321, @rnk wrote:

Can you elaborate on the use case? What tool do you expect will consume these Unix style paths?

We can use the paths to provide better linker error messages. See D45467.

Tue, Apr 10, 4:11 PM
rnk added a comment to D45123: [CodeView] Emit function options for subprogram and member functions.

I think we actually want the triviality marker on the DISubprogram, not the DICompositeType. If I understand, this patch will still emit LF_METHOD records for trivial, but user-defaulted constructors, like:

struct Foo { Foo() = default; };
Tue, Apr 10, 3:09 PM
rnk added inline comments to D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics.
Tue, Apr 10, 3:04 PM · Restricted Project
rnk accepted D45500: [MinGW] Look for libc++ headers in a triplet prefixed path as well.

lgtm

Tue, Apr 10, 2:50 PM
rnk added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Tue, Apr 10, 2:49 PM
rnk added inline comments to D45396: [DebugInfo] Create merged locations for instructions other than calls.
Tue, Apr 10, 2:45 PM
rnk added a comment to D45122: [DebugInfo] Add a new DI flag to record if a C++ record is a trivial type.

In D44406, I was imagining instead that we'd mark DISubprograms as trivial or non-trivial, not the type itself, and then CodeViewDebug.cpp in LLVM could skip trivial constructors when looking for classes.

Tue, Apr 10, 1:32 PM
rnk added a reviewer for D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant: rjmccall.
Tue, Apr 10, 11:44 AM
rnk accepted D45234: CMake: Check LLVM_ENABLE_LIBXML2 in clang.

lgtm

Tue, Apr 10, 10:50 AM
rnk accepted D45276: Fix a couple of incorrect fields in our generated PDBs..

Looks good, feel free to address the is64() check in a follow-up.

Tue, Apr 10, 10:48 AM
rnk added a comment to D45473: CodeGen: Don't try to canonicalize Unix-style paths in CodeView debug info..

Can you elaborate on the use case? What tool do you expect will consume these Unix style paths? What aspect of the path is invalidated by changing the slash style?

Tue, Apr 10, 10:48 AM
rnk accepted D44976: Change DEBUG() macro to LLVM_DEBUG() in clang-tools-extra.

lgtm

Tue, Apr 10, 10:40 AM
rnk accepted D44975: Change DEBUG() macro to LLVM_DEBUG().

lgtm

Tue, Apr 10, 10:39 AM
rnk accepted D45422: [Driver] Allow drivers to add multiple libc++ include paths.

lgtm

Tue, Apr 10, 10:39 AM
rnk requested changes to D45438: [CodeView] Enable debugging of captured variables within C++ lambdas.

I don't think this is enough, this issue generalizes to more than just lambdas. The more general problem is this:

  • C++ classes with methods create a cycle in the type hierarchy (class refers to method, method refers to class)
  • Some C++ classes without names may not be externally visibible
  • CodeView requires class identifiers to enable this circular reference.
Tue, Apr 10, 10:34 AM

Wed, Apr 4

rnk added inline comments to D45180: libcalls must check for "RtLibUseGOT" metadata during simplification.
Wed, Apr 4, 4:02 PM
rnk accepted D45295: report error messages in annotated_builder.py.

lgtm I'm so happy that fixing this doesn't require a buildmaster restart.

Wed, Apr 4, 3:58 PM
rnk accepted D45283: [COFF] Minimal serialization support for precompiled types records.

Looks good to me, but let's ask @zturner too.

Wed, Apr 4, 2:50 PM
rnk accepted D45233: [Driver] Update GCC libraries detection logic for Gentoo..

Thousands of lines of C++ to answer one of life's most simple questions: where are the libraries and where are the headers. =p

Wed, Apr 4, 2:43 PM
rnk added a comment to D45234: CMake: Check LLVM_ENABLE_LIBXML2 in clang.

The only think that links against libxml2 is c-index-test. Surely nobody cares about installing c-index-test on some other machine that lacks the local version of libxml2. What's the use case for this to justify the complexity of interacting with LLVM's configuration options?

Wed, Apr 4, 12:47 PM
rnk added a comment to D45260: COFF: Layout sections in the same order as link.exe.

Thanks for getting this and grinding through and updating the test cases!

Wed, Apr 4, 12:25 PM
rnk accepted D45116: Don't inline branch funnels.

Looks good to me, what do you think, Peter?

Wed, Apr 4, 11:59 AM
rnk added inline comments to D45146: [x86] Introduce a pass to begin more systematically fixing PR36028 and similar issues..
Wed, Apr 4, 11:22 AM

Tue, Apr 3

rnk committed rL329123: 'cat' command for internal shell - Support Python 3.
'cat' command for internal shell - Support Python 3
Tue, Apr 3, 3:44 PM
rnk closed D45077: 'cat' command for internal shell - Support Python 3.
Tue, Apr 3, 3:44 PM
rnk accepted D45077: 'cat' command for internal shell - Support Python 3.

lgtm

Tue, Apr 3, 3:35 PM
rnk added a comment to D45116: Don't inline branch funnels.
In D45116#1056216, @pcc wrote:

Are you sure that would work? The pass runs after RA.

Good point. You might want X86ISelLowering to copy these registers into and out of the ICALL_BRANCH_FUNNEL node, though.

Tue, Apr 3, 3:28 PM
rnk added a comment to D45116: Don't inline branch funnels.
In D45116#1055897, @pcc wrote:

Maybe I forgot to check intrinsics, or the arguments that are part of the variadic pack in the verifier.

I had to disable the verifier check for intrinsics because the check is not appropriate for all intrinsics, specifically this one.

Have you considered passing these extra parameters as an operand bundle, or do those not support variable numbers of operands?

Something like that might work, but even if we were to represent the target list like that, that wouldn't be enough, because the backend would also need to know how to emit the other arguments as if they were being passed as part of a regular call.

That is, not being able to inline arguments into an intrinsic is effectively a backend limitation that the inliner ought to respect. If that changes, we can change the inliner as well.

Tue, Apr 3, 3:17 PM
rnk added a comment to D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH.
In D45152#1055871, @rnk wrote:

Seems reasonable, looks good.

Any opinion on the wording of the option name?

Tue, Apr 3, 1:48 PM
rnk added inline comments to D45146: [x86] Introduce a pass to begin more systematically fixing PR36028 and similar issues..
Tue, Apr 3, 1:43 PM
rnk added a comment to D45213: [COFF][LLD] Add link support for precompiled headers .objs.

Can you split this into a patch that adds the basic YAML serialization and dumping functionality to LLVM, and a patch that adds the type merging capabilities across LLVM and LLD? The first one should take care of a lot of the boilerplate and we can stamp it quickly and then move on to review the complex type merging code.

Tue, Apr 3, 10:58 AM
rnk accepted D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH.

Seems reasonable, looks good.

Tue, Apr 3, 10:56 AM
rnk added a comment to D45116: Don't inline branch funnels.

Thanks, I looked, and now I understand better what's going on. These are thunks, and we do in fact want the argument forwarding behavior of a variadic musttail call.

Tue, Apr 3, 10:42 AM

Mon, Apr 2

rnk added a comment to D45116: Don't inline branch funnels.

Why is it invalid to inline the call to the branch funnel?

Mon, Apr 2, 5:04 PM
rnk committed rL329027: [InstCombine] Don't strip function type casts from musttail calls.
[InstCombine] Don't strip function type casts from musttail calls
Mon, Apr 2, 3:55 PM
rnk closed D45186: [InstCombine] Don't strip function type casts from musttail calls.
Mon, Apr 2, 3:55 PM
rnk accepted D44185: [Coroutines] Avoid assert splitting hidden coros.

lgtm, thanks!

Mon, Apr 2, 3:50 PM
rnk added inline comments to D45186: [InstCombine] Don't strip function type casts from musttail calls.
Mon, Apr 2, 3:47 PM
rnk updated the diff for D45186: [InstCombine] Don't strip function type casts from musttail calls.
  • rebase over committed changes
Mon, Apr 2, 3:38 PM
rnk updated the diff for D45186: [InstCombine] Don't strip function type casts from musttail calls.
  • this isn't about thunks
Mon, Apr 2, 3:34 PM
rnk retitled D45186: [InstCombine] Don't strip function type casts from musttail calls from [InstCombine] Don't optimize call target casts inside thunks to [InstCombine] Don't strip function type casts from musttail calls.
Mon, Apr 2, 3:30 PM
rnk added a comment to D43165: [lit] Fix problem in how Python versions open files with different encodings.

I committed a one line fix before noticing you reverted the change.
Did you happen to check if the fix works for you?

Mon, Apr 2, 3:25 PM
rnk committed rL329024: [lit] Attempt to fix builtin diff code for Python 2.
[lit] Attempt to fix builtin diff code for Python 2
Mon, Apr 2, 3:24 PM
rnk added inline comments to D45186: [InstCombine] Don't strip function type casts from musttail calls.
Mon, Apr 2, 3:14 PM
rnk updated subscribers of D45174: non-zero-length bit-fields should make a class non-empty.

+rnk This might also affect the MS ABI, but it does not result in any test case failures at least (and MSVC's type trait matches our state after this patch rather than before).

Mon, Apr 2, 3:09 PM
rnk committed rL329017: Revert r329012 "[lit] Fix problem in how Python versions open files with….
Revert r329012 "[lit] Fix problem in how Python versions open files with…
Mon, Apr 2, 2:37 PM
rnk added inline comments to D43165: [lit] Fix problem in how Python versions open files with different encodings.
Mon, Apr 2, 2:28 PM
rnk committed rL329015: Treat inlining a notail call as a regular, non-tail call.
Treat inlining a notail call as a regular, non-tail call
Mon, Apr 2, 2:26 PM
rnk created D45186: [InstCombine] Don't strip function type casts from musttail calls.
Mon, Apr 2, 1:54 PM
rnk committed rL329009: [MS] Emit vftable thunks for functions with incomplete prototypes.
[MS] Emit vftable thunks for functions with incomplete prototypes
Mon, Apr 2, 1:24 PM
rnk committed rC329009: [MS] Emit vftable thunks for functions with incomplete prototypes.
[MS] Emit vftable thunks for functions with incomplete prototypes
Mon, Apr 2, 1:24 PM