Page MenuHomePhabricator

silvas (Sean Silva)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 20 2012, 11:51 AM (484 w, 1 d)

Recent Activity

Mon, Nov 22

silvas updated subscribers of D114384: Fix expand folder to avoid folding memref cast.

The idea here of preventing invalid ops makes sense to me, but @herhut probably is more knowledgeable about this code for a detailed review.

Mon, Nov 22, 12:50 PM · Restricted Project

Thu, Nov 11

silvas accepted D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo.

Nice :) This looks pretty straightforward, but given how core this is, wait for another approver or two.

Thu, Nov 11, 11:14 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mon, Nov 8

silvas added a comment to D111594: [MLIR] Attribute and type formats in ODS.

Yay!!

Mon, Nov 8, 11:12 AM · Restricted Project

Tue, Nov 2

silvas added a comment to D111620: [mlir:DialectConversion] Restructure how argument/target materializations get invoked.

This change broke 1->N type conversion (as noted in a FIXME in here). This is really unfortunate and there doesn't seem to be any workaround now that the materialization is deferred and the casts are inserted in a way that doesn't allow for patterns to match them. Is this something that can be rolled back or fixed soon? 1->N type conversion during dialect conversion did work before this change so this is a regression.

The FIXME is more of further elaborating on the existing state, it was not some new problem added by this patch. Can you file a bug report with what your exact situation is?

Tue, Nov 2, 4:49 PM · Restricted Project

Mon, Nov 1

silvas accepted D112968: [NFC] Add size inference to to_vector.

This looks good to me. @rriddle @dexonsmith @dblaikie for thoughts.

Mon, Nov 1, 4:27 PM · Restricted Project

Oct 28 2021

silvas added a comment to D112736: [mlir] MathApproximations: unroll virtual vectors into hardware vectors for ISA specific operation.

Basically add a pass something like X86vectorMathApproximationsPass?

Oct 28 2021, 2:55 PM · Restricted Project
silvas added a comment to D112736: [mlir] MathApproximations: unroll virtual vectors into hardware vectors for ISA specific operation.
  1. I would just like Math dialect transforms to not depend on target-specific dialects. So concretely just RsqrtOpApproximation pattern would move to x86vector/Transform/, since it is the target specific one.
  2. I agree, removing Polynomial would be good.
Oct 28 2021, 2:42 PM · Restricted Project
silvas added a comment to D112736: [mlir] MathApproximations: unroll virtual vectors into hardware vectors for ISA specific operation.

Can we move this RsqrtOp lowering to the x86vector dialect? It seems really out of place in the math dialect, which should remain target agnostic. (also, since this approximation is newton-raphson based instead of polynomial based, it seems out of place in PolynomialApproximation.cpp as well)

Oct 28 2021, 2:27 PM · Restricted Project
silvas added a comment to D112736: [mlir] MathApproximations: unroll virtual vectors into hardware vectors for ISA specific operation.

This doesn't seem to be the right pass to do target-specific lowerings. Can you add math.rsqrt and move the vector unrolling to a generic pass in the vector dialect? Then this pass wouldn't depend on target-details.

Oct 28 2021, 2:05 PM · Restricted Project

Oct 26 2021

silvas added a comment to D111620: [mlir:DialectConversion] Restructure how argument/target materializations get invoked.

Glad to see this happening!

Oct 26 2021, 10:28 PM · Restricted Project

Oct 25 2021

silvas committed rGf1b922188ead: [MLIR][Math] Add erf to math dialect (authored by sogartar).
[MLIR][Math] Add erf to math dialect
Oct 25 2021, 11:31 AM
silvas closed D112200: [MLIR][Math] Add erf to math dialect.
Oct 25 2021, 11:31 AM · Restricted Project

Oct 21 2021

silvas added a comment to D112120: [mlir:GreedyPatternRewriter] Add debug logging for pattern rewriter actions.

nice!

Oct 21 2021, 11:28 AM · Restricted Project
silvas accepted D112200: [MLIR][Math] Add erf to math dialect.

LGTM. Probably wait for another pair of eyes to LGTM it. @ezhulenev perhaps?

Oct 21 2021, 11:26 AM · Restricted Project

