Page MenuHomePhabricator

antiagainst (Lei Zhang)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 24 2019, 5:47 AM (9 w, 2 d)

Recent Activity

Yesterday

antiagainst added a comment to D75195: [mlir][spirv] Add some folders for spv.LogicalAnd/spv.LogicalOr.

Comments addressed as https://github.com/llvm/llvm-project/commit/63779fb462d828d16b87f427a6490dded842ca15

Wed, Feb 26, 12:39 PM · Restricted Project
antiagainst committed rG63779fb462d8: [mlir][spirv] Refactoring to avoid calling the same function twice (authored by antiagainst).
[mlir][spirv] Refactoring to avoid calling the same function twice
Wed, Feb 26, 12:37 PM
antiagainst added a comment to D75195: [mlir][spirv] Add some folders for spv.LogicalAnd/spv.LogicalOr.

Oops. Pushed an older commit without squashing fixes...

Wed, Feb 26, 12:26 PM · Restricted Project
antiagainst committed rG5bc6ff6455ec: [mlir][spirv] Add some folders for spv.LogicalAnd/spv.LogicalOr (authored by antiagainst).
[mlir][spirv] Add some folders for spv.LogicalAnd/spv.LogicalOr
Wed, Feb 26, 12:18 PM
antiagainst closed D75195: [mlir][spirv] Add some folders for spv.LogicalAnd/spv.LogicalOr.
Wed, Feb 26, 12:18 PM · Restricted Project
antiagainst committed rG1e9321e97aba: [mlir][spirv] NFC: move folders and canonicalizers in a separate file (authored by antiagainst).
[mlir][spirv] NFC: move folders and canonicalizers in a separate file
Wed, Feb 26, 9:51 AM
antiagainst created D75195: [mlir][spirv] Add some folders for spv.LogicalAnd/spv.LogicalOr.
Wed, Feb 26, 9:50 AM · Restricted Project
antiagainst closed D75170: [mlir][spirv] NFC: move folders and canonicalizers in a separate file.

Thanks Denis!

Wed, Feb 26, 9:50 AM · Restricted Project
antiagainst updated the diff for D75170: [mlir][spirv] NFC: move folders and canonicalizers in a separate file.

Add file to CMakeLists.txt

Wed, Feb 26, 6:14 AM · Restricted Project
antiagainst created D75170: [mlir][spirv] NFC: move folders and canonicalizers in a separate file.
Wed, Feb 26, 6:14 AM · Restricted Project

Mon, Feb 24

antiagainst committed rG8358ddbe5d32: [mlir][spirv] NFC: Move test passes to test/lib (authored by antiagainst).
[mlir][spirv] NFC: Move test passes to test/lib
Mon, Feb 24, 11:17 AM
antiagainst closed D75066: [mlir][spirv] NFC: Move test passes to test/lib.
Mon, Feb 24, 11:17 AM · Restricted Project
antiagainst accepted D74893: [mlir] Intrinsics generator: use TableGen-defined builder function.

Nice clean up!

Mon, Feb 24, 11:16 AM · Restricted Project
antiagainst added a comment to D74143: [MLIR] Adding attribute setters generation for table gen.

To truly allow dialect writers to dictate what naming style they want to use, the dialect writer need to control what naming convention is used to generate these attribute getter methods too.

I suspect any attempt to provide such facility (switching naming convention) will hit the issue of mismatch with Traits and inherited properties: I think it will require a major change in our infrastructure to provide this.

Mon, Feb 24, 11:08 AM · Restricted Project
antiagainst created D75066: [mlir][spirv] NFC: Move test passes to test/lib.
Mon, Feb 24, 10:48 AM · Restricted Project

Fri, Feb 21

antiagainst added a comment to D74681: [mlir][DeclarativeParser] Add basic support for optional groups in the assembly format..

Don't forget to update the CL message too.

Fri, Feb 21, 1:53 PM · Restricted Project
antiagainst accepted D74681: [mlir][DeclarativeParser] Add basic support for optional groups in the assembly format..

Nice! Thanks!

Fri, Feb 21, 1:53 PM · Restricted Project
antiagainst accepted D74789: [mlir][DeclarativeParser] Add support for formatting the successors of an operation..

Nice!

Fri, Feb 21, 12:46 PM · Restricted Project
antiagainst accepted D74783: [mlir][ODS] Add support for specifying the successors of an operation..

Cool! Thanks River for adding this! Can we also update the doc ?

Fri, Feb 21, 12:31 PM · Restricted Project
antiagainst added a comment to D74681: [mlir][DeclarativeParser] Add basic support for optional groups in the assembly format..

