Page MenuHomePhabricator

GeorgeLyon (George)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 20 2020, 10:14 AM (35 w, 2 d)

Recent Activity

May 24 2021

GeorgeLyon committed rGd3e6c2ddc3d3: Surface clone APIs in CAPI (authored by GeorgeLyon).
Surface clone APIs in CAPI
May 24 2021, 11:54 AM
GeorgeLyon closed D102987: Surface clone APIs in CAPI.
May 24 2021, 11:54 AM · Restricted Project
GeorgeLyon updated the diff for D102987: Surface clone APIs in CAPI.
  • Remove suprious function
May 24 2021, 11:45 AM · Restricted Project
GeorgeLyon added inline comments to D102987: Surface clone APIs in CAPI.
May 24 2021, 11:19 AM · Restricted Project
GeorgeLyon updated the diff for D102987: Surface clone APIs in CAPI.
  • Remove mlirModuleClone
May 24 2021, 11:18 AM · Restricted Project

May 23 2021

GeorgeLyon added inline comments to D102987: Surface clone APIs in CAPI.
May 23 2021, 6:43 PM · Restricted Project
GeorgeLyon updated the diff for D102987: Surface clone APIs in CAPI.
  • Address feedback
May 23 2021, 6:41 PM · Restricted Project
GeorgeLyon added reviewers for D102987: Surface clone APIs in CAPI: mehdi_amini, stellaraccident, mikeurbach, ftynse.
May 23 2021, 12:20 PM · Restricted Project
GeorgeLyon updated the diff for D102987: Surface clone APIs in CAPI.

Test

May 23 2021, 12:19 PM · Restricted Project
GeorgeLyon updated the diff for D102987: Surface clone APIs in CAPI.

Test

May 23 2021, 12:18 PM · Restricted Project
GeorgeLyon updated the diff for D102987: Surface clone APIs in CAPI.

Fix comment

May 23 2021, 12:18 PM · Restricted Project
GeorgeLyon requested review of D102987: Surface clone APIs in CAPI.
May 23 2021, 10:50 AM · Restricted Project

Feb 9 2021

GeorgeLyon retitled D96321: [MLIR] Add context accessor to identifier from [MLIR} Add context accessor to identifier to [MLIR] Add context accessor to identifier.
Feb 9 2021, 12:48 PM · Restricted Project

Feb 8 2021

GeorgeLyon retitled D96321: [MLIR] Add context accessor to identifier from Add context accessor to identifier to [MLIR} Add context accessor to identifier.
Feb 8 2021, 10:28 PM · Restricted Project
GeorgeLyon requested review of D96321: [MLIR] Add context accessor to identifier.
Feb 8 2021, 10:27 PM · Restricted Project
GeorgeLyon updated the diff for D96229: [MLIR] Replace dialect registration hooks with dialect handle.

Slight tewak

Feb 8 2021, 10:26 PM · Restricted Project
GeorgeLyon added inline comments to D96229: [MLIR] Replace dialect registration hooks with dialect handle.
Feb 8 2021, 10:26 PM · Restricted Project
GeorgeLyon added inline comments to D96229: [MLIR] Replace dialect registration hooks with dialect handle.
Feb 8 2021, 10:10 PM · Restricted Project
GeorgeLyon updated the diff for D96229: [MLIR] Replace dialect registration hooks with dialect handle.

Address feedback

Feb 8 2021, 10:08 PM · Restricted Project
GeorgeLyon added a reviewer for D96229: [MLIR] Replace dialect registration hooks with dialect handle: ftynse.
Feb 8 2021, 8:20 PM · Restricted Project
GeorgeLyon updated the summary of D96301: [MLIR] Add C API for navigating up the IR tree.
Feb 8 2021, 7:33 PM · Restricted Project
GeorgeLyon updated the diff for D96301: [MLIR] Add C API for navigating up the IR tree.

Rebase

Feb 8 2021, 6:37 PM · Restricted Project
GeorgeLyon updated the diff for D96301: [MLIR] Add C API for navigating up the IR tree.

Missed one

Feb 8 2021, 6:28 PM · Restricted Project
GeorgeLyon updated the diff for D96301: [MLIR] Add C API for navigating up the IR tree.

Missed a few

Feb 8 2021, 6:25 PM · Restricted Project
GeorgeLyon updated the diff for D96301: [MLIR] Add C API for navigating up the IR tree.