Oct 11 2021

silvas added a comment to D111540: [mlir] add user-level documentation for Python bindings.

Thanks!!!

Oct 11 2021, 9:52 AM · Restricted Project

Oct 6 2021

silvas added inline comments to D110768: [MLIR] Introduce the type inference pass..
Oct 6 2021, 4:50 PM · Restricted Project
silvas added inline comments to D110768: [MLIR] Introduce the type inference pass..
Oct 6 2021, 3:27 PM · Restricted Project
silvas added inline comments to D110768: [MLIR] Introduce the type inference pass..
Oct 6 2021, 12:13 PM · Restricted Project

Oct 5 2021

silvas accepted D110766: [MLIR] Rename Shape dialect's `join` to `meet`..
Oct 5 2021, 4:24 PM · Restricted Project
silvas added a comment to D111139: [mlir] Don't allow dynamic extent tensor types for ConstShapeOp..

I think it's fine for const_shape to always be static and require a tensor.cast to erase the static shape to dynamic.

Oct 5 2021, 10:04 AM · Restricted Project

Oct 4 2021

silvas accepted D111046: [mlir] rename the "packing" flag of linalg.pad_tensor to "nofold".

Thanks!

Oct 4 2021, 11:58 AM · Restricted Project

Sep 30 2021

silvas accepted D110766: [MLIR] Rename Shape dialect's `join` to `meet`..
Sep 30 2021, 1:25 PM · Restricted Project
silvas added a comment to D110766: [MLIR] Rename Shape dialect's `join` to `meet`..

+1 to Jacques suggestion to listing some prior art here. In the project ideally or in the industry as well would be good.

Sep 30 2021, 1:25 PM · Restricted Project
silvas added inline comments to D110768: [MLIR] Introduce the type inference pass..
Sep 30 2021, 1:02 PM · Restricted Project
silvas added inline comments to D110768: [MLIR] Introduce the type inference pass..
Sep 30 2021, 12:58 PM · Restricted Project
silvas added a comment to D110425: [mlir] Linalg: ensure tile-and-pad always creates padding as requested.

What would be a more correct phrasing for the semantics description in your opinion? We want the op to always result in a new value.

I don't really understand the argument about the operation having to be always folded away. This would effectively render impossible any clone or copy operation. Yet, there is a TensorCloneOp in IREE if I remember correctly and it isn't an always-folded noop. So there is clearly precedent and a use case for operations defining new values with same "content" as the existing values.

The additional attribute may indeed be interpreted as preventing some canonicalization wrt the semantics of the original op (or its version with the attribute unset). However, I argue that we just choose to define different canonical forms for the op depending on it having the attribute. Referring to the post our doc on canonicalization quotes in the opening paragraph - https://sunfishcode.github.io/blog/2018/10/22/Canonicalization.html - that describes how canonicalization and redundancy elimination get ambiguous: "<...> ultimately, in its purest form, canonicalization just focuses on removing unnecessary variation so that subsequent optimizations can be simpler", this change is exactly about removing, or rather shifting, variation to simplify subsequent optimization. There exist subsequent optimizations such as hoisting that are rendered significantly simpler by always having a fresh value that is always defined by pad_tensor.

Sep 30 2021, 11:56 AM · Restricted Project

Sep 29 2021

silvas updated subscribers of D110764: [MLIR] Add scalar tests for `Same*Shape` traits..

@herhut - thoughts on keeping scalars (i1) as disallowed from Same*Shape traits?

Sep 29 2021, 2:22 PM · Restricted Project
silvas added a comment to D110764: [MLIR] Add scalar tests for `Same*Shape` traits..

Personally I think we should deprecate this behavior. In my mind, an i1 doesn't have a shape at all. So it doesn't make sense to say it has the same shape as anything else. Is there a specific use case for this behavior (IIRC this might have fallen out of an implementation detail of an earlier implementation of the trait).

Sep 29 2021, 2:13 PM · Restricted Project
silvas added a comment to D110425: [mlir] Linalg: ensure tile-and-pad always creates padding as requested.

Specifically, it is impossible for these two functions to have different semantics (as long as linalg.pad_tensor is marked NoSideEffect) -- both return a value that is identical to their argument, and it is always legal to fold @with_pad to @without_pad per tensor value semantics:

