Page MenuHomePhabricator

lattner (Chris Lattner)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 28 2016, 7:50 PM (259 w, 2 d)

Recent Activity

Today

lattner added a comment to D104302: [mlir][ODS] Fix copy ctor for generate Pass classes.

Do we need a unit test at all here? If I understand the patch correctly, you're saying that the canonicalize options aren't being copied over correctly when a pass is cloned. If that is true, then we can exercise this with a standard filecheck test of the canonicalize pass.

Thu, Jun 17, 9:33 AM · Restricted Project

Yesterday

lattner added inline comments to rGce77039596a9: [Verifier] Parallelize verification and dom checking. NFC..
Wed, Jun 16, 2:22 PM
lattner added inline comments to rGce77039596a9: [Verifier] Parallelize verification and dom checking. NFC..
Wed, Jun 16, 2:21 PM
lattner added a comment to D104302: [mlir][ODS] Fix copy ctor for generate Pass classes.

Whoa, great catch. Only request: please merge the unit test into the existing test/lib/Pass stuff. Thank you!

Wed, Jun 16, 1:32 PM · Restricted Project
lattner resigned from D104394: [MachineCopyPropagation] Fix differences in code gen when compiling with -g.

I'm not a competent reviewer for this patch.

Wed, Jun 16, 1:23 PM · Restricted Project
lattner added a comment to D104207: [Verifier] Parallelize verification and dom checking. NFC..

This is just using llvm::parallelForEachN (not doing anything particularly fancy) so I can't imagine how it would be different than other similar things using it. It is possible this is exposing a lower level problem in LLVM threading.

Wed, Jun 16, 1:18 PM · Restricted Project

Tue, Jun 15

lattner added a comment to D104218: [ADT] Add StringRef consume_front_lower and consume_back_lower.

Oh, I completely missed that. I would recommend using the word case_insensitive here. I don't think stringref has any other case insensitive support, and I don't think this is the obvious place to start. I would start with case insensitive equality comparison. Your client can use that as a primitive that it builds higher level functionality on top of.

Tue, Jun 15, 2:28 PM · Restricted Project
lattner added a comment to D104218: [ADT] Add StringRef consume_front_lower and consume_back_lower.

I was thinking that the closure would take a character, not a stringref. So you could do something like str.consume_back(islower).

Tue, Jun 15, 10:36 AM · Restricted Project
lattner added a comment to D104296: [SCCP] Remove code redundancy in resolvedUndefsIn.

I'd prefer it if someone else could review this code. It has been a very long time since I've touched it.

Tue, Jun 15, 10:22 AM · Restricted Project

Mon, Jun 14

lattner added a comment to D104218: [ADT] Add StringRef consume_front_lower and consume_back_lower.

I don't think this is general enough requirement to add a specialized method for it. Did you consider adding a consumes_front/back overload that takes a lambda?

Mon, Jun 14, 5:14 PM · Restricted Project
lattner committed rGa490ca8e014a: [PassManager] Save compile time by not running the verifier unnecessarily. NFC (authored by lattner).
[PassManager] Save compile time by not running the verifier unnecessarily. NFC
Mon, Jun 14, 11:44 AM
lattner closed D104243: [PassManager] Save compile time by not running the verifier unnecessarily. NFC.
Mon, Jun 14, 11:44 AM · Restricted Project
lattner updated the diff for D104243: [PassManager] Save compile time by not running the verifier unnecessarily. NFC.

Clean up the comments.

Mon, Jun 14, 10:35 AM · Restricted Project
lattner requested review of D104243: [PassManager] Save compile time by not running the verifier unnecessarily. NFC.
Mon, Jun 14, 10:31 AM · Restricted Project
lattner committed rGce77039596a9: [Verifier] Parallelize verification and dom checking. NFC. (authored by lattner).
[Verifier] Parallelize verification and dom checking. NFC.
Mon, Jun 14, 10:03 AM
lattner closed D104207: [Verifier] Parallelize verification and dom checking. NFC..
Mon, Jun 14, 10:03 AM · Restricted Project
lattner updated the diff for D104207: [Verifier] Parallelize verification and dom checking. NFC..