Implementation looks good to me.

Fri, Feb 21, 12:22 PM · Restricted Project
antiagainst accepted D74682: [mlir][DeclarativeParser] Add an 'attr-dict-with-keyword' directive.
Fri, Feb 21, 12:22 PM · Restricted Project
antiagainst added inline comments to D74681: [mlir][DeclarativeParser] Add basic support for optional groups in the assembly format..
Fri, Feb 21, 12:08 PM · Restricted Project
antiagainst added inline comments to D74681: [mlir][DeclarativeParser] Add basic support for optional groups in the assembly format..
Fri, Feb 21, 12:08 PM · Restricted Project
antiagainst accepted D74648: [mlir][DeclarativeParser] Add support for the TypesMatchWith trait..

Nice! Can we also update the doc regarding which traits are supported? Otherwise it's quite obscure for somebody to figure out what works and what not.

Fri, Feb 21, 12:04 PM · Restricted Project
antiagainst committed rG35b685270b41: [mlir] Add a signedness semantics bit to IntegerType (authored by antiagainst).
[mlir] Add a signedness semantics bit to IntegerType
Fri, Feb 21, 6:24 AM
antiagainst closed D72533: [mlir] Add a signedness semantics bit to IntegerType.
Fri, Feb 21, 6:24 AM · Restricted Project
antiagainst added inline comments to D72533: [mlir] Add a signedness semantics bit to IntegerType.
Fri, Feb 21, 6:24 AM · Restricted Project

Wed, Feb 19

antiagainst committed rG476ca094c846: [mlir][ods] Adding attribute setters generation (authored by AlexEichenberger).
[mlir][ods] Adding attribute setters generation
Wed, Feb 19, 8:53 AM
antiagainst closed D74143: [MLIR] Adding attribute setters generation for table gen.
Wed, Feb 19, 8:53 AM · Restricted Project
antiagainst committed rG896ee361a641: [mlir][spirv] Add mlir-vulkan-runner (authored by denis13).
[mlir][spirv] Add mlir-vulkan-runner
Wed, Feb 19, 8:43 AM
antiagainst closed D72696: [mlir][spirv] Add mlir-vulkan-runner.
Wed, Feb 19, 8:42 AM · Restricted Project
antiagainst accepted D72696: [mlir][spirv] Add mlir-vulkan-runner.

Awesome! Thanks again, Denis! This is very helpful!!

Wed, Feb 19, 7:18 AM · Restricted Project

Tue, Feb 18

antiagainst accepted D74647: [mlir][ODS] Add a new trait `TypesMatchWith`.

LGTM!

Tue, Feb 18, 3:47 PM · Restricted Project
antiagainst accepted D74768: [mlir] Support OptionalAttr inside a StructAttr.

Nice, thanks!

Tue, Feb 18, 3:34 PM · Restricted Project
antiagainst accepted D74143: [MLIR] Adding attribute setters generation for table gen.

That seems independent of this though, or are you thinking the variants of attribute methods? E.g., we'll have multiple different convenience accessors generated for the same attribute beyond the raw and convenience one we have today?

Tue, Feb 18, 3:25 PM · Restricted Project

Thu, Feb 13

antiagainst added inline comments to D74549: [mlir][spirv] Add pass ConvertGpuLaunchFuncToVulkanCallsPass..
Thu, Feb 13, 11:11 AM · Restricted Project
antiagainst committed rGa062a3ed7fd8: [mlir][spirv] Add ConvertGpuLaunchFuncToVulkanCallsPass (authored by denis13).
[mlir][spirv] Add ConvertGpuLaunchFuncToVulkanCallsPass
Thu, Feb 13, 11:10 AM
antiagainst closed D74549: [mlir][spirv] Add pass ConvertGpuLaunchFuncToVulkanCallsPass..
Thu, Feb 13, 11:10 AM · Restricted Project
antiagainst accepted D73826: [mlir][Linalg] Provide a Tablegen backend to specify named Linalg ops declaratively.

LGTM, but may want another pair of eyes to check the Linalg specific logic. :)

Thu, Feb 13, 9:28 AM · Restricted Project
antiagainst added a comment to D73826: [mlir][Linalg] Provide a Tablegen backend to specify named Linalg ops declaratively.

The main value I see with the OpDefinitionGen hook is that it allows one to automatically insert traits and methods into the generated C++ op classes. This is preferable when a dialect's ops have a large variation of potential traits and methods, and that information is already encoded with other fields in the op definitions