Add Tests

Feb 8 2021, 6:22 PM · Restricted Project
GeorgeLyon added a reviewer for D96301: [MLIR] Add C API for navigating up the IR tree: ftynse.
Feb 8 2021, 4:34 PM · Restricted Project
GeorgeLyon updated the diff for D96301: [MLIR] Add C API for navigating up the IR tree.

Add context accessor on operation

Feb 8 2021, 3:59 PM · Restricted Project
GeorgeLyon retitled D96301: [MLIR] Add C API for navigating up the IR tree from Add C API for navigating up the IR tree to [MLIR] Add C API for navigating up the IR tree.
Feb 8 2021, 3:47 PM · Restricted Project
GeorgeLyon requested review of D96301: [MLIR] Add C API for navigating up the IR tree.
Feb 8 2021, 3:46 PM · Restricted Project
GeorgeLyon added inline comments to D96229: [MLIR] Replace dialect registration hooks with dialect handle.
Feb 8 2021, 9:38 AM · Restricted Project

Feb 7 2021

GeorgeLyon retitled D96229: [MLIR] Replace dialect registration hooks with dialect handle from Replace hooks with dialect handle to [MLIR] Replace dialect registration hooks with dialect handle.
Feb 7 2021, 5:31 PM · Restricted Project
GeorgeLyon reclaimed D96229: [MLIR] Replace dialect registration hooks with dialect handle.
Feb 7 2021, 5:20 PM · Restricted Project
GeorgeLyon abandoned D96229: [MLIR] Replace dialect registration hooks with dialect handle.
Feb 7 2021, 5:13 PM · Restricted Project
GeorgeLyon requested review of D96229: [MLIR] Replace dialect registration hooks with dialect handle.
Feb 7 2021, 5:12 PM · Restricted Project

Feb 3 2021

GeorgeLyon updated the diff for D95968: Add API for adding arguments to blocks.

Add test

Feb 3 2021, 1:14 PM · Restricted Project
GeorgeLyon added a comment to D95968: Add API for adding arguments to blocks.

It looks like there are three tests failing currently on master for me, but they also fail for the previous commit (8d73bee4edc2)
[build] Failed Tests (3):
[build] MLIR :: mlir-cpu-runner/async-group.mlir
[build] MLIR :: mlir-cpu-runner/async-value.mlir
[build] MLIR :: mlir-cpu-runner/async.mlir

Feb 3 2021, 1:13 PM · Restricted Project
GeorgeLyon updated the summary of D95968: Add API for adding arguments to blocks.
Feb 3 2021, 12:32 PM · Restricted Project
GeorgeLyon requested review of D95968: Add API for adding arguments to blocks.
Feb 3 2021, 12:31 PM · Restricted Project

Jan 9 2021

GeorgeLyon accepted D94370: [mlir][CAPI] Introduce standard source layout for mlir-c dialect registration..

It is great to see a unified approach to dialects, this will make further improvements much easier. Thanks, Stella!

Jan 9 2021, 7:15 PM · Restricted Project

Dec 15 2020

GeorgeLyon added reviewers for D93334: Add call site location getter to C API: mehdi_amini, stellaraccident.
Dec 15 2020, 12:36 PM · Restricted Project
GeorgeLyon requested review of D93334: Add call site location getter to C API.
Dec 15 2020, 12:35 PM · Restricted Project

Dec 3 2020

GeorgeLyon closed D92613: Use MlirStringRef in StandardAttributes.h.

Accidentally committed without "Differential" log: https://github.com/llvm/llvm-project/commit/5f65c4a8e6a9fbcbf45ff4cdf0b4815795dd4845

Dec 3 2020, 4:25 PM · Restricted Project
GeorgeLyon added a comment to D92613: Use MlirStringRef in StandardAttributes.h.

Whoops, I accidentally pushed without the "Differential" comment... anything I should do to clean this up?

Dec 3 2020, 4:14 PM · Restricted Project
GeorgeLyon updated the diff for D92613: Use MlirStringRef in StandardAttributes.h.

Fix comments

