Page MenuHomePhabricator

modocache (Brian Gesiak)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 16 2014, 1:10 AM (308 w, 6 d)

Recent Activity

Wed, Jan 15

modocache committed rGdaab9227ff01: [IR] Module's NamedMD table needn't be 'void *' (authored by modocache).
[IR] Module's NamedMD table needn't be 'void *'
Wed, Jan 15, 3:30 PM
modocache closed D72812: [IR] Module's NamedMD table needn't be 'void *'.
Wed, Jan 15, 3:29 PM · Restricted Project
modocache added a comment to D72812: [IR] Module's NamedMD table needn't be 'void *'.

Previously this might've been an effort to keep the dependencies of a core header like Module.h small, but since it now has the dependencies this relies on - this change isn't making the situation worse in any way I can think of.

Wed, Jan 15, 3:29 PM · Restricted Project
modocache created D72812: [IR] Module's NamedMD table needn't be 'void *'.
Wed, Jan 15, 3:15 PM · Restricted Project

Wed, Jan 8

modocache updated the diff for D71903: [Coroutines][6/6] Clang schedules new passes.

Initialize PipelineTuningOptions properly.

Wed, Jan 8, 10:25 PM · Restricted Project, Restricted Project
modocache added inline comments to D72025: [PM][CG-SCC] Add a helper to update the call graph from SCC passes.
Wed, Jan 8, 3:28 PM · Restricted Project
modocache updated the diff for D71903: [Coroutines][6/6] Clang schedules new passes.

Update tests -- we now re-run the SCC pass, but don't insert the coroutine funclets into the SCC, so we no longer see the funclets in the output being tested here.

Wed, Jan 8, 12:41 PM · Restricted Project, Restricted Project

Tue, Jan 7

modocache added inline comments to D71899: [Coroutines][2/6] New pass manager: coro-split.
Tue, Jan 7, 2:24 PM · Restricted Project
modocache updated the diff for D71901: [Coroutines][4/6] New pass manager: coro-cleanup.

Apply clang-format.

Tue, Jan 7, 1:36 PM · Restricted Project
modocache updated the diff for D71899: [Coroutines][2/6] New pass manager: coro-split.

Apply clang-format.

Tue, Jan 7, 1:27 PM · Restricted Project
modocache updated the diff for D71898: [Coroutines][1/6] New pass manager: coro-early.

Use clang-format, re-add retcon instrinsics that I'd removed accidentally.

Tue, Jan 7, 1:27 PM · Restricted Project
modocache updated the diff for D71899: [Coroutines][2/6] New pass manager: coro-split.

Rebase onto my update of D72226.

Tue, Jan 7, 12:49 PM · Restricted Project
modocache updated the diff for D72226: [PM][CGSCC] API for outlined ref edges.

Rebase onto the latest revision of D70927.

Tue, Jan 7, 12:49 PM · Restricted Project

Mon, Jan 6

modocache updated the diff for D72226: [PM][CGSCC] API for outlined ref edges.

Add documentation for the new member function.

Mon, Jan 6, 9:28 PM · Restricted Project
modocache updated the diff for D71899: [Coroutines][2/6] New pass manager: coro-split.

To avoid an assert when compiling recursive coroutine functions, use the new API I added in D72226: CallGraphUpdater::registerReferredToOutlinedFunction. This is also a more valid method of updating the call graph. Previously, we were inserting coroutine funclets into the same SCC, despite the fact that they did not form a strongly-connected cycle with the original coroutine function. Now, we insert them as referred-to-by the original coroutine.

Mon, Jan 6, 9:21 PM · Restricted Project
modocache updated the diff for D72226: [PM][CGSCC] API for outlined ref edges.

Sorry for the noise -- this undo's a comment change.

Mon, Jan 6, 9:11 PM · Restricted Project
modocache updated the diff for D72226: [PM][CGSCC] API for outlined ref edges.

Remove the now unneeded check for self-referential edges.

Mon, Jan 6, 9:09 PM · Restricted Project
modocache updated the diff for D72226: [PM][CGSCC] API for outlined ref edges.