Sep 29 2021, 9:38 AM · Restricted Project
silvas added a comment to D110425: [mlir] Linalg: ensure tile-and-pad always creates padding as requested.

Sorry I didn't see this earlier, but I don't think this direction makes sense. This op operates on tensors, which are value semantic, so terminology like "guaranteed to create a new tensor suitable for packing" simply don't make sense.

I fail to see what doesn't make sense? Packing is done on tensor semantics too: this can be also called "tensor tiling" by opposition to "op tiling".

Having an attribute that prevents folding from happening gives me bad memories of e.g. tf.Identity being marked as having side effects to prevent folding. I would rather not go down that route.

I do not see the relation between the 2.
Marking tf.Identity as having side effects is clearly a hack misusing side effects for other purposes: the op still has no side effects.
Here, the semantics of the op are : it is illegitimate to fold this op. There is no discussion about side effects or any other interface.

Sep 29 2021, 9:26 AM · Restricted Project

Sep 28 2021

silvas added a comment to D110425: [mlir] Linalg: ensure tile-and-pad always creates padding as requested.

Sorry I didn't see this earlier, but I don't think this direction makes sense. This op operates on tensors, which are value semantic, so terminology like "guaranteed to create a new tensor suitable for packing" simply don't make sense.

Sep 28 2021, 4:22 PM · Restricted Project
silvas committed rG204d301bb192: [mlir][Python] Fix lifetime of ExecutionEngine runtime functions. (authored by silvas).
[mlir][Python] Fix lifetime of ExecutionEngine runtime functions.
Sep 28 2021, 3:32 PM
silvas closed D110661: [mlir][Python] Fix lifetime of ExecutionEngine runtime functions..
Sep 28 2021, 3:32 PM · Restricted Project
silvas updated the diff for D110661: [mlir][Python] Fix lifetime of ExecutionEngine runtime functions..

address

Sep 28 2021, 3:22 PM · Restricted Project
silvas added reviewers for D110661: [mlir][Python] Fix lifetime of ExecutionEngine runtime functions.: stellaraccident, mehdi_amini.
Sep 28 2021, 3:02 PM · Restricted Project
silvas requested review of D110661: [mlir][Python] Fix lifetime of ExecutionEngine runtime functions..
Sep 28 2021, 3:01 PM · Restricted Project

Sep 22 2021

silvas accepted D110200: [MLIR] Split arith dialect from the std dialect.

did a pass. looks good to me. probably want to wait for a handful of LGTM's given the far-reaching consequences of this change. Ideally both River and Chris could weigh in.

Sep 22 2021, 9:49 AM · Restricted Project

Sep 17 2021

silvas accepted D109965: [MLIR][Linalg] Make detensoring cost-model more flexible..
Sep 17 2021, 9:58 AM · Restricted Project

Sep 15 2021

silvas added a comment to D109819: [mlir][Linalg] Replace DenseSet by UnionFind in ComprehensiveBufferize - NFC.

Nice!

Sep 15 2021, 8:59 AM · Restricted Project
silvas accepted D109830: [mlir][Linalg] Revisit insertion points in comprehensive bufferization..
Sep 15 2021, 8:59 AM · Restricted Project

Sep 14 2021

silvas committed rG8dca953dd39c: [mlir] Apply py::module_local() to a few more classes. (authored by silvas).
[mlir] Apply py::module_local() to a few more classes.
Sep 14 2021, 3:27 PM
silvas closed D109776: [mlir] Apply py::module_local() to a few more classes..
Sep 14 2021, 3:26 PM · Restricted Project
silvas added a reviewer for D109776: [mlir] Apply py::module_local() to a few more classes.: stellaraccident.
Sep 14 2021, 11:34 AM · Restricted Project
silvas requested review of D109776: [mlir] Apply py::module_local() to a few more classes..
Sep 14 2021, 11:34 AM · Restricted Project

Sep 9 2021

silvas accepted D109480: [MLIR] Use memref.copy ops in BufferResultsToOutParams pass..
Sep 9 2021, 9:44 AM · Restricted Project

Sep 3 2021