Dec 3 2020, 4:01 PM · Restricted Project
GeorgeLyon abandoned D92279: Add SCF Dialect C API.
Dec 3 2020, 3:26 PM · Restricted Project
GeorgeLyon updated the summary of D92613: Use MlirStringRef in StandardAttributes.h.
Dec 3 2020, 3:24 PM · Restricted Project
GeorgeLyon requested review of D92613: Use MlirStringRef in StandardAttributes.h.
Dec 3 2020, 3:23 PM · Restricted Project

Nov 29 2020

GeorgeLyon accepted D92292: [mlir][CAPI] Convert the rest of the API int -> bool..

LGTM. Thanks for doing this!

Nov 29 2020, 6:39 PM · Restricted Project
GeorgeLyon added a comment to D92193: Use bool in place of int for boolean things in the C API.

Only the helpers that are defined in the header are static inline. I think that the intent was to not exposing them in libMLIR.so and have them inlined in the client. I am not sure this is valid in C though: they has to be an external definition I believe and this would break if a client was taking the address.

Nov 29 2020, 4:34 PM · Restricted Project
GeorgeLyon added a comment to D92279: Add SCF Dialect C API.

The code in this revision should all be part of the TableGen generated code and not manually written.

Nov 29 2020, 4:22 PM · Restricted Project
GeorgeLyon added a comment to D92279: Add SCF Dialect C API.

That said, I think it would be nice to include a second dialect in the bindings as we work our way to a better system (and I need it for my test cases :-)

Nov 29 2020, 12:11 PM · Restricted Project
GeorgeLyon added a comment to D92279: Add SCF Dialect C API.

I very much agree @stellaraccident. I would even go one step further and say there should be an mlirGetDialectByNamespace(MlirStringRef) function to return this structure, which we can enable using static constructors (even though they are evil, this would be a very nice way to not have to create a CAPI for every dialect). For the record, in the Swift bindings I use "Dialect" to mean what you refer to as "Dialect Registration" and "Dialect Instance" to refer to the thing returned by getOrLoad. I also don't currently understand how Dialect Instances are useful (and why we can't just have "Dialect Registration" be the only thing (hence "Dialect")

Nov 29 2020, 12:10 PM · Restricted Project
GeorgeLyon added reviewers for D92279: Add SCF Dialect C API: mehdi_amini, ftynse, stellaraccident.
Nov 29 2020, 10:16 AM · Restricted Project
GeorgeLyon requested review of D92279: Add SCF Dialect C API.
Nov 29 2020, 10:15 AM · Restricted Project
GeorgeLyon added a comment to D92193: Use bool in place of int for boolean things in the C API.

I keep getting this error saying that the Debian build failed, though I don't see any errors:

Testing Time: 121.11s
  Unsupported      :   584
  Passed           : 40988
  Expectedly Failed:   174
Nov 29 2020, 10:10 AM · Restricted Project
GeorgeLyon updated the diff for D92252: Use `const` for array pointers in `StandardTypes.h`.

Add some missing consts

Nov 29 2020, 9:46 AM · Restricted Project
GeorgeLyon updated the diff for D92193: Use bool in place of int for boolean things in the C API.

Let's give Debian another chance...

Nov 29 2020, 9:02 AM · Restricted Project
GeorgeLyon updated the diff for D92252: Use `const` for array pointers in `StandardTypes.h`.

Rebasing again

Nov 29 2020, 9:00 AM · Restricted Project

Nov 28 2020

GeorgeLyon updated the diff for D92252: Use `const` for array pointers in `StandardTypes.h`.

Rebasing to re-run CI

Nov 28 2020, 10:10 AM · Restricted Project
GeorgeLyon updated the diff for D92193: Use bool in place of int for boolean things in the C API.

Rebasing to re-run CI

Nov 28 2020, 10:08 AM · Restricted Project

Nov 27 2020

GeorgeLyon updated the diff for D92252: Use `const` for array pointers in `StandardTypes.h`.

Try again after rebase

Nov 27 2020, 4:57 PM · Restricted Project
GeorgeLyon updated the diff for D92252: Use `const` for array pointers in `StandardTypes.h`.

Fix diff

Nov 27 2020, 4:55 PM · Restricted Project
GeorgeLyon retitled D92252: Use `const` for array pointers in `StandardTypes.h` from More const pointers to Use `const` for array pointers in `StandardTypes.h` .
Nov 27 2020, 4:52 PM · Restricted Project
GeorgeLyon requested review of D92252: Use `const` for array pointers in `StandardTypes.h`.
Nov 27 2020, 4:51 PM · Restricted Project
GeorgeLyon added a comment to D92193: Use bool in place of int for boolean things in the C API.

