Page MenuHomePhabricator

rnk (Reid Kleckner)
User

Projects

User Details

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

Recent Activity

Fri, Sep 25

rnk added a comment to D87732: [Support] Provide sys::path::guess_style.

I'd prefer it if we didn't have to guess the path style in the first place. Initially I thought the target triple might be able to help us out, but it doesn't, because really you want to know the path style of the file system in use where compilation took place. Normally, clang is the one reading the source files, so whatever clang's host's style is is the right style. These test cases are getting tripped up because compilation is split over two host OSs: Clang's OS and llc's OS. Now I wonder if we shouldn't just persist the path style through the IR module, or even through the assembler if DWARF needs to join paths after assembly.

Fri, Sep 25, 4:10 PM · Restricted Project
rnk added inline comments to D87657: [DebugInfo] Remove dots from getFilenameByIndex return value.
Fri, Sep 25, 3:35 PM · Restricted Project

Thu, Sep 24

rnk added a reverting change for rG9bcf7b1c7a13: [NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen…: rG495a5e94baad: Revert "[NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen….
Thu, Sep 24, 4:48 PM
rnk committed rG495a5e94baad: Revert "[NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen… (authored by rnk).
Revert "[NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen…
Thu, Sep 24, 4:48 PM
rnk added a reverting change for rGa32feed0dbea: Add a static_assert confirming that DiagnosticBuilder is small: rG276f68eace7c: Revert "Add a static_assert confirming that DiagnosticBuilder is small".
Thu, Sep 24, 4:40 PM
rnk committed rG276f68eace7c: Revert "Add a static_assert confirming that DiagnosticBuilder is small" (authored by rnk).
Revert "Add a static_assert confirming that DiagnosticBuilder is small"
Thu, Sep 24, 4:40 PM
rnk committed rGa32feed0dbea: Add a static_assert confirming that DiagnosticBuilder is small (authored by rnk).
Add a static_assert confirming that DiagnosticBuilder is small
Thu, Sep 24, 4:39 PM
rnk committed rGecfc9b971269: [MS] For unknown ISAs, pass non-trivially copyable arguments indirectly (authored by rnk).
[MS] For unknown ISAs, pass non-trivially copyable arguments indirectly
Thu, Sep 24, 4:30 PM
rnk committed rGb8a50e920704: [MS] Simplify rules for passing C++ records (authored by rnk).
[MS] Simplify rules for passing C++ records
Thu, Sep 24, 4:30 PM
rnk accepted D87811: [CodeGen] emit CG profile for COFF object file.

lgtm

Thu, Sep 24, 2:20 PM · Restricted Project
rnk abandoned D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86.

I actually forgot about this change entirely and uploaded and landed a new one:
https://reviews.llvm.org/D87923

Thu, Sep 24, 1:21 PM · Restricted Project
rnk added a reverting change for rG8e780a1653e6: Recommit [NFC] Refactor DiagnosticBuilder and PartialDiagnostic: rGb62fd436a3e6: Revert "Recommit [NFC] Refactor DiagnosticBuilder and PartialDiagnostic".
Thu, Sep 24, 11:17 AM
rnk committed rGb62fd436a3e6: Revert "Recommit [NFC] Refactor DiagnosticBuilder and PartialDiagnostic" (authored by rnk).
Revert "Recommit [NFC] Refactor DiagnosticBuilder and PartialDiagnostic"
Thu, Sep 24, 11:17 AM
rnk added a reverting change for rGe39da8ab6a28: Recommit "[CUDA][HIP] Defer overloading resolution diagnostics for host device…: rG3453b6928da3: Revert "Recommit "[CUDA][HIP] Defer overloading resolution diagnostics for host….
Thu, Sep 24, 11:17 AM
rnk committed rG3453b6928da3: Revert "Recommit "[CUDA][HIP] Defer overloading resolution diagnostics for host… (authored by rnk).
Revert "Recommit "[CUDA][HIP] Defer overloading resolution diagnostics for host…
Thu, Sep 24, 11:17 AM
rnk updated subscribers of D84362: [NFC] Refactor DiagnosticBuilder and PartialDiagnostic.
Thu, Sep 24, 11:16 AM · Restricted Project
rnk accepted D87187: [Driver] Perform Linux distribution detection just once.

llvm::call_once is fine, but IMO the static local version is preferable: it's built in to the language, no headers to include.

Thu, Sep 24, 10:43 AM · Restricted Project

Wed, Sep 23

rnk accepted D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.

In any case, if you want to do that separately, I think this change stands on its own, so I will approve it as is.

Wed, Sep 23, 4:53 PM · Restricted Project
rnk added a comment to D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.

That's reverted. (https://reviews.llvm.org/rG91aed9bf975f1e4346cc8f4bdefc98436386ced2). I don't know if I should put two patches together.

Wed, Sep 23, 4:52 PM · Restricted Project
rnk accepted D70378: [LLD][COFF] Cover usage of LLD as a library.

Looks good to me, I didn't review very in depth, but I see the test case that we need. :)

Wed, Sep 23, 4:17 PM · Restricted Project, lld, Restricted Project
rnk added a comment to D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.

I'm saying the fix belongs in TargetObjectFileLoweringImpl.cpp, which is the point where we translate from IR to assembly. We can't fix this after producing assembly, because we don't know what symbols are marked dllimport at that point.

Wed, Sep 23, 4:14 PM · Restricted Project
rnk added a comment to D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.

I think this is reasonable. There is no need to record call graph profile edges from functions to imported functions in the metadata. However, I would prefer to move the check into CodeGen, so that if we receive some IR that has edges like this, we don't emit an object file that cannot be linked. I'd be OK doing the check in both places.

Wed, Sep 23, 3:12 PM · Restricted Project
rnk accepted D88121: [X86] Add a memory clobber to the bittest intrinsic inline asm. Get default clobbers from the target.

lgtm

Wed, Sep 23, 11:12 AM · Restricted Project
rnk added a comment to D87187: [Driver] Perform Linux distribution detection just once.

I think we can go forward with the reviewers we have. I have one more concern. Are the other reviewers happy?

Wed, Sep 23, 11:11 AM · Restricted Project
rnk added a comment to D87474: Ensure that we don't update Uses after MarkLive.

Arthur, do you mind taking a look?

Wed, Sep 23, 11:06 AM · Restricted Project
rnk edited reviewers for D87474: Ensure that we don't update Uses after MarkLive, added: aeubanks; removed: rnk.
Wed, Sep 23, 11:05 AM · Restricted Project

Tue, Sep 22

rnk added inline comments to D87811: [CodeGen] emit CG profile for COFF object file.
Tue, Sep 22, 2:31 PM · Restricted Project
rnk added a reverting change for rG91aed9bf975f: [CodeGen] emit CG profile for COFF object file: rG90242caca207: Revert "[CodeGen] emit CG profile for COFF object file".
Tue, Sep 22, 1:48 PM
rnk committed rG90242caca207: Revert "[CodeGen] emit CG profile for COFF object file" (authored by rnk).
Revert "[CodeGen] emit CG profile for COFF object file"
Tue, Sep 22, 1:47 PM
rnk added a reverting change for D87811: [CodeGen] emit CG profile for COFF object file: rG90242caca207: Revert "[CodeGen] emit CG profile for COFF object file".
Tue, Sep 22, 1:47 PM · Restricted Project
rnk added inline comments to D88112: Clang is crashing after generating the right diagnostic for a re-declaration of a friend method - Fix for PR47544.
Tue, Sep 22, 1:16 PM
rnk added a comment to D88059: [Coroutine] Split PHI Nodes in `cleanuppad` blocks in a way that obeys EH pad rules.

Seems reasonable, if the new multi-value phi works with later passes.

Tue, Sep 22, 11:42 AM · Restricted Project
rnk added a comment to D74158: make the AsmPrinterHandler array public.

I can take a look.

Tue, Sep 22, 11:33 AM · Restricted Project
rnk added a comment to D88056: [gn build] Allow option to build with asan/tsan/ubsan.

Looks good to me. My only question is whether to optimistically assume these work out of the box on Mac, and to relax the asserts. Let's wait for input from Nico, though.

Tue, Sep 22, 10:54 AM · Restricted Project

Mon, Sep 21

rnk accepted D87448: [CodeGen] [WinException] Only produce handler data at the end of the function if needed.

@rnk - Can you follow up on the discussion above?

Mon, Sep 21, 12:00 PM · Restricted Project
rnk accepted D88005: [clang] [MinGW] Add an implicit .exe suffix even when crosscompiling.

lgtm

Mon, Sep 21, 11:57 AM · Restricted Project
rnk added a comment to D87888: [X86] Use inlineasm flag output for the _bittest* intrinsics..

Honestly, I forget exactly what the memory clobber does beyond the "sideeffect" marker. I would expect LLVM to model these just as external function calls that could read or write memory that is passed to them.

Mon, Sep 21, 11:56 AM · Restricted Project
rnk committed rG3b3a16548568: [MS] On x86_32, pass overaligned, non-copyable arguments indirectly (authored by rnk).
[MS] On x86_32, pass overaligned, non-copyable arguments indirectly
Mon, Sep 21, 11:49 AM
rnk closed D87923: [MS] On x86_32, pass overaligned, non-copyable arguments indirectly.
Mon, Sep 21, 11:49 AM · Restricted Project
rnk accepted D88002: [clang-cl] Always interpret the LIB env var as separated with semicolons.

lgtm

Mon, Sep 21, 8:58 AM · Restricted Project, Restricted Project

Fri, Sep 18

rnk added a comment to D87805: [PDB] Merge types in parallel when using ghashing.

Thanks a lot for working on this Reid, really neat!

Fri, Sep 18, 3:20 PM · Restricted Project
rnk committed rG9932561b4892: [COFF] Move per-global .drective emission from AsmPrinter to TLOFCOFF (authored by rnk).
[COFF] Move per-global .drective emission from AsmPrinter to TLOFCOFF
Fri, Sep 18, 2:31 PM
rnk requested review of D87923: [MS] On x86_32, pass overaligned, non-copyable arguments indirectly.
Fri, Sep 18, 11:29 AM · Restricted Project
rnk accepted D87811: [CodeGen] emit CG profile for COFF object file.

I see the assembler support is already there, looks like it works.

Fri, Sep 18, 10:48 AM · Restricted Project
rnk added a comment to D87808: [DebugInfo] Fix bug in constructor homing for classes with trivial constructors..

@rsmith What's the deal with these anonymous structs/unions? Why do they have copy/move constructors (are those technically called from the enclosing class's copy/move constructors?) but no default constructor to be called from the other ctors of the enclosing class?