There is no disagreement that this may be useful for some cases: but we should strive to improve ODS itself to provide this facilities. I don't think we want dialect author to write Tablegen backends: this defeats the whole point of ODS in my opinion!

Thu, Feb 13, 9:19 AM · Restricted Project
antiagainst added a comment to D74143: [MLIR] Adding attribute setters generation for table gen.

I'm concerned about the method name here. Although we can rely on compiler symbol resolution, it's still quite confusing to see two attributes with the same name but one for getters and one for setters. If we also think about the getter method returning "unwrapped" values, that's three.. So if I have a some_stuff attribute, we have some_stuff(), some_stuffAttr(), and some_stuffAttr(some-params). It's also quite sad to see the mix of different naming conventions here. And I think the number of attribute methods is just gonna to grow. It would be far better if we have getSomeStuff(), getSomeStuffAttr(), and setSomeStuff(). FWIW, I still think having proper naming convention and method prefix/postfix control at dialect level is the better way to go given that we don't mandate how attributes should be named for ops.

Thu, Feb 13, 9:01 AM · Restricted Project
antiagainst accepted D74438: [mlir] Allow adding extra class declarations to interfaces..
Thu, Feb 13, 8:15 AM · Restricted Project
antiagainst added a comment to D72696: [mlir][spirv] Add mlir-vulkan-runner.

Thanks a lot for review! I addressed comments related to pass in separate revision https://reviews.llvm.org/D74549 and will update mlir-vulkan-runner, VulkanRuntime as well.

Thu, Feb 13, 8:06 AM · Restricted Project
antiagainst accepted D74549: [mlir][spirv] Add pass ConvertGpuLaunchFuncToVulkanCallsPass..

Nice, thanks Denis! Reviews already happened in D72696 so I just have two more nits here.

Thu, Feb 13, 8:06 AM · Restricted Project
antiagainst accepted D74525: [mlir][DeclarativeParser] Add support for formatting enum attributes in the string form..

Nice! Thanks River for adding support for this!

Thu, Feb 13, 7:56 AM · Restricted Project

Wed, Feb 12

antiagainst requested changes to D72696: [mlir][spirv] Add mlir-vulkan-runner.

Super nice! This is so great! Thanks a ton, Denis! I don't have major concerns given that a large portion of this patch has already went through reviews multiple times previously. Just a few nits. We can land and then iterate on it. :)

Wed, Feb 12, 6:20 PM · Restricted Project
antiagainst committed rGd3e7816d8546: [mlir][spirv] Introduce spv.func (authored by antiagainst).
[mlir][spirv] Introduce spv.func
Wed, Feb 12, 5:09 AM
antiagainst closed D74226: [mlir][spirv] Introduce spv.function.
Wed, Feb 12, 5:09 AM · Restricted Project
antiagainst added inline comments to D74226: [mlir][spirv] Introduce spv.function.
Wed, Feb 12, 4:50 AM · Restricted Project

Tue, Feb 11

antiagainst updated the diff for D74226: [mlir][spirv] Introduce spv.function.

Add comments on FunctionLike

Tue, Feb 11, 4:02 PM · Restricted Project
antiagainst added inline comments to D74226: [mlir][spirv] Introduce spv.function.
Tue, Feb 11, 4:02 PM · Restricted Project
antiagainst accepted D73579: [mlir] Add elementAttr to TypedArrayAttrBase..
Tue, Feb 11, 3:17 PM · Restricted Project
antiagainst added inline comments to D74226: [mlir][spirv] Introduce spv.function.
Tue, Feb 11, 2:50 PM · Restricted Project
antiagainst updated the diff for D74226: [mlir][spirv] Introduce spv.function.

Address comments

Tue, Feb 11, 2:49 PM · Restricted Project
antiagainst added inline comments to D74226: [mlir][spirv] Introduce spv.function.
Tue, Feb 11, 2:49 PM · Restricted Project
antiagainst committed rGb04885a55c2a: [mlir][ods] Added RankedIntElementsAttr class (authored by Joonsoo).
[mlir][ods] Added RankedIntElementsAttr class
Tue, Feb 11, 7:05 AM
antiagainst closed D73764: [mlir] Added RankedIntElementsAttr class.
Tue, Feb 11, 7:05 AM · Restricted Project
antiagainst added a comment to D72533: [mlir] Add a signedness semantics bit to IntegerType.

For the naming part, could we consider using IntegerType for signless integers the way it is now and add SignedIntegerType? i.e., see "Signed" as an addition as opposed to an adjective/specialization, and continuing to maintain that integer types in MLIR are signless.