I'm guessing it is safe to ignore the unused function warnings? Also, It looks like the build failure was unrelated to the code change... is there any way to re-run a build?

Nov 27 2020, 1:07 PM · Restricted Project
GeorgeLyon updated the summary of D92193: Use bool in place of int for boolean things in the C API.
Nov 27 2020, 1:06 PM · Restricted Project

Nov 26 2020

GeorgeLyon added a comment to D92192: Fix appending-zero issues.

Sorry I abandoned this because the situation I thought I was able to reproduce, stopped reproducing. I think the underlying issue was unrelated

Nov 26 2020, 8:23 PM · Restricted Project
GeorgeLyon retitled D92193: Use bool in place of int for boolean things in the C API from Use bool in place of int for boolean things to Use bool in place of int for boolean things in the C API.
Nov 26 2020, 12:34 PM · Restricted Project
GeorgeLyon updated the summary of D92193: Use bool in place of int for boolean things in the C API.
Nov 26 2020, 12:33 PM · Restricted Project
GeorgeLyon requested review of D92193: Use bool in place of int for boolean things in the C API.
Nov 26 2020, 12:31 PM · Restricted Project
GeorgeLyon abandoned D92192: Fix appending-zero issues.
Nov 26 2020, 12:08 PM · Restricted Project
GeorgeLyon requested review of D92192: Fix appending-zero issues.
Nov 26 2020, 12:02 PM · Restricted Project

Nov 24 2020

GeorgeLyon updated subscribers of D92007: [mlir][Python] Sync Python bindings with C API MlirStringRef modification..

The difference is that all the consumers of the C++ API are C++, so it is pretty straightforward to update when making breaking changes even if the surface is greater. Language bindings will live at least partially in their own ecosystem and requiring folks who work on the C API to understand each individual ecosystem may be too onerous, even if the surface area is smaller. To be clear, I’m just saying there is a tradeoff here, not advocating for us to decide on a particular point on that spectrum. I agree with Stella that we can wait until this becomes a bigger issue and then have a more formal discussion (hopefully with more data in the form of diverse language bindings).

Nov 24 2020, 11:05 AM · Restricted Project
GeorgeLyon added a comment to D92007: [mlir][Python] Sync Python bindings with C API MlirStringRef modification..

Small process question: I authored the change to us MlirStringRef instead of const char *, what is the policy about breaking changes in the C API? I think it is likely unreasonable to expect any change in the C API to fix all in-tree language bindings, but since the Python bindings are in-tree, these types of changes can break them. I'm might be the only person working on non-Python bindings here, so I may be biased :)

Nov 24 2020, 9:26 AM · Restricted Project

Nov 23 2020

GeorgeLyon updated the diff for D91905: Use MlirStringRef throughout the C API.

Some more formatting fixes

Nov 23 2020, 12:43 PM · Restricted Project
GeorgeLyon abandoned D91991: Some more formatting fixes.
Nov 23 2020, 12:42 PM · Restricted Project
GeorgeLyon removed rG LLVM Github Monorepo as the repository for D91991: Some more formatting fixes.
Nov 23 2020, 12:42 PM · Restricted Project
GeorgeLyon requested review of D91991: Some more formatting fixes.
Nov 23 2020, 12:41 PM · Restricted Project
GeorgeLyon added a comment to D91905: Use MlirStringRef throughout the C API.

Sorry for the spam, I am still learning my way around arc diff. This should be ready now though.

Nov 23 2020, 11:48 AM · Restricted Project
GeorgeLyon retitled D91905: Use MlirStringRef throughout the C API from Allow Location to be created with a string ref to Use MlirStringRef throughout the C API.
Nov 23 2020, 11:48 AM · Restricted Project
GeorgeLyon updated the diff for D91905: Use MlirStringRef throughout the C API.

Use MlirStringRef throught the C API

Nov 23 2020, 11:46 AM · Restricted Project
GeorgeLyon updated the diff for D91905: Use MlirStringRef throughout the C API.

Use MlirStringRef for all strings in the C API

Nov 23 2020, 11:43 AM · Restricted Project
GeorgeLyon updated the diff for D91905: Use MlirStringRef throughout the C API.