Fri, Sep 18, 9:59 AM · Restricted Project
rnk updated subscribers of rGb959906cb9e7: [PGO] Use multiple comdat groups for COFF.

I'm aware the ELF supports comdat groups, but when I spoke to @xur back in 2019 we decided it was best not to use this feature. That resulted in D68041, which I see you reverted.

Fri, Sep 18, 9:31 AM

Thu, Sep 17

rnk updated the diff for D87805: [PDB] Merge types in parallel when using ghashing.
  • rebase over previous changes
  • fix funcIdToType map for /Yu files
Thu, Sep 17, 2:50 PM · Restricted Project
rnk committed rG1e5b7e91aa64: [PDB] Split TypeServerSource and extend type index map lifetime (authored by rnk).
[PDB] Split TypeServerSource and extend type index map lifetime
Thu, Sep 17, 11:53 AM
rnk closed D87736: [PDB] Split TypeServerSource and extend type index map lifetime.
Thu, Sep 17, 11:53 AM · Restricted Project
rnk updated rnk.
Thu, Sep 17, 11:41 AM
rnk added a comment to D87736: [PDB] Split TypeServerSource and extend type index map lifetime.

Thanks!

Thu, Sep 17, 11:36 AM · Restricted Project
rnk added inline comments to D87187: [Driver] Perform Linux distribution detection just once.
Thu, Sep 17, 10:52 AM · Restricted Project
rnk added inline comments to D87187: [Driver] Perform Linux distribution detection just once.
Thu, Sep 17, 10:46 AM · Restricted Project
rnk added inline comments to D87736: [PDB] Split TypeServerSource and extend type index map lifetime.
Thu, Sep 17, 10:40 AM · Restricted Project
rnk updated the diff for D87736: [PDB] Split TypeServerSource and extend type index map lifetime.
  • Ensure all objs with symbols have TpiSources