Update fields in Phabricator.

Mon, Jan 6, 9:09 PM · Restricted Project
modocache updated the diff for D72226: [PM][CGSCC] API for outlined ref edges.

I updated this patch such that it can address the underlying problem encountered by the coro-split pass in D71899: we need a method of inserting new ref edges into an existing call graph, whereas currently the CallGraphUpdater only allows the insertion of call edges. I don't know if there's any use for such an API in the old CallGraph, so I left that unimplemented. I also considered simply adding this functionality as a member function on LazyCallGraph or one of its inner classes, but decided to reuse CallGraphUpdater in order to share some of the logic.

Mon, Jan 6, 9:09 PM · Restricted Project
modocache added inline comments to D72025: [PM][CG-SCC] Add a helper to update the call graph from SCC passes.
Mon, Jan 6, 7:38 PM · Restricted Project
modocache planned changes to D72226: [PM][CGSCC] API for outlined ref edges.
Mon, Jan 6, 11:22 AM · Restricted Project
modocache added inline comments to D72226: [PM][CGSCC] API for outlined ref edges.
Mon, Jan 6, 11:22 AM · Restricted Project

Sun, Jan 5

modocache updated the diff for D71901: [Coroutines][4/6] New pass manager: coro-cleanup.

Added missing intrinsics.

Sun, Jan 5, 4:25 PM · Restricted Project
modocache added inline comments to D71901: [Coroutines][4/6] New pass manager: coro-cleanup.
Sun, Jan 5, 4:25 PM · Restricted Project
modocache updated the diff for D71899: [Coroutines][2/6] New pass manager: coro-split.

Addressed review comments.

Sun, Jan 5, 4:25 PM · Restricted Project
modocache added a comment to D71899: [Coroutines][2/6] New pass manager: coro-split.

Thanks for the quick review! I'll send an update in a sec.

Sun, Jan 5, 3:57 PM · Restricted Project
modocache committed rG83a9321f60d8: [Coroutines] Remove corresponding phi values when apply… (authored by modocache).
[Coroutines] Remove corresponding phi values when apply…
Sun, Jan 5, 3:29 PM
modocache added a comment to D71826: [Coroutines] Remove corresponding phi values when apply simplifyTerminatorLeadingToRet.

Done! Please excuse the wait. Have you considered getting commit access to LLVM? You won't have to wait on me in the future :) https://llvm.org/docs/DeveloperPolicy.html#new-contributors

Sun, Jan 5, 3:29 PM · Restricted Project
modocache closed D71826: [Coroutines] Remove corresponding phi values when apply simplifyTerminatorLeadingToRet.
Sun, Jan 5, 3:28 PM · Restricted Project
modocache added a comment to D72226: [PM][CGSCC] API for outlined ref edges.