Remove the changes to Pass.cpp, they were supposed to be in a follow-on

Mon, Jun 14, 10:02 AM · Restricted Project
lattner added inline comments to D104207: [Verifier] Parallelize verification and dom checking. NFC..
Mon, Jun 14, 10:02 AM · Restricted Project
lattner updated the diff for D104207: [Verifier] Parallelize verification and dom checking. NFC..

Incorporate River's feedback.

Mon, Jun 14, 9:26 AM · Restricted Project
lattner added a comment to D104207: [Verifier] Parallelize verification and dom checking. NFC..

Thank you for the quick review!

Mon, Jun 14, 9:26 AM · Restricted Project

Sun, Jun 13

lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Yes, thank you! Too many similar things.

Sun, Jun 13, 10:38 PM · Restricted Project
lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

The new patch is available here: https://reviews.llvm.org/D103373/new/

Sun, Jun 13, 10:28 PM · Restricted Project
lattner added a comment to D104207: [Verifier] Parallelize verification and dom checking. NFC..

I filed https://bugs.llvm.org/show_bug.cgi?id=50701 to track cases where the PassManager is verifying things it shouldn't IMO. Fixing that will be an orthogonal improvement to this patch.

Sun, Jun 13, 10:10 PM · Restricted Project
lattner requested review of D104207: [Verifier] Parallelize verification and dom checking. NFC..
Sun, Jun 13, 9:42 PM · Restricted Project
lattner committed rG0dd4c4b5ae49: [Testsuite] Change these tests to only have a single verification error, NFC. (authored by lattner).
[Testsuite] Change these tests to only have a single verification error, NFC.
Sun, Jun 13, 9:37 PM
lattner committed rG4fa867786043: [DominanceInfo] Make the ctor take a defaulted value for the operand. NFC. (authored by lattner).
[DominanceInfo] Make the ctor take a defaulted value for the operand. NFC.
Sun, Jun 13, 6:26 PM
lattner accepted D102763: [LLParser] Remove outdated deplibs.

Thanks!

Sun, Jun 13, 2:16 PM · Restricted Project, Restricted Project

Fri, Jun 11

lattner added a comment to D103811: [docs] Set Phabricator as the tool for pre-commit code reviews.

nice, thank you!

Fri, Jun 11, 10:17 AM · Restricted Project

Wed, Jun 9

lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

I don't know how to revert this, but feel free if you see it as a smoking gun. I only abandoned this because "it didn't show a big speedup and I have a hopefully better idea of how to improve things", not because I think this is harmful

Wed, Jun 9, 2:36 PM · Restricted Project

Tue, Jun 8

lattner accepted D103935: Add Twine support for std::string_view..

LGTM, thanks!

Tue, Jun 8, 5:51 PM · Restricted Project
lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

I'm not sure how this landed, it looks like it got pushed as a consequence of the twine patch. Maybe I put them both into the same directory accidentally (my git fu is not great).

Tue, Jun 8, 3:53 PM · Restricted Project
lattner committed rG92a79dbe9141: [Core] Add Twine support for StringAttr and Identifier. NFC. (authored by lattner).
[Core] Add Twine support for StringAttr and Identifier. NFC.
Tue, Jun 8, 9:47 AM
lattner committed rG08664d005c02: [Verifier] Speed up and parallelize dominance checking. NFC (authored by lattner).
[Verifier] Speed up and parallelize dominance checking. NFC
Tue, Jun 8, 9:47 AM
lattner closed D103754: [Core] Add Twine support for StringAttr and Identifier. NFC..
Tue, Jun 8, 9:47 AM · Restricted Project
lattner updated the diff for D103754: [Core] Add Twine support for StringAttr and Identifier. NFC..

spell out auto

Tue, Jun 8, 9:46 AM · Restricted Project

Mon, Jun 7

lattner updated the diff for D103754: [Core] Add Twine support for StringAttr and Identifier. NFC..