Thu, Sep 17, 10:40 AM · Restricted Project

Wed, Sep 16

rnk updated the summary of D87805: [PDB] Merge types in parallel when using ghashing.
Wed, Sep 16, 4:33 PM · Restricted Project
rnk requested review of D87805: [PDB] Merge types in parallel when using ghashing.
Wed, Sep 16, 4:30 PM · Restricted Project
rnk retitled D87736: [PDB] Split TypeServerSource and extend type index map lifetime from [PDB] Split TypeServerSource and extend CVIndexMap lifetime to [PDB] Split TypeServerSource and extend type index map lifetime.
Wed, Sep 16, 3:16 PM · Restricted Project
rnk added a comment to D87736: [PDB] Split TypeServerSource and extend type index map lifetime.

Thanks, looks like I can remove it. I made things more uniform by having ipiMap always point to something, so if we have an item reference, always use ipiMap.

Wed, Sep 16, 3:16 PM · Restricted Project
rnk updated the diff for D87736: [PDB] Split TypeServerSource and extend type index map lifetime.
  • remove debugging noinline
Wed, Sep 16, 3:14 PM · Restricted Project
rnk updated the diff for D87736: [PDB] Split TypeServerSource and extend type index map lifetime.
  • eliminate CVIndexMap entirely
Wed, Sep 16, 2:31 PM · Restricted Project
rnk added inline comments to D87544: [CFGuard] Add address-taken IAT tables and delay-load support.
Wed, Sep 16, 1:57 PM · Restricted Project
rnk added a comment to D86485: [test] Fix FullUnroll.ll.