Yeah, very good questions. When I was writing this patch, I was focused more on getting my specific use case, the one covered by the unit test, to work without hitting an assert. (I also think this is potentially a performance optimization, but I haven't measured in any meaningful way.) But I'm also curious why this occurs, and will look into it a little more. I'd ideally like to get a unit test case that demonstrates the issue without being based on top of D70927, but I wasn't able to in this first iteration of the patch.

Sun, Jan 5, 2:18 PM · Restricted Project
modocache updated subscribers of D71899: [Coroutines][2/6] New pass manager: coro-split.
Sun, Jan 5, 6:03 AM · Restricted Project
modocache updated subscribers of D71900: [Coroutines][3/6] New pass manager: coro-elide.
Sun, Jan 5, 6:03 AM · Restricted Project
modocache updated subscribers of D71903: [Coroutines][6/6] Clang schedules new passes.
Sun, Jan 5, 6:03 AM · Restricted Project, Restricted Project
modocache updated subscribers of D71902: [Coroutines][5/6] Support 'opt -passes=coroutines'.
Sun, Jan 5, 6:03 AM · Restricted Project
modocache updated the diff for D71903: [Coroutines][6/6] Clang schedules new passes.

Thanks for the reviews. Based on my latest revision of D71899, the coro-split diff, this patch now no longer manually enqueues coro-split twice. In addition, it uses the PassBuilder registration callbacks to enqueue coroutine passes in appropriate spots in the pipeline. coro-split now restarts the entire MainCGPipeline that's built as part of module optimization. As discussed, the coroutine passes are enqueued at -O0, and they're not enqueued when -disable-llvm-passes is specified.

Sun, Jan 5, 5:53 AM · Restricted Project, Restricted Project
modocache updated the diff for D71902: [Coroutines][5/6] Support 'opt -passes=coroutines'.

Small cleanup.

Sun, Jan 5, 5:46 AM · Restricted Project
modocache updated the diff for D71900: [Coroutines][3/6] New pass manager: coro-elide.

Addressed review comments from D71899.

Sun, Jan 5, 5:46 AM · Restricted Project
modocache added a child revision for D70927: Introduce a CallGraph updater helper class: D71899: [Coroutines][2/6] New pass manager: coro-split.
Sun, Jan 5, 5:43 AM · Restricted Project
modocache added parent revisions for D71899: [Coroutines][2/6] New pass manager: coro-split: D72226: [PM][CGSCC] API for outlined ref edges, D70927: Introduce a CallGraph updater helper class.
Sun, Jan 5, 5:43 AM · Restricted Project
modocache added a child revision for D72226: [PM][CGSCC] API for outlined ref edges: D71899: [Coroutines][2/6] New pass manager: coro-split.
Sun, Jan 5, 5:43 AM · Restricted Project
modocache updated the diff for D71899: [Coroutines][2/6] New pass manager: coro-split.

Thanks for the reviews! This latest revision makes use of the private LazyCallGraph API exposed via the CallGraphUpdater class from D70927, and the helper function updateCGAndAnalysisManagerForCGSCCPass added in D72025, in order to properly update the call graph. It also uses CGSCCUpdateResult in order to enqueue the second phase of coro-split, instead of relying on function devirtualization. This patch exposed what I think is a bug, so it now also relies on a fix for that behavior, D72226.

Sun, Jan 5, 5:43 AM · Restricted Project
modocache added a comment to D72025: [PM][CG-SCC] Add a helper to update the call graph from SCC passes.

My patch D71899 makes use of the API introduced in this patch (it's adds an outlining CGSCC pass to the new pass manager pipeline), and it works great! Thanks for this.

Sun, Jan 5, 5:36 AM · Restricted Project
modocache added a comment to D70927: Introduce a CallGraph updater helper class.

Thanks for this! I'm depending on this patch in D71899.

Sun, Jan 5, 5:36 AM · Restricted Project
modocache added a parent revision for D72226: [PM][CGSCC] API for outlined ref edges: D70927: Introduce a CallGraph updater helper class.
Sun, Jan 5, 5:34 AM · Restricted Project
modocache added a child revision for D70927: Introduce a CallGraph updater helper class: D72226: [PM][CGSCC] API for outlined ref edges.
Sun, Jan 5, 5:34 AM · Restricted Project
modocache created D72226: [PM][CGSCC] API for outlined ref edges.
Sun, Jan 5, 5:34 AM · Restricted Project

Fri, Jan 3

modocache added a comment to D71903: [Coroutines][6/6] Clang schedules new passes.

I'm currently working on ensuring that CGSCC optimizations are rerun to optimize coroutine funclets -- the primary feedback I received on this and on D71899 -- but I just realized I didn't respond to one comment on this set of reviews, from @junparser:

Fri, Jan 3, 7:54 PM · Restricted Project, Restricted Project
modocache added a comment to D70350: [DWARF] Allow cross-CU references of subprogram definitions.

Just FYI, @vsk, I work on a large codebase that uses Clang to build several projects with (monolithic, not thin) LTO. We have a nightly build that uses LLVM trunk (with some patches internal to us rebased on top) to compile these monolithic LTO projects. Since this patch landed on Dec 20 2019 as rG79daafc9030, each of these LTO builds (>10 in total) began failing during the LTO stage. The stack trace, which I pasted in P8180, seems to indicate there's a problem with debug info, and that's what led me to this diff. I've reverted this diff in our internal LLVM fork, and doing so has our LTO builds succeeding again.

Fri, Jan 3, 12:01 PM · Restricted Project

Wed, Jan 1

modocache planned changes to D71903: [Coroutines][6/6] Clang schedules new passes.
Wed, Jan 1, 7:31 PM · Restricted Project, Restricted Project
modocache updated the diff for D71901: [Coroutines][4/6] New pass manager: coro-cleanup.

As per one of the review comments on D71899, I split out the trivial parts of this patch, such as the 'legacy' pass renaming, and committed them separately. Here's an updated version of this patch without those 'NFC' changes.

Wed, Jan 1, 7:13 PM · Restricted Project
modocache updated the diff for D71900: [Coroutines][3/6] New pass manager: coro-elide.

As per one of the review comments on D71899, I split out the trivial parts of this patch, such as the 'legacy' pass renaming, and committed them separately. Here's an updated version of this patch without those 'NFC' changes.

Wed, Jan 1, 7:13 PM · Restricted Project
modocache planned changes to D71899: [Coroutines][2/6] New pass manager: coro-split.
Wed, Jan 1, 7:13 PM · Restricted Project
modocache updated the diff for D71899: [Coroutines][2/6] New pass manager: coro-split.

As per one of the first review comments, I split out the trivial parts of this patch, such as the 'legacy' pass renaming, and committed them separately. Here's an updated version of this patch without those 'NFC' changes. Next I'll work on the other comments, both the ones left here and on D71903. I'll set the status of this diff to 'Changes Planned' to reflect that. Thanks for the reviews so far!

Wed, Jan 1, 7:13 PM · Restricted Project
modocache updated the diff for D71898: [Coroutines][1/6] New pass manager: coro-early.

As per one of the review comments on D71899, I split out the trivial parts of this patch, such as the 'legacy' pass renaming, and committed them separately. Here's an updated version of this patch without those 'NFC' changes.

Wed, Jan 1, 7:05 PM · Restricted Project
modocache committed rG9ce0ff2eefcd: [Coroutines] const-ify internal helpers (NFC) (authored by modocache).
[Coroutines] const-ify internal helpers (NFC)
Wed, Jan 1, 7:04 PM
modocache committed rG2fcf7691dfb4: [Coroutines] Rename "legacy" passes (NFC) (authored by modocache).
[Coroutines] Rename "legacy" passes (NFC)
Wed, Jan 1, 6:55 PM

Fri, Dec 27

modocache committed rG0bc7665d9882: [ADT] Fix FoldingSet documentation typos (authored by modocache).
[ADT] Fix FoldingSet documentation typos
Fri, Dec 27, 6:28 PM
modocache accepted D71826: [Coroutines] Remove corresponding phi values when apply simplifyTerminatorLeadingToRet.

Sorry for the wait here, @junparser! I wanted to send my own patches out before looking at this one 😛

Fri, Dec 27, 11:21 AM · Restricted Project
modocache accepted D71916: High-Level Code-Review Documentation Update.

Thanks for this! This LGTM but I have two non-blocking suggestions.

Fri, Dec 27, 10:06 AM · Restricted Project
modocache added inline comments to D71903: [Coroutines][6/6] Clang schedules new passes.
Fri, Dec 27, 9:47 AM · Restricted Project, Restricted Project

Thu, Dec 26

modocache added a comment to D71899: [Coroutines][2/6] New pass manager: coro-split.

Also consider using the child/parent revision feature to ease the navigation between dependent patches.

Thu, Dec 26, 9:38 AM · Restricted Project
modocache added a child revision for D71902: [Coroutines][5/6] Support 'opt -passes=coroutines': D71903: [Coroutines][6/6] Clang schedules new passes.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a parent revision for D71903: [Coroutines][6/6] Clang schedules new passes: D71902: [Coroutines][5/6] Support 'opt -passes=coroutines'.
Thu, Dec 26, 9:29 AM · Restricted Project, Restricted Project
modocache added a child revision for D71901: [Coroutines][4/6] New pass manager: coro-cleanup: D71902: [Coroutines][5/6] Support 'opt -passes=coroutines'.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a parent revision for D71902: [Coroutines][5/6] Support 'opt -passes=coroutines': D71901: [Coroutines][4/6] New pass manager: coro-cleanup.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a child revision for D71900: [Coroutines][3/6] New pass manager: coro-elide: D71901: [Coroutines][4/6] New pass manager: coro-cleanup.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a parent revision for D71900: [Coroutines][3/6] New pass manager: coro-elide: D71899: [Coroutines][2/6] New pass manager: coro-split.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a child revision for D71898: [Coroutines][1/6] New pass manager: coro-early: D71899: [Coroutines][2/6] New pass manager: coro-split.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a parent revision for D71901: [Coroutines][4/6] New pass manager: coro-cleanup: D71900: [Coroutines][3/6] New pass manager: coro-elide.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a parent revision for D71899: [Coroutines][2/6] New pass manager: coro-split: D71898: [Coroutines][1/6] New pass manager: coro-early.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache added a child revision for D71899: [Coroutines][2/6] New pass manager: coro-split: D71900: [Coroutines][3/6] New pass manager: coro-elide.
Thu, Dec 26, 9:29 AM · Restricted Project
modocache created D71903: [Coroutines][6/6] Clang schedules new passes.
Thu, Dec 26, 6:31 AM · Restricted Project, Restricted Project
modocache created D71902: [Coroutines][5/6] Support 'opt -passes=coroutines'.
Thu, Dec 26, 6:31 AM · Restricted Project
modocache created D71901: [Coroutines][4/6] New pass manager: coro-cleanup.
Thu, Dec 26, 6:26 AM · Restricted Project
modocache created D71900: [Coroutines][3/6] New pass manager: coro-elide.
Thu, Dec 26, 6:26 AM · Restricted Project
modocache created D71899: [Coroutines][2/6] New pass manager: coro-split.
Thu, Dec 26, 6:26 AM · Restricted Project
modocache created D71898: [Coroutines][1/6] New pass manager: coro-early.
Thu, Dec 26, 6:26 AM · Restricted Project

Sun, Dec 22

modocache updated subscribers of D71709: Give frontend dump flags consistent names (*-dump instead of dump-*).

Ah, I'm sorry I wasn't clear -- instead of changing a lot of tests to use the new names exclusively, my suggestion was to change one or two tests to use the new canonical name, and have the remaining tests keep using the alias. That would keep the patch small, but still exercise both the new option name -tokens-dump as well as the old alias -dump-tokens.

Sun, Dec 22, 3:30 PM · Restricted Project

Fri, Dec 20

modocache added reviewers for D71730: Make lazyload_metadata.ll resilient to the addition of new metadata kinds: mehdi_amini, ostannard.

As a maintainer of a downstream fork of LLVM with its own metadata kinds, I'm a big fan of this change. Until this patch:

Fri, Dec 20, 6:48 AM · Restricted Project

Thu, Dec 19

modocache added a comment to D71731: [Format] fix dereference of pointers in co_yeld and co_return statements.

Nice! Thanks for this. This looks good to me, but I'll defer to the other reviewers you specified, since I think they're more familiar with clang-format.

Thu, Dec 19, 4:40 PM · Restricted Project
modocache requested changes to D71709: Give frontend dump flags consistent names (*-dump instead of dump-*).

Grepping for "dump-tokens", I can see one regression test that exercises this option: clang/test/Lexer/dollar-idents.c. "dump-coverage-mapping" also shows up in a few regression tests. One way to exercise the new option names is to modify these tests to use both the new and old option names. I think this would test that they can be used interchangeably.

Thu, Dec 19, 10:32 AM · Restricted Project
modocache added reviewers for D71709: Give frontend dump flags consistent names (*-dump instead of dump-*): echristo, jroelofs.
Thu, Dec 19, 10:32 AM · Restricted Project

Dec 18 2019

modocache requested changes to D71542: [coroutines][PR41909] don't build dependent coroutine statements if the coroutine still has a dependent promise type.

Great minds think alike! This looks like the patch I sent in November, D70579. I only just committed it two days ago in rG376cf43, but I ought to have updated the bug report. Thanks for pointing this out!

Dec 18 2019, 9:51 AM · Restricted Project

Dec 16 2019

modocache added a comment to D70579: [coroutines][PR41909] Generalize fix from D62550.

Thanks for the review! Much appreciated :)

Dec 16 2019, 3:27 PM · Restricted Project
modocache committed rG376cf43729c8: [coroutines][PR41909] Generalize fix from D62550 (authored by modocache).
[coroutines][PR41909] Generalize fix from D62550
Dec 16 2019, 2:44 PM
modocache closed D70579: [coroutines][PR41909] Generalize fix from D62550.
Dec 16 2019, 2:44 PM · Restricted Project

Dec 4 2019

modocache added a comment to D70219: Make `-fmodule-file=<name>=<path>` apply to .pcm file compilations.

This seems like a outright improvement, but as mentioned above it would be nice to get @rsmith's take on it since I'm not an expert. Two nitpicks, though:

Dec 4 2019, 11:27 AM · Restricted Project

Dec 3 2019

modocache added a comment to D70579: [coroutines][PR41909] Generalize fix from D62550.

@GorNishanov, @rsmith, friendly ping! @rsmith this patch addresses your requests from https://reviews.llvm.org/D62550.

Dec 3 2019, 1:21 PM · Restricted Project

Nov 30 2019

modocache committed rG8682d29a2877: [Format] Add format check for coroutine keywords with negative numbers (authored by modocache).
[Format] Add format check for coroutine keywords with negative numbers
Nov 30 2019, 12:50 PM
modocache closed D69180: [Format] Add format check for coroutine keywords with negative numbers.
Nov 30 2019, 12:49 PM · Restricted Project
modocache added inline comments to D69180: [Format] Add format check for coroutine keywords with negative numbers.
Nov 30 2019, 12:49 PM · Restricted Project

Nov 23 2019

modocache abandoned D70639: [IR] Take const references to LLVMContext.

I spoke with @mehdi_amini in the (experimental?) LLVM Discord and his take was that this leaks internal implementation details of LLVMContext: while accessing these types can in fact use const because LLVMContext constructs the types eagerly (as a performance optimization), there's no reason LLVMContext must do so eagerly. Modifying these to take a const reference inhibits the ability to change LLVMContext to lazily construct these types, and so this isn't a desirable change.

Nov 23 2019, 6:49 PM · Restricted Project
modocache created D70639: [IR] Take const references to LLVMContext.
Nov 23 2019, 6:40 PM · Restricted Project

Nov 22 2019

modocache added a comment to D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves.

Thanks again for the patch @junparser! And sorry the review took so long!

Nov 22 2019, 8:43 AM · Restricted Project
modocache committed rG0b3d1d1348da: [coroutines] Remove assert on CoroutineParameterMoves in Sema… (authored by modocache).
[coroutines] Remove assert on CoroutineParameterMoves in Sema…
Nov 22 2019, 8:43 AM
modocache closed D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves.
Nov 22 2019, 8:42 AM · Restricted Project

Nov 21 2019

modocache abandoned D27893: [lit] Fix shtest-format test on Windows.

Sorry for the noise bringing up this change, I don't think I'll be pursuing this after all.

Nov 21 2019, 8:24 PM
modocache abandoned D25398: [lit] Use path as test suite name.

Sorry for the noise bringing up this change, I don't think I'll be pursuing this after all.

Nov 21 2019, 8:15 PM
modocache resigned from D37602: Properly hook debuginfo-tests up to lit and CMake.
Nov 21 2019, 8:15 PM · Restricted Project
modocache resigned from D37946: [lit] Fix some Python 3 compatibility issues..
Nov 21 2019, 8:15 PM · Restricted Project