tweak commit message.

Mon, Jun 7, 11:19 PM · Restricted Project
lattner updated the diff for D103754: [Core] Add Twine support for StringAttr and Identifier. NFC..

Simplify the API for Identifier::get and StringAttr::get to always take Twines.

Mon, Jun 7, 11:18 PM · Restricted Project

Sat, Jun 5

lattner added a comment to D103754: [Core] Add Twine support for StringAttr and Identifier. NFC..

I'm open to that, only two downsides I can think of:

  1. absolutely tiny additional runtime overhead, slight codebloat for making the temporary twine. I'm not concerned about this.
Sat, Jun 5, 9:14 PM · Restricted Project
lattner added a comment to D103754: [Core] Add Twine support for StringAttr and Identifier. NFC..

xref: https://github.com/llvm/circt/pull/1202#pullrequestreview-676800939

Sat, Jun 5, 11:42 AM · Restricted Project
lattner requested review of D103754: [Core] Add Twine support for StringAttr and Identifier. NFC..
Sat, Jun 5, 11:38 AM · Restricted Project

Wed, Jun 2

lattner added inline comments to D103353: [mlir][NFC] Split the non-templated bits out of IROperand into a base class.
Wed, Jun 2, 1:26 PM · Restricted Project
lattner added inline comments to D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC).
Wed, Jun 2, 10:58 AM · Restricted Project, Restricted Project, Restricted Project

Tue, Jun 1

lattner abandoned D103373: [Verifier] Speed up and parallelize dominance checking. NFC.
Tue, Jun 1, 4:41 PM · Restricted Project
lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

I'm abandoning this patch. With the other general improvements to dominators and with the inefficiency in the threading stuff, this is no longer a win.

Tue, Jun 1, 4:41 PM · Restricted Project
lattner added inline comments to D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC).
Tue, Jun 1, 4:04 PM · Restricted Project, Restricted Project, Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

update commit message for real this time.

Tue, Jun 1, 3:32 PM · Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Update commit message

Tue, Jun 1, 3:29 PM · Restricted Project
lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Ok, I think this patch is ready for a re-check. Thanks!

Tue, Jun 1, 3:22 PM · Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Final grooming, remove commented out #include, use range base for loop again.

Tue, Jun 1, 3:22 PM · Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Dramatically simplify this patch by merging in the dominator improvements.

Tue, Jun 1, 3:20 PM · Restricted Project
lattner committed rG6134231a78bf: [CSE] Ask DominanceInfo about "hasSSADominance" instead of reconstructing it. (authored by lattner).
[CSE] Ask DominanceInfo about "hasSSADominance" instead of reconstructing it.
Tue, Jun 1, 3:16 PM
lattner closed D103494: [CSE] Ask DominanceInfo about "hasSSADominance" instead of reconstructing it..
Tue, Jun 1, 3:16 PM · Restricted Project
lattner accepted D103494: [CSE] Ask DominanceInfo about "hasSSADominance" instead of reconstructing it..

Self approving as 'obvious'

Tue, Jun 1, 3:16 PM · Restricted Project
lattner requested review of D103494: [CSE] Ask DominanceInfo about "hasSSADominance" instead of reconstructing it..
Tue, Jun 1, 3:15 PM · Restricted Project
lattner committed rG412ae15de49a: [Dominators] Rewrite the dominator implementation for efficiency. NFC. (authored by lattner).
[Dominators] Rewrite the dominator implementation for efficiency. NFC.
Tue, Jun 1, 2:58 PM
lattner closed D103384: [Dominators] Rewrite the dominator implementation for efficiency. NFC..
Tue, Jun 1, 2:58 PM · Restricted Project
lattner accepted D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC).

This required adding quite a few additional includes, as many files were relying on various things pulled in by ArrayRef.h.

Tue, Jun 1, 2:54 PM · Restricted Project, Restricted Project, Restricted Project
lattner accepted D103384: [Dominators] Rewrite the dominator implementation for efficiency. NFC..