Here is the sequence of what happened as I understand it:

  • D71687 is intended to fix the full loop unrolling pragma with the NPM at -O1. The accompanying clang test covers this.
  • D71687 also adds an LLVM IR test. It is an integration test: it started from unoptimized IR and ran the full -O1 NPM pipeline. The IR happens to contain optnone. Maybe that was unintentional, since one used to be able to run clang foo.c -emit-llvm -o - | opt -O1 -S and get optimized IR. Now clang applies the optnone attribute, and that no longer works.
  • D85578 made the IR test more targeted: now it only runs the full unrolling pass instead of the O1 pipeline.
Wed, Sep 16, 1:41 PM · Restricted Project
rnk added a reviewer for D87544: [CFGuard] Add address-taken IAT tables and delay-load support: hans.
Wed, Sep 16, 12:54 PM · Restricted Project
rnk added a comment to D83154: clang: Add -fcoverage-prefix-map.

Yeah, my goal is for the build system to be able to avoid having to embed PWD into the compiler flags. For example, I believe it is a goal of the LLVM gn build system for the build tree to be relocatable. If the compiler command lines need to contain absolute paths to make the paths in the coverage relative, we won't be able to achieve that goal.

Wed, Sep 16, 12:23 PM · Restricted Project
rnk committed rGe47d2927de79: Include (Type|Symbol)Record.h less (authored by rnk).
Include (Type|Symbol)Record.h less
Wed, Sep 16, 9:59 AM

Tue, Sep 15

rnk requested review of D87736: [PDB] Split TypeServerSource and extend type index map lifetime.
Tue, Sep 15, 7:03 PM · Restricted Project
rnk committed rG1b88845ce1b7: [PDB] Drop LF_PRECOMP from debugTypes earlier (authored by rnk).
[PDB] Drop LF_PRECOMP from debugTypes earlier
Tue, Sep 15, 7:02 PM
rnk requested changes to D81803: [Support] PR42623: Avoid setting the delete-on-close bit for TempFile.

The situation where a temp file won't be delete without the delete-on-close bit set seems rare so just avoid setting it entirely.

Tue, Sep 15, 11:32 AM · Restricted Project
rnk accepted D87701: Do not apply calling conventions to MSVC entry points.

lgtm, thanks.

Tue, Sep 15, 11:23 AM · Restricted Project

Fri, Sep 11

rnk committed rG12a281d368e3: [gn] Remove unneeded MC dep from llvm-tblgen (authored by rnk).
[gn] Remove unneeded MC dep from llvm-tblgen
Fri, Sep 11, 6:29 PM
rnk closed D87553: [gn] Remove unneeded MC dep from llvm-tblgen.
Fri, Sep 11, 6:29 PM · Restricted Project
rnk requested review of D87553: [gn] Remove unneeded MC dep from llvm-tblgen.
Fri, Sep 11, 5:48 PM · Restricted Project
rnk accepted D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral.

lgtm, thanks!

Fri, Sep 11, 2:42 PM · Restricted Project
rnk added a comment to D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral.

Looks good, but we should test both sides of the MSVC behavior.

Fri, Sep 11, 12:21 PM · Restricted Project

Thu, Sep 10

rnk added a comment to D82883: [LLD][COFF] Deduplicate .pdata entries.

This all sounds like this bug: https://bugs.llvm.org/show_bug.cgi?id=35337
Which was supposed to be fixed in rG107f55005bc9c9de2378057f56ae02016795a3ae