silvas added inline comments to D107236: Add a new interface allowing to set a default dialect to be used for printing/parsing regions.
Sep 3 2021, 2:22 PM · Restricted Project
silvas added inline comments to D107236: Add a new interface allowing to set a default dialect to be used for printing/parsing regions.
Sep 3 2021, 1:44 PM · Restricted Project
silvas added inline comments to D107236: Add a new interface allowing to set a default dialect to be used for printing/parsing regions.
Sep 3 2021, 1:33 PM · Restricted Project

Sep 2 2021

silvas accepted D109152: [mlir] speed up construction of LLVM IR constants when possible.

Awesome! Thanks!!!

Sep 2 2021, 11:23 AM · Restricted Project

Aug 11 2021

silvas added a comment to D107891: [mlir] Add std.bitcast -> llvm.bitcast conversion.

Thanks Alex!

Aug 11 2021, 10:29 AM · Restricted Project
silvas added a comment to D107858: [mlir] Change the pattern for TiledLoopOp bufferization..

Thanks Alex!

Aug 11 2021, 10:28 AM · Restricted Project

Aug 10 2021

silvas added a comment to D107858: [mlir] Change the pattern for TiledLoopOp bufferization..

Can you please revert this commit and the previous one I objected to? I think they are fundamentally going in the wrong direction.

Aug 10 2021, 3:48 PM · Restricted Project
silvas added a comment to D107858: [mlir] Change the pattern for TiledLoopOp bufferization..

Can you please add the integration test cases I suggested in the other patch? I'm still pretty sure this will miscompile. Fundamentally you are writing this as an "in-place" bufferization and that cannot be done with the current framework because it involves non-local reasoning.

Aug 10 2021, 3:39 PM · Restricted Project

Aug 9 2021

silvas added a comment to D107762: [mlir] Add a bufferization pattern for linalg.tiled_loop..

It feels like this bufferization pattern would miscompile if we were to write integration tests analogous to these:

Aug 9 2021, 4:25 PM · Restricted Project

Aug 5 2021

silvas accepted D105376: [MLIR][std] Introduce bitcast operation.
Aug 5 2021, 2:35 PM · Restricted Project

Aug 3 2021

silvas accepted D107358: [MLIR][Linalg] Extend detensoring control flow model..

Beautiful :)

Aug 3 2021, 8:55 AM · Restricted Project

Aug 2 2021

silvas added inline comments to D105376: [MLIR][std] Introduce bitcast operation.
Aug 2 2021, 2:49 PM · Restricted Project
silvas added a comment to D106658: [mlir][linalg] Add pooling_nchw_max, conv_2d_nchw as yaml ops..

great work yi!

Aug 2 2021, 7:47 AM · Restricted Project

Jul 10 2021

silvas accepted D105748: [mlir] Fix broadcasting check with 1 values.
Jul 10 2021, 12:51 PM · Restricted Project

Jul 9 2021

silvas added inline comments to D105558: [mlir][MemRef] Fix DimOp folding of OffsetSizeAndStrideInterface..
Jul 9 2021, 10:47 AM · Restricted Project
silvas added a comment to D105702: [mlir] factor math-to-llvm out of standard-to-llvm.

Thanks!

Jul 9 2021, 10:40 AM · Restricted Project
silvas accepted D105702: [mlir] factor math-to-llvm out of standard-to-llvm.
Jul 9 2021, 10:40 AM · Restricted Project

Jul 8 2021

silvas committed rG7c35aae35b2c: Mark TensorDialect legal and PadTensor op illegal (authored by cathyzhyi).
Mark TensorDialect legal and PadTensor op illegal
Jul 8 2021, 3:03 PM
silvas closed D105642: Mark TensorDialect legal and PadTensor op illegal.
Jul 8 2021, 3:02 PM · Restricted Project
silvas accepted D105642: Mark TensorDialect legal and PadTensor op illegal.
Jul 8 2021, 1:41 PM · Restricted Project
silvas added a comment to D105642: Mark TensorDialect legal and PadTensor op illegal.

I don't think we need an integration test here. A regular IR test would be enough to exercise it? (just snapshot the IR from the itnegration test before linalg-bufferize)