This got a lot of valuable feedback already, I'm going to land this to hill climb on top of it (e.g. https://reviews.llvm.org/D103373). I value any other input of course!

Tue, Jun 1, 2:49 PM · Restricted Project
lattner added inline comments to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.
Tue, Jun 1, 2:48 PM · Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Use setOrderIDForThread correctly.

Tue, Jun 1, 2:47 PM · Restricted Project
lattner added inline comments to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.
Tue, Jun 1, 10:24 AM · Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Updates from River's review

Tue, Jun 1, 10:22 AM · Restricted Project
lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

SGTM, I'll revise this after the other patch lands, thank you for the review!

Tue, Jun 1, 10:10 AM · Restricted Project
lattner updated the diff for D103384: [Dominators] Rewrite the dominator implementation for efficiency. NFC..

Incorporate feedback from review.

Tue, Jun 1, 10:01 AM · Restricted Project
lattner added a comment to D103384: [Dominators] Rewrite the dominator implementation for efficiency. NFC..

thanks for all the feedback, incorporated!

Tue, Jun 1, 10:00 AM · Restricted Project
lattner accepted D103422: [ADT] Move DenseMapInfo for APInt into APInt.h (PR50527).

Thank you so much for tackling this!

Tue, Jun 1, 8:46 AM · Restricted Project

Sun, May 30

lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

The dominators rewrite is here: https://reviews.llvm.org/D103384. Once that goes into tree I'll merge it into this patch to simplify it - we can drop the "don't recurse into IsolatedFromAbove regions" change clearly and the manual laziness isn't needed, but the parallel constructs are.

Sun, May 30, 6:05 PM · Restricted Project
lattner added reviewers for D103384: [Dominators] Rewrite the dominator implementation for efficiency. NFC.: stephenneuendorffer, bondhugula, dfki-mako.

Adding a few others who have touched this code.

Sun, May 30, 6:04 PM · Restricted Project
lattner requested review of D103384: [Dominators] Rewrite the dominator implementation for efficiency. NFC..
Sun, May 30, 6:02 PM · Restricted Project
lattner committed rG1e344ce4f3fa: [CSE] Make domInfo a stored property, cut use of DominanceInfo… (authored by lattner).
[CSE] Make domInfo a stored property, cut use of DominanceInfo…
Sun, May 30, 12:24 PM
lattner added a comment to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Actually, instead of making this more complicated, I'd like to keep this simple in this patch as it is.

Sun, May 30, 10:41 AM · Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Use isBeforeInBlock instead of maintaining a SmallPtrSet.

Sun, May 30, 10:39 AM · Restricted Project
lattner added inline comments to D103373: [Verifier] Speed up and parallelize dominance checking. NFC.
Sun, May 30, 10:22 AM · Restricted Project

Sat, May 29

lattner added reviewers for D103373: [Verifier] Speed up and parallelize dominance checking. NFC: mehdi_amini, jpienaar.
Sat, May 29, 5:13 PM · Restricted Project
lattner updated the diff for D103373: [Verifier] Speed up and parallelize dominance checking. NFC.

Improve comments.

Sat, May 29, 5:13 PM · Restricted Project
lattner requested review of D103373: [Verifier] Speed up and parallelize dominance checking. NFC.
Sat, May 29, 5:08 PM · Restricted Project
lattner committed rG67d0e79b1f41: [Dominance] Speed up recalculate noticable, NFC. (authored by lattner).
[Dominance] Speed up recalculate noticable, NFC.
Sat, May 29, 10:51 AM
lattner closed D103367: [Dominance] Speed up recalculate noticable, NFC..
Sat, May 29, 10:50 AM · Restricted Project
lattner accepted D103367: [Dominance] Speed up recalculate noticable, NFC..

Self approving as "obvious"