A couple of things.

  1. The commit message has no mention of 'SignedIntegerType' or 'SignlessIntegerType'. Please add something to the effect "add 'SignedIntegerType' and IntegerType -> SignlessIntegerType".
  2. The last message on https://groups.google.com/a/tensorflow.org/d/msg/mlir/XmkV8HOPWpo/7O4X0Nb_AQAJ was in Jul 2019. Could you mention the naming change on the discussion forum?
Tue, Feb 11, 6:37 AM · Restricted Project
antiagainst updated the diff for D72533: [mlir] Add a signedness semantics bit to IntegerType.
  • Fix Rationale wording
Tue, Feb 11, 6:19 AM · Restricted Project
antiagainst committed rG8d96aed56652: [mlir] Use the first location in the fused location for diagnostic handler (authored by liufengdb).
[mlir] Use the first location in the fused location for diagnostic handler
Tue, Feb 11, 4:49 AM
antiagainst closed D71851: Use the first location in the fused location for diagnostic handler.
Tue, Feb 11, 4:49 AM · Restricted Project

Mon, Feb 10

antiagainst added a comment to D72696: [mlir][spirv] Add mlir-vulkan-runner.

@antiagainst @mravishankar this is an initial version of mlir-vulkan-runner. It's now possible to execute GPU code on Vulkan capable devices via JitRunner.