Jul 8 2021, 12:34 PM · Restricted Project
silvas added a comment to D105642: Mark TensorDialect legal and PadTensor op illegal.

Do you know why previous tests didn't need this? Should we add a new test?

Jul 8 2021, 10:41 AM · Restricted Project
silvas added inline comments to D105558: [mlir][MemRef] Fix DimOp folding of OffsetSizeAndStrideInterface..
Jul 8 2021, 10:29 AM · Restricted Project
silvas accepted D105625: [mlir] factor memref-to-llvm lowering out of std-to-llvm.
Jul 8 2021, 10:10 AM · Restricted Project

Jul 7 2021

silvas committed rGda289a174fc6: [mlir] Allow conversion to LLVM for ElementsAttr's with size 0 (authored by silvas).
[mlir] Allow conversion to LLVM for ElementsAttr's with size 0
Jul 7 2021, 11:16 AM
silvas closed D105379: [mlir] Allow conversion to LLVM for ElementsAttr's with size 0.
Jul 7 2021, 11:16 AM · Restricted Project
silvas added a comment to D105376: [MLIR][std] Introduce bitcast operation.

Please add lowering to LLVM for this too (doesn't have to be this patch precisely, but a fast follow-on please).

Jul 7 2021, 11:09 AM · Restricted Project
silvas added a comment to D105383: [mlir][tosa] Added more shape inference for tosa ops.

Will let @sjarus review this as the detailed shape transfer function review is best done by someone deeply aware of the spec.

Jul 7 2021, 10:41 AM · Restricted Project

Jul 2 2021

silvas retitled D105379: [mlir] Allow conversion to LLVM for ElementsAttr's with size 0 from [mlir] Allow convertion to LLVM for ElementsAttr's with size 0 to [mlir] Allow conversion to LLVM for ElementsAttr's with size 0.
Jul 2 2021, 3:57 PM · Restricted Project
silvas updated the diff for D105379: [mlir] Allow conversion to LLVM for ElementsAttr's with size 0.

reword

Jul 2 2021, 3:57 PM · Restricted Project
silvas requested review of D105379: [mlir] Allow conversion to LLVM for ElementsAttr's with size 0.
Jul 2 2021, 3:55 PM · Restricted Project
silvas accepted D105293: Refactor GenericPadTensorOpVectorizationPattern.

LGTM. Wait for @springerm / @nicolasvasilache approval too.

Jul 2 2021, 1:41 PM · Restricted Project
silvas added a comment to D105293: Refactor GenericPadTensorOpVectorizationPattern.

Oh, sorry, I thought you meant which woudl use it. It would be nice if it was in a generic helper file. You can probably put it next to PadTensorOpTransformationPattern for consistency.

Jul 2 2021, 12:36 PM · Restricted Project
silvas added a comment to D105293: Refactor GenericPadTensorOpVectorizationPattern.

@silvas @nicolasvasilache @springerm Any suggestion on which pass to put this pattern in? There is a LinalgGeneralizationPass which currently only applies to patterns lowering to Linalg.generic.

Jul 2 2021, 11:59 AM · Restricted Project

Jul 1 2021

silvas accepted D105312: [mlir][tosa] Add tosa shape inference with InferReturnTypeComponent.
Jul 1 2021, 4:00 PM · Restricted Project
silvas requested changes to D105312: [mlir][tosa] Add tosa shape inference with InferReturnTypeComponent.
Jul 1 2021, 3:56 PM · Restricted Project
silvas requested changes to D105312: [mlir][tosa] Add tosa shape inference with InferReturnTypeComponent.
Jul 1 2021, 3:46 PM · Restricted Project
silvas added a comment to D105293: Refactor GenericPadTensorOpVectorizationPattern.

Note that I asked for a similar rewrite in https://reviews.llvm.org/D102804 but it was eventually not done.
I am wondering whether the pattern you add here should be more generally be the rewrite on tensors I mentioned and then just let existing bufferization kick in?

Jul 1 2021, 1:50 PM · Restricted Project
silvas requested changes to D105312: [mlir][tosa] Add tosa shape inference with InferReturnTypeComponent.
Jul 1 2021, 1:02 PM · Restricted Project
silvas accepted D105256: [mlir] Move BufferizeDimOp to Tensor/Transforms/Bufferize.cpp.
Jul 1 2021, 10:59 AM · Restricted Project
silvas resigned from D102216: [Clang][docs]JSONLinkDatabase.
Jul 1 2021, 10:57 AM · Restricted Project
silvas requested changes to D105293: Refactor GenericPadTensorOpVectorizationPattern.

Can you add an integration test analogous to mlir/test/Integration/Dialect/Linalg/CPU/test-subtensor-insert.mlir ?

Jul 1 2021, 10:14 AM · Restricted Project

Jun 30 2021

silvas accepted D105228: [mlir][Linalg] Drop comprehensive-func-bufferize (12/n).

Nice cleanup! Thanks!

Jun 30 2021, 2:17 PM · Restricted Project
silvas added a comment to D105165: [mlir][tensor] Add tensor.dim operation.

(before submitting, please wait for LGTM from at least one other person since this change covers a lot of stuff)

Jun 30 2021, 9:36 AM · Restricted Project
silvas added a comment to D105156: [mlir][Linalg] Add comprehensive bufferization support for ConstantOp (13/n).

I would personally like to see this as a nice clean module pass, and then make targeted changes informed by concrete use cases (actual need for more parallelism, need to be applied locally within a particular kind of enclosing op, etc.). Really for more advanced use cases this is going to be accessed via a direct C++ API, so the pass structure we choose here should be clean and simple and have nice invariants that make it easy to reason about extracting a targeted C++ API for new use cases. For example, the C++ API could take a std::function<Value(ConstantOp op)> for creating memref values from constants, which for the module pass would use a GlobalCreator, but for other users would be implemented otherwise. I don't want to promote uses of ComprehensiveBufferize with this weird non-support for constants.

Jun 30 2021, 9:33 AM · Restricted Project
silvas accepted D105165: [mlir][tensor] Add tensor.dim operation.
Jun 30 2021, 9:26 AM · Restricted Project

Jun 29 2021

silvas accepted D105156: [mlir][Linalg] Add comprehensive bufferization support for ConstantOp (13/n).

Can you just delete the function pass and have the module pass be the only registered pass?

Jun 29 2021, 3:26 PM · Restricted Project

Jun 18 2021

silvas added a comment to D104499: [mlir][NFC] Move SubTensorOp and SubTensorInsertOp to TensorDialect.

Thanks you Matthias for doing this! I apologize for leaving this in the sorry state that it was left in.

Jun 18 2021, 7:52 AM · Restricted Project
silvas committed rG7f7be19e6a5e: [mlir] Add notes about using external interface application. (authored by silvas).
[mlir] Add notes about using external interface application.
Jun 18 2021, 7:43 AM
silvas closed D104489: [mlir] Add notes about using external interface application..
Jun 18 2021, 7:43 AM · Restricted Project
silvas updated subscribers of D102590: [YAMLParser] Add multi-line literal folding support.

@HassanElDesouky sorry, I'm not that familiar with this code.

Jun 18 2021, 7:21 AM · Restricted Project

Jun 17 2021

silvas added a comment to rG3ed3e438a75d: [mlir] Move `memref.dim` canonicalization using `InferShapedTypeOpInterface` to….

This makes me so sad that the memref.dim infection is forcing this to go in memref/transforms :'(

Jun 17 2021, 2:51 PM
silvas requested review of D104489: [mlir] Add notes about using external interface application..
Jun 17 2021, 2:34 PM · Restricted Project

Jun 15 2021

Herald added a reviewer for D98041: [MLIR] Create memref dialect and move dialect-specific ops from std.: clementval.

Hi @dfki-jugr, can we please split memref.dim into tensor.dim (and std.rank into tensor.rank if you have time). I've now independently heard complaints about this from 5+ people across many different teams I interact with.

Jun 15 2021, 6:34 PM · Restricted Project

Jun 14 2021

silvas committed rG853a61486475: [mlir:OpFormatGen] Add Support for `$_ctxt` in the transformer. (authored by silvas).
[mlir:OpFormatGen] Add Support for `$_ctxt` in the transformer.
Jun 14 2021, 6:03 PM