Sat, May 29, 10:50 AM · Restricted Project
lattner requested review of D103367: [Dominance] Speed up recalculate noticable, NFC..
Sat, May 29, 10:50 AM · Restricted Project
lattner committed rGd11abdfd5a27: [Verifier] Inline a method to simplify the code in preparation for bigger… (authored by lattner).
[Verifier] Inline a method to simplify the code in preparation for bigger…
Sat, May 29, 10:34 AM
lattner closed D103365: [Verifier] Inline a method to simplify the code in preparation for bigger changes, NFC..
Sat, May 29, 10:34 AM · Restricted Project
lattner accepted D103365: [Verifier] Inline a method to simplify the code in preparation for bigger changes, NFC..

Self approving as "obvious"

Sat, May 29, 10:33 AM · Restricted Project
lattner requested review of D103365: [Verifier] Inline a method to simplify the code in preparation for bigger changes, NFC..
Sat, May 29, 10:33 AM · Restricted Project
lattner accepted D103353: [mlir][NFC] Split the non-templated bits out of IROperand into a base class.

This is really great River, thank you and nice work!

Sat, May 29, 9:50 AM · Restricted Project

Fri, May 28

lattner committed rGbde21b624585: [Verifier] Significantly speed up IsolatedFromAbove checking. NFC. (authored by lattner).
[Verifier] Significantly speed up IsolatedFromAbove checking. NFC.
Fri, May 28, 4:14 PM
lattner closed D103345: [Verifier] Significantly speed up IsolatedFromAbove checking. NFC..
Fri, May 28, 4:14 PM · Restricted Project
lattner updated the diff for D103345: [Verifier] Significantly speed up IsolatedFromAbove checking. NFC..

Improvements suggested by River

Fri, May 28, 4:13 PM · Restricted Project
lattner added a comment to D103345: [Verifier] Significantly speed up IsolatedFromAbove checking. NFC..

Thank you for the quick review!

Fri, May 28, 4:02 PM · Restricted Project
lattner added reviewers for D103345: [Verifier] Significantly speed up IsolatedFromAbove checking. NFC.: mehdi_amini, jpienaar.
Fri, May 28, 3:13 PM · Restricted Project
lattner requested review of D103345: [Verifier] Significantly speed up IsolatedFromAbove checking. NFC..
Fri, May 28, 3:12 PM · Restricted Project

Tue, May 25

lattner committed rGaaa2982d7191: [MLIR Core] Cache the empty StringAttr like we do for empty dictionaries. NFC. (authored by lattner).
[MLIR Core] Cache the empty StringAttr like we do for empty dictionaries. NFC.
Tue, May 25, 2:58 PM
lattner closed D103117: [MLIR Core] Cache the empty StringAttr like we do for empty dictionaries. NFC..
Tue, May 25, 2:58 PM · Restricted Project
lattner updated the diff for D103117: [MLIR Core] Cache the empty StringAttr like we do for empty dictionaries. NFC..

Move emptyString and rename it to be more similar to other cached attrs.

Tue, May 25, 2:57 PM · Restricted Project
lattner added a comment to D103117: [MLIR Core] Cache the empty StringAttr like we do for empty dictionaries. NFC..

Good call, thanks for the review!

Tue, May 25, 2:52 PM · Restricted Project
lattner committed rGa6a57f03be40: [Toy] Update tests to pass with top-down canonicalize pass. NFC (authored by lattner).
[Toy] Update tests to pass with top-down canonicalize pass. NFC
Tue, May 25, 2:51 PM
lattner added a comment to D103053: [Canonicalize] Switch the default setting to "top down"..

Fixed in a6a57f03be40, thanks for the head's up!

Tue, May 25, 2:51 PM · Restricted Project
lattner added a comment to D103053: [Canonicalize] Switch the default setting to "top down"..

taking a look

Tue, May 25, 2:40 PM · Restricted Project
lattner requested review of D103117: [MLIR Core] Cache the empty StringAttr like we do for empty dictionaries. NFC..
Tue, May 25, 2:38 PM · Restricted Project
lattner committed rGa004da0d77c4: [Canonicalize] Switch the default setting to "top down". (authored by lattner).
[Canonicalize] Switch the default setting to "top down".
Tue, May 25, 1:42 PM