Use MlirStringRef for all strings in the C API

Nov 23 2020, 11:40 AM · Restricted Project
GeorgeLyon updated the diff for D91905: Use MlirStringRef throughout the C API.

Fix formatting

Nov 23 2020, 11:37 AM · Restricted Project
GeorgeLyon updated the diff for D91905: Use MlirStringRef throughout the C API.

Use MlirStringRef throughout the API.

Nov 23 2020, 11:34 AM · Restricted Project

Nov 21 2020

GeorgeLyon added a comment to D91905: Use MlirStringRef throughout the C API.

To be perfectly clear, I agree that we should use MlirStringRef throughout, but if we decide that we should do that across the API in one fell swoop (I’m happy to author that PR if folks are on board).

Nov 21 2020, 8:57 PM · Restricted Project

Nov 20 2020

GeorgeLyon updated the diff for D91905: Use MlirStringRef throughout the C API.

Removing spurious formatting changes

Nov 20 2020, 6:50 PM · Restricted Project
GeorgeLyon updated the summary of D91905: Use MlirStringRef throughout the C API.
Nov 20 2020, 6:48 PM · Restricted Project
GeorgeLyon requested review of D91905: Use MlirStringRef throughout the C API.
Nov 20 2020, 6:41 PM · Restricted Project
GeorgeLyon added a comment to D91738: Add userData to the diagnostic handler C API.

I have worked with many C APIs in a number of languages and the attitude of "just do the memory management bookkeeping in the host language" has created some very unfortunate complexity in the bindings and more than a few bugs. This is a very low-cost change for the C bindings, and doesn't add much complexity for language bindings that don't care about this functionality (they can just pass NULL to the extra arguments). I think this is also a win from a self-documentation standpoint: before, I would need to go and figure out what are the different ways diagnostic handlers can be unregistered to understand the semantics. I would also need to worry about concurrency, specifically whether or not my handler is guaranteed to not be called after detach returns. I could read the code to figure this out (it turns out, the answer is "no") but I think it is better to minimize this sort of spelunking and just say "we will call your handler and when we are done calling your handler we will call your callback".

Nov 20 2020, 10:25 AM · Restricted Project
GeorgeLyon updated the diff for D91738: Add userData to the diagnostic handler C API.

Address feedback

Nov 20 2020, 10:25 AM · Restricted Project

Nov 19 2020

GeorgeLyon added a comment to D91738: Add userData to the diagnostic handler C API.

If a host language is registering dialect handlers that may need to be cleaned up (for instance, via a reference count decrement), it theoretically _could_ keep a list of attached handlers and manually release them on detach or when the context is destroyed. deleteUserData is a simple way to eliminate this bit of bookkeeping and make "ownership" a bit more formal in this case, namely the context is responsible for telling you when it is done using the handler (and thus, the userData). This way, if we hypothetically add a setHandlers method which takes an array of handles and _replaces_ the current handler stack, then the host language wouldn't need to account for this, deleteUserData would just be called when the handler is no longer in use.

Nov 19 2020, 4:36 PM · Restricted Project
GeorgeLyon added a comment to D91738: Add userData to the diagnostic handler C API.

@mehdi_amini I added one more bit of functionality here, so you may want to take another look.

Nov 19 2020, 2:17 PM · Restricted Project
GeorgeLyon updated the diff for D91738: Add userData to the diagnostic handler C API.

Added deleteUserData callback

Nov 19 2020, 2:16 PM · Restricted Project
GeorgeLyon accepted D91822: [mlir] Add missing const * updates in StandardAttributes.

Whoops, thank you for catching this... these must have disappeared when I was fixing MLIR_CAPI_EXPORTED-related merge conflicts

Nov 19 2020, 1:54 PM · Restricted Project

Nov 18 2020

GeorgeLyon updated the summary of D91738: Add userData to the diagnostic handler C API.
Nov 18 2020, 1:22 PM · Restricted Project
GeorgeLyon updated the summary of D91740: Make array pointers in the CAPI const.
Nov 18 2020, 1:20 PM · Restricted Project
GeorgeLyon requested review of D91740: Make array pointers in the CAPI const.
Nov 18 2020, 1:18 PM · Restricted Project
GeorgeLyon requested review of D91738: Add userData to the diagnostic handler C API.
Nov 18 2020, 1:07 PM · Restricted Project