Thu, Sep 10, 5:13 PM · Restricted Project
rnk committed rG2c73bef7fad4: Fix wrong comment about enabling optimizations to work around a bug (authored by rnk).
Fix wrong comment about enabling optimizations to work around a bug
Thu, Sep 10, 4:52 PM
rnk added inline comments to D87475: Use pragmas to work around MSVC x86_32 debug miscompile bug.
Thu, Sep 10, 4:42 PM · Restricted Project
rnk committed rG4e3edef4b8b6: Use pragmas to work around MSVC x86_32 debug miscompile bug (authored by rnk).
Use pragmas to work around MSVC x86_32 debug miscompile bug
Thu, Sep 10, 2:50 PM
rnk closed D87475: Use pragmas to work around MSVC x86_32 debug miscompile bug.
Thu, Sep 10, 2:50 PM · Restricted Project
rnk added inline comments to D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral.
Thu, Sep 10, 2:33 PM · Restricted Project
rnk added a comment to D87448: [CodeGen] [WinException] Only produce handler data at the end of the function if needed.

Makes sense.

Thu, Sep 10, 2:19 PM · Restricted Project
rnk accepted D87406: [DebugInfo] Fixing CodeView assert related to lowerBound field of DISubrange..

I would ask you to add a test, but I can't recommend the existing test, llvm/test/DebugInfo/COFF/types-array.ll, as a good basis for a test. I'll look into rewriting it and adding coverage after you land.

Thu, Sep 10, 2:07 PM · debug-info, Restricted Project
rnk requested review of D87475: Use pragmas to work around MSVC x86_32 debug miscompile bug.
Thu, Sep 10, 11:42 AM · Restricted Project
rnk added inline comments to D83154: clang: Add -fcoverage-prefix-map.
Thu, Sep 10, 9:53 AM · Restricted Project
rnk added inline comments to D87406: [DebugInfo] Fixing CodeView assert related to lowerBound field of DISubrange..
Thu, Sep 10, 9:19 AM · debug-info, Restricted Project

Wed, Sep 9

rnk added inline comments to D83154: clang: Add -fcoverage-prefix-map.
Wed, Sep 9, 11:16 AM · Restricted Project

Jun 30 2020

rnk committed rG926fab7c4fcd: [gn build] Update build for new OpenMP tablegen logic (authored by rnk).
[gn build] Update build for new OpenMP tablegen logic
Jun 30 2020, 4:19 PM
rnk committed rGb7402edce315: [PDB] Defer public serialization until PDB writing (authored by rnk).
[PDB] Defer public serialization until PDB writing
Jun 30 2020, 11:57 AM
rnk closed D81296: [PDB] Defer public serialization until PDB writing.
Jun 30 2020, 11:57 AM · Restricted Project
rnk updated the diff for D81296: [PDB] Defer public serialization until PDB writing.
  • update
Jun 30 2020, 11:24 AM · Restricted Project
rnk added inline comments to D81296: [PDB] Defer public serialization until PDB writing.
Jun 30 2020, 11:24 AM · Restricted Project

Jun 29 2020

rnk committed rG1c15229ba3a1: [gn build] Add missing llvm-lipo dep to check-lld (authored by rnk).
[gn build] Add missing llvm-lipo dep to check-lld
Jun 29 2020, 12:29 PM
rnk committed rG6d01a9419351: Silence unused var warning in NDEBUG build (authored by rnk).
Silence unused var warning in NDEBUG build
Jun 29 2020, 11:55 AM

Jun 11 2020

rnk committed rG1c03389c29f3: Re-land "Migrate the rest of COFFObjectFile to Error" (authored by rnk).
Re-land "Migrate the rest of COFFObjectFile to Error"
Jun 11 2020, 2:55 PM
rnk added a reverting change for rG101fbc01382e: Revert "Migrate the rest of COFFObjectFile to Error": rG1c03389c29f3: Re-land "Migrate the rest of COFFObjectFile to Error".
Jun 11 2020, 2:55 PM

Jun 5 2020

rnk committed rGb5289656b865: Migrate the rest of COFFObjectFile to Error (authored by rnk).
Migrate the rest of COFFObjectFile to Error
Jun 5 2020, 4:50 PM
rnk committed rGe03a135be8cf: Re-land "Migrate Binary::checkOffset from error_code to Error, NFC" (authored by rnk).
Re-land "Migrate Binary::checkOffset from error_code to Error, NFC"
Jun 5 2020, 4:49 PM
rnk added a reverting change for rG38f3ba591e3a: Revert "Migrate Binary::checkOffset from error_code to Error, NFC": rGe03a135be8cf: Re-land "Migrate Binary::checkOffset from error_code to Error, NFC".
Jun 5 2020, 4:49 PM
rnk committed rG74bd98829d82: Migrate Binary::checkOffset from error_code to Error, NFC (authored by rnk).
Migrate Binary::checkOffset from error_code to Error, NFC
Jun 5 2020, 2:04 PM