Page MenuHomePhabricator

GMNGeoffrey (Geoffrey Martin-Noble)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 15 2020, 6:41 PM (20 w, 21 h)

Recent Activity

Wed, May 27

GMNGeoffrey added a comment to D79640: Add Operation::moveAfter.

Pardon my phabricator ignorance, but why is the comment here "This revision was not accepted when it landed; it landed in state Needs Review" immediately below "mehdi_amini accepted this revision"? Is that because River is marked as a blocking reviewer? Seems like a fragile system to have one person be the only one who can approve some set of code. What if they go on vacation?

In general if you are touching something and the code owner is on vacation you wait for them to get back. Whether you care about getting a proper review from the code owner or not is up to you morally. If you don't care and there is a proper alternative reviewer, then you don't necessarily have to wait, but to each their own...

Wed, May 27, 1:02 PM · Restricted Project

Fri, May 8

GMNGeoffrey added a comment to D79640: Add Operation::moveAfter.

Pardon my phabricator ignorance, but why is the comment here "This revision was not accepted when it landed; it landed in state Needs Review" immediately below "mehdi_amini accepted this revision"? Is that because River is marked as a blocking reviewer? Seems like a fragile system to have one person be the only one who can approve some set of code. What if they go on vacation?

Fri, May 8, 4:08 PM · Restricted Project
GMNGeoffrey added a comment to D79640: Add Operation::moveAfter.

Mehdi, can you land this?

Fri, May 8, 1:57 PM · Restricted Project
GMNGeoffrey updated the summary of D79640: Add Operation::moveAfter.
Fri, May 8, 12:20 PM · Restricted Project
GMNGeoffrey added inline comments to D79640: Add Operation::moveAfter.
Fri, May 8, 11:47 AM · Restricted Project
GMNGeoffrey created D79640: Add Operation::moveAfter.
Fri, May 8, 11:47 AM · Restricted Project
GMNGeoffrey added a reviewer for D79640: Add Operation::moveAfter: mehdi_amini.
Fri, May 8, 11:47 AM · Restricted Project

May 4 2020

GMNGeoffrey added a comment to D79377: [mlir] Remove tabs from predecessor comments.

Thanks Mehdi :-)

May 4 2020, 7:55 PM · Restricted Project
GMNGeoffrey added a comment to D79377: [mlir] Remove tabs from predecessor comments.

Can someone land this for me, since I don't have commit access?

May 4 2020, 5:46 PM · Restricted Project
GMNGeoffrey created D79377: [mlir] Remove tabs from predecessor comments.
May 4 2020, 4:41 PM · Restricted Project

Apr 24 2020

GMNGeoffrey added inline comments to D78790: [mlir][DialectConversion] Add support for properly tracking replaceUsesOfBlockArgument.
Apr 24 2020, 3:12 PM · Restricted Project
GMNGeoffrey accepted D78790: [mlir][DialectConversion] Add support for properly tracking replaceUsesOfBlockArgument.

Thanks!

Apr 24 2020, 10:15 AM · Restricted Project

Mar 18 2020

GMNGeoffrey updated the diff for D76329: [MLIR] Deduplicate dialect registration by ClassID.

Break templates declaration to placate clang format pre-merge.

Mar 18 2020, 6:28 PM · Restricted Project
GMNGeoffrey added a comment to D76329: [MLIR] Deduplicate dialect registration by ClassID.

Looks like the failure is a clang format diff. I think this is related to the discussion in https://llvm.discourse.group/t/remove-mlir-custom-clang-format-configuration/647/4? I can reformat as needed, but +1 for unifying LLVM and MLIR configs

Mar 18 2020, 6:28 PM · Restricted Project
GMNGeoffrey updated the summary of D76329: [MLIR] Deduplicate dialect registration by ClassID.
Mar 18 2020, 5:55 PM · Restricted Project
GMNGeoffrey added a comment to D76329: [MLIR] Deduplicate dialect registration by ClassID.

River if this LGTY, could you please commit it?

Mar 18 2020, 5:55 PM · Restricted Project
GMNGeoffrey added inline comments to D76329: [MLIR] Deduplicate dialect registration by ClassID.
Mar 18 2020, 5:25 PM · Restricted Project
GMNGeoffrey updated the diff for D76329: [MLIR] Deduplicate dialect registration by ClassID.

Stable maps and private registration.

Mar 18 2020, 5:24 PM · Restricted Project

Mar 17 2020

GMNGeoffrey added a comment to D76329: [MLIR] Deduplicate dialect registration by ClassID.

LGTM, but I'd like to hear from River as well.

Yeah I'd also like Rivers' review

I am not sure what you mean with:

This potentially introduces some issues if callers using non-standard dialect registration functions passed to registerDialectAllocator and they are silently overridden.

If people call registerDialect<Foo>() we're all good as if there's a duplicate, it would have been the same thing anyway. If someone is directly calling registerDialectAllocator (which I don't think anyone is right now) and they pass in their custom allocator that they expect to do something and someone else also registers the same dialect (either with registerDialect or registerDialectAllocator) then we might have surprising behavior. We could move registerDialectAllocator out of the public API, although it's used by the template function, so a bit tricky (maybe just an impl/detail namespace?), but I wanted to hear from others before doing that.

Mar 17 2020, 6:56 PM · Restricted Project
GMNGeoffrey created D76329: [MLIR] Deduplicate dialect registration by ClassID.
Mar 17 2020, 3:07 PM · Restricted Project

Jan 15 2020

GMNGeoffrey updated the diff for D72821: [mlir] Add missing dependency on LinalgUtils.

Remove LinalgUtils dependency from test transforms

Jan 15 2020, 7:48 PM · Restricted Project
GMNGeoffrey added a comment to D72821: [mlir] Add missing dependency on LinalgUtils.

LinalgToSpirv is only in the IREE project, but I removed the extra dependency from the test transforms, which I think was masking this error as you describe.

Jan 15 2020, 7:48 PM · Restricted Project
GMNGeoffrey added a comment to D72821: [mlir] Add missing dependency on LinalgUtils.

This is my first contribution now that MLIR has moved to the monorepo. Feedback on what I got wrong welcome

Jan 15 2020, 7:11 PM · Restricted Project
GMNGeoffrey created D72821: [mlir] Add missing dependency on LinalgUtils.
Jan 15 2020, 7:11 PM · Restricted Project