Mon, Feb 10, 3:00 PM · Restricted Project
antiagainst committed rG50aeeed8a2dd: [mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions (authored by antiagainst).
[mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions
Mon, Feb 10, 1:31 PM
antiagainst closed D74012: [mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions.
Mon, Feb 10, 1:30 PM · Restricted Project
antiagainst added a comment to D74012: [mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions.

@herhut: +1 to what Mahesh said. Additionally, I'd like to tighten SPIR-V side to use attributes in general for passing in pattern configurations.

Mon, Feb 10, 1:04 PM · Restricted Project
antiagainst added a comment to D74226: [mlir][spirv] Introduce spv.function.

I would actually prefer that we call this FuncOp('func'). It is consistent with all of the other functions that have been defined, and seems a little weird to deviate. For example, spirv::ModuleOp/ConstantOp are also named the same as other common ops.

Mon, Feb 10, 7:48 AM · Restricted Project
antiagainst updated the diff for D74226: [mlir][spirv] Introduce spv.function.

Address comments

Mon, Feb 10, 7:48 AM · Restricted Project

Sat, Feb 8

antiagainst accepted D74276: [mlir][DeclarativeParser] Add support for attributes with buildable types..
Sat, Feb 8, 3:13 PM · Restricted Project
antiagainst accepted D74283: [mlir][DeclarativeParser] Move several missed parsers over to the declarative form..

Nice, thanks!

Sat, Feb 8, 3:13 PM · Restricted Project
antiagainst added inline comments to D72533: [mlir] Add a signedness semantics bit to IntegerType.
Sat, Feb 8, 3:05 PM · Restricted Project
antiagainst updated the diff for D72533: [mlir] Add a signedness semantics bit to IntegerType.

Address comments

  • Went through isa<IntergerType> and isInterger(...) use sites to make sure they now use signless integer type where possible.
  • Updated several APIs to include 'Signless' in the name to be explicit.
  • Fixed typos
Sat, Feb 8, 3:04 PM · Restricted Project
antiagainst added a comment to D73826: [mlir][Linalg] Provide a Tablegen backend to specify named Linalg ops declaratively.

The main value I see with the OpDefinitionGen hook is that it allows one to automatically insert traits and methods into the generated C++ op classes. This is preferable when a dialect's ops have a large variation of potential traits and methods, and that information is already encoded with other fields in the op definitions.

Sat, Feb 8, 8:41 AM · Restricted Project
antiagainst requested changes to D74276: [mlir][DeclarativeParser] Add support for attributes with buildable types..

It seems the parsers/printers modified in this patch does not use the attribute-type-ellison feature introduced in this patch? Can we touch parsers/printers really uses this feature? Also should we document this? Otherwise it's subtle behavior users might find very confusing. (The modifications to parsers/printers in this patch can be split out.)

Sat, Feb 8, 7:05 AM · Restricted Project
antiagainst added a comment to D74270: [mlir][GPUToSPIRV] Modify the lowering of gpu.block_dim to be consistent with Vulkan SPEC.

Please also fix the test.

Sat, Feb 8, 6:07 AM · Restricted Project
antiagainst accepted D74270: [mlir][GPUToSPIRV] Modify the lowering of gpu.block_dim to be consistent with Vulkan SPEC.

LGTM.

Sat, Feb 8, 6:07 AM · Restricted Project
antiagainst accepted D73764: [mlir] Added RankedIntElementsAttr class.
Sat, Feb 8, 4:06 AM · Restricted Project
antiagainst requested changes to D73579: [mlir] Add elementAttr to TypedArrayAttrBase..
Sat, Feb 8, 4:06 AM · Restricted Project

Fri, Feb 7

antiagainst committed rG9c1c825b7249: [mlir][spirv] Adding sin op in the GLSL extension (authored by NatashaKnk).
[mlir][spirv] Adding sin op in the GLSL extension
Fri, Feb 7, 1:40 PM
antiagainst closed D74151: Adding sin op in the GLSL extension.
Fri, Feb 7, 1:39 PM · Restricted Project
antiagainst added a reviewer for D74226: [mlir][spirv] Introduce spv.function: denis13.
Fri, Feb 7, 8:45 AM · Restricted Project
antiagainst created D74226: [mlir][spirv] Introduce spv.function.
Fri, Feb 7, 8:45 AM · Restricted Project

Wed, Feb 5

antiagainst added a comment to D73934: [mlir] Add support for basic location translation to LLVM..

Cool stuff! :) This reminds me that we don't have proper debug information to and from SPIR-V binary format too. Need to add that some day too. ;-P

Wed, Feb 5, 8:12 AM · Restricted Project
antiagainst added inline comments to D73590: [mlir] Add a document detailing the design of the SymbolTable..
Wed, Feb 5, 8:03 AM · Restricted Project
antiagainst added a comment to D74012: [mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions.

I understand what the intent is here, but the input already has an attribute that belongs to the SPIR-V dialect before lowering.

Wed, Feb 5, 7:39 AM · Restricted Project
antiagainst added inline comments to D73944: [mlir][wip] Start Shape dialect.
Wed, Feb 5, 6:54 AM · Restricted Project
antiagainst added a reviewer for D73944: [mlir][wip] Start Shape dialect: antiagainst.
Wed, Feb 5, 6:54 AM · Restricted Project
antiagainst accepted D73764: [mlir] Added RankedIntElementsAttr class.

LGTM, just two nits regarding the test names.

Wed, Feb 5, 6:30 AM · Restricted Project

Tue, Feb 4

antiagainst accepted D73983: [mlir][ODS] Add documentation for the declarative assembly format..

Nice!

Tue, Feb 4, 6:49 PM · Restricted Project
antiagainst committed rG13b197c7d18b: [mlir][spirv] Add dialect-specific attribute for target environment (authored by antiagainst).
[mlir][spirv] Add dialect-specific attribute for target environment
Tue, Feb 4, 6:40 PM
antiagainst closed D73959: [mlir][spirv] Add dialect-specific attribute for target environment.
Tue, Feb 4, 6:40 PM · Restricted Project
antiagainst updated the diff for D74012: [mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions.

Remove \n

Tue, Feb 4, 6:12 PM · Restricted Project
antiagainst updated the diff for D74012: [mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions.

Update doc

Tue, Feb 4, 6:12 PM · Restricted Project
antiagainst created D74012: [mlir][spirv] Use spv.entry_point_abi in GPU to SPIR-V conversions.
Tue, Feb 4, 6:03 PM · Restricted Project
antiagainst added a comment to D73579: [mlir] Add elementAttr to TypedArrayAttrBase..

@antiagainst : what is the best way to test that?

Tue, Feb 4, 7:45 AM · Restricted Project
antiagainst added inline comments to D73819: [mlir] Add $_op hook to RewriterGen..
Tue, Feb 4, 7:27 AM · Restricted Project
antiagainst committed rGaad352f77c47: [mlir][spirv] Wrap debug-only method in #ifndef NDEBUG (authored by antiagainst).
[mlir][spirv] Wrap debug-only method in #ifndef NDEBUG
Tue, Feb 4, 6:05 AM
antiagainst added a reviewer for D73959: [mlir][spirv] Add dialect-specific attribute for target environment: benvanik.
Tue, Feb 4, 6:05 AM · Restricted Project
antiagainst committed rG399887c9e43b: [mlir][spirv] Add resource limits into target environment (authored by antiagainst).
[mlir][spirv] Add resource limits into target environment
Tue, Feb 4, 5:38 AM
antiagainst closed D73905: [mlir][spirv] Add resource limits into target environment.
Tue, Feb 4, 5:37 AM · Restricted Project
antiagainst created D73959: [mlir][spirv] Add dialect-specific attribute for target environment.
Tue, Feb 4, 5:37 AM · Restricted Project