Page MenuHomePhabricator

mehdi_amini (Mehdi AMINI)
UserAdministrator

Projects

User does not belong to any projects.

User Details

User Since
Apr 30 2013, 5:34 PM (394 w, 5 d)
Roles
Administrator

Recent Activity

Today

mehdi_amini accepted D91652: [mlir] Make attributes mutable in Python bindings.
Mon, Nov 23, 10:40 AM · Restricted Project
mehdi_amini accepted D91961: [mlir] move lib/Bindings/Python/Attributes.td to include/mlir/Bindings/Python.
Mon, Nov 23, 10:39 AM · Restricted Project
mehdi_amini requested changes to D91967: [mlir][std] Mark tensor_to_memref as no side-effect.

NoSideEffect does not seem to fit to me: there is an effect for allocation. It should also write to its output.

Mon, Nov 23, 9:21 AM · Restricted Project
mehdi_amini added a comment to D91959: [mlir] use STATUS instead of CHECK_* in MLIRDetectPythonEnv.cmake.

LGTM

Mon, Nov 23, 9:19 AM · Restricted Project

Sat, Nov 21

mehdi_amini added inline comments to D91896: [mlir] Add microbenchmark for linalg+async-parallel-for.
Sat, Nov 21, 8:30 PM · Restricted Project
mehdi_amini added inline comments to D91905: Allow Location to be created with a string ref.
Sat, Nov 21, 8:21 PM · Restricted Project

Fri, Nov 20

mehdi_amini accepted D91903: [mlir][Python] Support finding pybind11 from the python environment..
Fri, Nov 20, 9:46 PM · Restricted Project
mehdi_amini added a comment to D91837: ADT: Fix reference invalidation in SmallVector APIs that pass in a value.

LGTM!

Fri, Nov 20, 5:42 PM · Restricted Project
mehdi_amini added a comment to D91896: [mlir] Add microbenchmark for linalg+async-parallel-for.

Are the changes in mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp bug fixes? Can you add IR tests for this (and commit separately so there is a proper commit description)?

Fri, Nov 20, 5:38 PM · Restricted Project
mehdi_amini added a comment to rG0caa82e2ac53: Revert "[mlir][Linalg] Fuse sequence of Linalg operation (on buffers)".
Fri, Nov 20, 2:03 PM
mehdi_amini accepted D91738: Add userData to the diagnostic handler C API.

LGTM

Fri, Nov 20, 10:47 AM · Restricted Project
mehdi_amini added a comment to D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation.

So to clarify policy, we reverted patches when (1) a test based on an experimental intrinsic was crashing and (2) that crash is easily reproducible independent of this patch as shown here:
7ae346434

I understand that we revert first and ask questions later, but should that be the rule for experimental code?

Fri, Nov 20, 10:44 AM · Restricted Project

Thu, Nov 19

mehdi_amini committed rGfbfbfa5c713f: [mlir] Support big-endian systems in DenseElementsAttr (multiple word) (authored by imaihal).
[mlir] Support big-endian systems in DenseElementsAttr (multiple word)
Thu, Nov 19, 9:33 PM
mehdi_amini closed D80272: [mlir] Support big-endian systems in DenseElementsAttr (multiple word).
Thu, Nov 19, 9:33 PM · Restricted Project, Restricted Project
mehdi_amini added inline comments to D91652: [mlir] Make attributes mutable in Python bindings.
Thu, Nov 19, 9:03 PM · Restricted Project
mehdi_amini added a comment to D91738: Add userData to the diagnostic handler C API.

@stellaraccident / @ftynse can you double check on the deleteUserData part and the shared_ptr?

Thu, Nov 19, 5:37 PM · Restricted Project
mehdi_amini accepted D91830: [mlir] Expose parseDimAndSymbolList from affineops.h.
Thu, Nov 19, 5:36 PM · Restricted Project
mehdi_amini added a comment to D91738: Add userData to the diagnostic handler C API.

Can you expand a bit on the use-case for deleteUserData? When is the user not in control of the lifetime? This seems all kind of tied to the context in the end?

Thu, Nov 19, 3:27 PM · Restricted Project

Wed, Nov 18

mehdi_amini added a comment to D87981: [X86] AMX programming model..

(Drive by comments)

Wed, Nov 18, 7:47 PM · Restricted Project, Restricted Project
mehdi_amini added inline comments to D91672: Add a shape function library op.
Wed, Nov 18, 7:38 PM · Restricted Project
mehdi_amini added inline comments to D91672: Add a shape function library op.
Wed, Nov 18, 7:37 PM · Restricted Project
mehdi_amini added a comment to D91467: ADT: Take small enough, trivially copyable T by value in SmallVector.

Oh I didn't read about https://reviews.llvm.org/D87326 before, isn't the work there making obsolete all of the assertions you (and me before) just added by making this behavior safe?

Wed, Nov 18, 5:50 PM · Restricted Project
mehdi_amini accepted D91750: [mlir] Add a missing dependency to LinalgToLLVM.
Wed, Nov 18, 5:28 PM · Restricted Project
mehdi_amini accepted D91738: Add userData to the diagnostic handler C API.
Wed, Nov 18, 5:24 PM · Restricted Project
mehdi_amini accepted D91740: Make array pointers in the CAPI const.
Wed, Nov 18, 5:19 PM · Restricted Project
mehdi_amini added a comment to D91467: ADT: Take small enough, trivially copyable T by value in SmallVector.

Can you make more clear in the description the motivation for this patch? Is this mostly about iterator invalidation? Is this expected to make code safer?
I'm not sure about making the mental model a bit harder by having different iterator invalidation behavior implicitly based on T, WDYT?

Wed, Nov 18, 5:04 PM · Restricted Project
mehdi_amini accepted D91744: ADT: Add assertions to SmallVector::insert, etc., for reference invalidation.
Wed, Nov 18, 4:30 PM · Restricted Project
mehdi_amini added inline comments to D91744: ADT: Add assertions to SmallVector::insert, etc., for reference invalidation.
Wed, Nov 18, 2:54 PM · Restricted Project
mehdi_amini added inline comments to D91672: Add a shape function library op.
Wed, Nov 18, 10:35 AM · Restricted Project
mehdi_amini added inline comments to D91672: Add a shape function library op.
Wed, Nov 18, 10:29 AM · Restricted Project

Mon, Nov 16

mehdi_amini added a reviewer for D91584: Add CalibratedQuantizedType to quant dialect: stellaraccident.
Mon, Nov 16, 8:15 PM · Restricted Project
mehdi_amini committed rG74207e78cf26: Fix python bindings tests after change in visibility requirement for symbol… (authored by mehdi_amini).
Fix python bindings tests after change in visibility requirement for symbol…
Mon, Nov 16, 8:10 PM
mehdi_amini committed rG65d4b5cb18e3: Add const qualifier to Type's utility functions (authored by Tei Jeong <taeheej@google.com>).
Add const qualifier to Type's utility functions
Mon, Nov 16, 8:00 PM
mehdi_amini closed D91491: Add const qualifier to Type's utility functions.
Mon, Nov 16, 7:59 PM · Restricted Project
mehdi_amini added a comment to D91523: [mlir] Require std.alloc() ops to have canonical layout during LLVM lowering..

[mlir] Require std.alloc() ops to have canonical layout during LLVM lowering.

Mon, Nov 16, 7:26 PM · Restricted Project
mehdi_amini accepted D91572: [mlir][NFC] Remove references to Module.h and Function.h.
Mon, Nov 16, 6:41 PM · Restricted Project
mehdi_amini accepted D91571: [mlir][IR] Use tablegen for the BuiltinDialect and operations.

Sweet!

Mon, Nov 16, 4:05 PM · Restricted Project
mehdi_amini added a comment to D91553: Optional argument for pattern rewriter max iteration count..

I tweaked slightly the doc and the commit message:
We like to tag commit messages with "NFC" when they don't modify an existing behavior and don't include tests (this change is a bit borderline, it *could* be plumbed and tested with a pass)

Mon, Nov 16, 2:59 PM · Restricted Project
mehdi_amini committed rGfbceee2d63bd: Add an optional argument for pattern rewriter max iteration count (NFC) (authored by Augusteijn).
Add an optional argument for pattern rewriter max iteration count (NFC)
Mon, Nov 16, 2:57 PM
mehdi_amini closed D91553: Optional argument for pattern rewriter max iteration count..
Mon, Nov 16, 2:57 PM · Restricted Project
mehdi_amini accepted D91553: Optional argument for pattern rewriter max iteration count..
Mon, Nov 16, 2:47 PM · Restricted Project

Sat, Nov 14

mehdi_amini added a comment to D91480: Avoid appending Pass where already present..

Seems like a "hack" to me, why not fix the source?

Sat, Nov 14, 10:39 AM · Restricted Project
mehdi_amini updated subscribers of D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".
Sat, Nov 14, 9:02 AM · Restricted Project
mehdi_amini committed rGd34add0ba1c2: Fix build (`ninja check` without running `ninja` first) (authored by mehdi_amini).
Fix build (`ninja check` without running `ninja` first)
Sat, Nov 14, 9:02 AM

Fri, Nov 13

mehdi_amini added inline comments to D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".
Fri, Nov 13, 8:16 PM · Restricted Project
mehdi_amini added a reverting change for rGe7ed27653292: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a…: rGac06b1af402e: Revert "Switch libLLVMFrontendOpenACC to be a regular CMake library and not a….
Fri, Nov 13, 8:15 PM
mehdi_amini committed rGac06b1af402e: Revert "Switch libLLVMFrontendOpenACC to be a regular CMake library and not a… (authored by mehdi_amini).
Revert "Switch libLLVMFrontendOpenACC to be a regular CMake library and not a…
Fri, Nov 13, 8:14 PM
mehdi_amini added a reverting change for D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component": rGac06b1af402e: Revert "Switch libLLVMFrontendOpenACC to be a regular CMake library and not a….
Fri, Nov 13, 8:14 PM · Restricted Project
mehdi_amini added a comment to D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".

We should dig in the OpenMP side as well.

What do you mean by that?

Fri, Nov 13, 7:50 PM · Restricted Project
mehdi_amini added a comment to D91396: [mlir] Remove SameOperandsAndResultShape when redundant with ElementwiseMappable.

Anyway, for now, the need for ensuring folds result in the same Type is a reality and that is why ElementwiseMappable is defined that way.

Fri, Nov 13, 7:18 PM · Restricted Project
mehdi_amini added inline comments to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..
Fri, Nov 13, 7:12 PM · Restricted Project
mehdi_amini added inline comments to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..
Fri, Nov 13, 7:11 PM · Restricted Project
mehdi_amini committed rGe7ed27653292: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a… (authored by mehdi_amini).
Switch libLLVMFrontendOpenACC to be a regular CMake library and not a…
Fri, Nov 13, 6:19 PM
mehdi_amini closed D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".
Fri, Nov 13, 6:19 PM · Restricted Project
mehdi_amini added a comment to D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".

Yeah it makes sense to have tests directly in llvm. Sorry for that I blindly followed the OpenMP stuffs and I should have think twice about it.

Fri, Nov 13, 6:18 PM · Restricted Project
mehdi_amini added a comment to D91396: [mlir] Remove SameOperandsAndResultShape when redundant with ElementwiseMappable.

In general I agree that ops that accept tensor<?xf32> should probably also accept tensor<2xf32>. (though probably not calls/functions, since those check Type by pointer equality, and I think we want to keep that?)

Fri, Nov 13, 6:16 PM · Restricted Project
mehdi_amini accepted D91430: [mlir] NFC: tiny fix in comment..

(Feel free to just push directly this kind of fixes)

Fri, Nov 13, 6:10 PM · Restricted Project
mehdi_amini added a comment to D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".

I think it is very unusual to have a library in LLVM that isn't used in LLVM and so does not come with any testing, so yes it'd be better to do so.
I'm not actually sure if llvm-config just relies implicitly on this?

Fri, Nov 13, 6:01 PM · Restricted Project
mehdi_amini accepted D91465: [mlir] refactor common idiom into AffineMap method.
Fri, Nov 13, 5:57 PM · Restricted Project
mehdi_amini added a comment to D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".

You are right, at the moment it is not connected directly to LLVM itself. It was done like this a little bit blindly follow what was done for for the libLLVMFrontendOpenMP.

Fri, Nov 13, 5:42 PM · Restricted Project
mehdi_amini added a comment to D91467: ADT: Take small enough, trivially copyable T by value in SmallVector.

Won't we end up with a different behavior (with respect to iterator invalidation) based on the platform?

Fri, Nov 13, 5:40 PM · Restricted Project
mehdi_amini added a comment to D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".

It seems like it is following what was done for OpenMP, however OpenMP is exposing LLVM IR Builder which connects it directly to LLVM. The directive part of the OpenMP library isn't tested either (uhhhh) in LLVM, it just does not expose the issue because it is bundles in the same library as the IR builders which are tested.

Fri, Nov 13, 5:34 PM · Restricted Project
mehdi_amini added a comment to D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".

@clementval : it isn't clear why this is in llvm/lib/Frontend/? Should this be a top-level frontend/lib kind of structure? There isn't any user of this inside LLVM right now other than flang and it isn't clear to me how it connects to LLVM itself.

Fri, Nov 13, 5:31 PM · Restricted Project
mehdi_amini added a comment to D91468: [llvm] Add OpenACC as a dependency to llvm-config.

I send out https://reviews.llvm.org/D91470 as another take on this.

Fri, Nov 13, 5:29 PM · Restricted Project
mehdi_amini requested review of D91470: Switch libLLVMFrontendOpenACC to be a regular CMake library and not a "component".
Fri, Nov 13, 5:29 PM · Restricted Project
mehdi_amini added a comment to D91468: [llvm] Add OpenACC as a dependency to llvm-config.

I know this fixes a bug, but I don't think this is quite correct.

Fri, Nov 13, 5:20 PM · Restricted Project
mehdi_amini added inline comments to D84293: Add an assertion in SmallVector::push_back().
Fri, Nov 13, 4:57 PM · Restricted Project, Restricted Project
mehdi_amini committed rG2c196bbc6bd8: Add an assertion in SmallVector::push_back() (authored by mehdi_amini).
Add an assertion in SmallVector::push_back()
Fri, Nov 13, 4:56 PM
mehdi_amini committed rG42e88bd6b185: Replace sequences of v.push_back(v[i]); v.erase(&v[i]); with std::rotate (NFC) (authored by mehdi_amini).
Replace sequences of v.push_back(v[i]); v.erase(&v[i]); with std::rotate (NFC)
Fri, Nov 13, 4:56 PM
mehdi_amini closed D84293: Add an assertion in SmallVector::push_back().
Fri, Nov 13, 4:56 PM · Restricted Project, Restricted Project
mehdi_amini updated the diff for D84293: Add an assertion in SmallVector::push_back().

Fix one clang instance failing this assertion

Fri, Nov 13, 4:01 PM · Restricted Project, Restricted Project
mehdi_amini added inline comments to D84293: Add an assertion in SmallVector::push_back().
Fri, Nov 13, 3:30 PM · Restricted Project, Restricted Project
mehdi_amini updated the diff for D84293: Add an assertion in SmallVector::push_back().

Update with Sean's suggestion

Fri, Nov 13, 3:10 PM · Restricted Project, Restricted Project
mehdi_amini added a comment to D90848: llvmbuildectomy - part 2.

It fails for me as well: configuring just LLVM alone and running ninja check-llvm instead of building first everything with ninja alone.

Fri, Nov 13, 3:00 PM · Restricted Project
mehdi_amini added inline comments to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..
Fri, Nov 13, 2:40 PM · Restricted Project
mehdi_amini added inline comments to D91456: [MLIR] Extend Symbol verification to reject public symbol declarations..
Fri, Nov 13, 1:57 PM · Restricted Project
mehdi_amini accepted D91456: [MLIR] Extend Symbol verification to reject public symbol declarations..
Fri, Nov 13, 1:56 PM · Restricted Project
mehdi_amini added inline comments to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..
Fri, Nov 13, 12:40 PM · Restricted Project
mehdi_amini added a comment to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..

+1 on both points!

Fri, Nov 13, 9:16 AM · Restricted Project

Thu, Nov 12

mehdi_amini committed rGa9386bb0f9d7: Fix MLIR lit test configuration after cmake Python detection change (authored by mehdi_amini).
Fix MLIR lit test configuration after cmake Python detection change
Thu, Nov 12, 8:45 PM
mehdi_amini added a comment to D91396: [mlir] Remove SameOperandsAndResultShape when redundant with ElementwiseMappable.

From the rationale:

/// - 3. guarantees that folding can be done across scalars/vectors/tensors
///   with the same pattern, as otherwise lots of special handling of type
///   mismatches would be needed.

I don't quite get what is the issue with this ?->2 ? Can you elaborate a bit? Do you have an example of a pattern maybe?

Consider xor(xor(a, b), b) -> a

%0 = xor %lhs, %rhs : (tensor<?xindex>, tensor<2xindex>) -> tensor<2xindex>
%1 = xor %0, %rhs : (tensor<2xindex>, tensor<2xindex>) -> tensor<2xindex>
"only_accepts_2xindex"(%1) : (tensor<2xindex>) -> ()

only_accepts_2xindex might be defined so that it only accepts tensor<2xindex>. If you apply the fold, you will replace %1 with %lhs, which will create invalid IR.

Thu, Nov 12, 7:25 PM · Restricted Project
mehdi_amini accepted D91227: [mlir][Asm] Add support for resolving operation locations after parsing has finished.
Thu, Nov 12, 6:37 PM · Restricted Project
mehdi_amini added a comment to D91396: [mlir] Remove SameOperandsAndResultShape when redundant with ElementwiseMappable.

From the rationale:

/// - 3. guarantees that folding can be done across scalars/vectors/tensors
///   with the same pattern, as otherwise lots of special handling of type
///   mismatches would be needed.
Thu, Nov 12, 6:20 PM · Restricted Project
mehdi_amini added a comment to D91396: [mlir] Remove SameOperandsAndResultShape when redundant with ElementwiseMappable.

SameOperandsAndResultShape allows `"foo"(%0) : tensor<2xf32> -> tensor<?xf32> but ElementwiseMappable does not.

Thu, Nov 12, 5:45 PM · Restricted Project
mehdi_amini accepted D91365: [MLIR] Allow setting "CodeView" flag in LLVMIR translation on MSVC..

LGTM

Thu, Nov 12, 4:47 PM · Restricted Project
mehdi_amini accepted D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..
Thu, Nov 12, 3:34 PM · Restricted Project
mehdi_amini added a comment to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..

having llvm::Vector be bigger than std::vector seems dangerous to me, simply because it would be surprising.

Thu, Nov 12, 1:59 PM · Restricted Project
mehdi_amini added a comment to D91365: [MLIR] Allow setting "CodeView" flag in LLVMIR translation on MSVC..

You may need a refactor like Alex did here: https://reviews.llvm.org/D85650 ; what about a factory method returning ErrorOr<Triple> ?

Thu, Nov 12, 11:25 AM · Restricted Project
mehdi_amini added a comment to D91365: [MLIR] Allow setting "CodeView" flag in LLVMIR translation on MSVC..

Can you add a test for this?

Thu, Nov 12, 11:16 AM · Restricted Project
mehdi_amini accepted D91304: [mlir][Interfaces] Add implicit casts from concrete operation types to the interfaces they implement..
Thu, Nov 12, 10:59 AM · Restricted Project
mehdi_amini accepted D90848: llvmbuildectomy - part 2.

LGTM

Thu, Nov 12, 10:02 AM · Restricted Project

Wed, Nov 11

mehdi_amini added inline comments to D91021: [mlir] Get array from the dense elements attribute with buffer protocol..
Wed, Nov 11, 8:44 PM · Restricted Project
mehdi_amini added a comment to D91316: [mlir][sparse] export sparse tensor runtime support through header file.

Do you have tests for all this?

Wed, Nov 11, 8:28 PM · Restricted Project
mehdi_amini added a comment to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..

I'd prefer to avoid SmallVec: it is too close to SmallVector (it is even a prefix, which will conflict with autocompletion in IDEs).
llvm::SVec or llvm::SVector on the other hand seems different enough to me to avoid confusion, and the using definition you propose seems like a good tradeoff to me!

Wed, Nov 11, 8:22 PM · Restricted Project
mehdi_amini added a reviewer for D90675: [MLIR] Added documentation and manual to use bufferization features.: silvas.
Wed, Nov 11, 6:22 PM · Restricted Project
mehdi_amini added a reviewer for D90848: llvmbuildectomy - part 2: beanz.
Wed, Nov 11, 6:15 PM · Restricted Project
mehdi_amini added a comment to D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers..

DefaultSmallVector is quite a "mouthful" though, I'd have thought we'd look for a "default container" that is easy to type if we have it everywhere.

Wed, Nov 11, 6:06 PM · Restricted Project
mehdi_amini added a comment to D82860: Port ObjCMTAction to new option parsing system.

Reverted in f917356f9ce0 ; I suspect a static constexpr in a class missing a definition out of class (required pre-c++17).

Wed, Nov 11, 12:02 PM · Restricted Project, Restricted Project
mehdi_amini added a reverting change for rG09248a5d25bb: [clang][cli] Port ObjCMTAction to new option parsing system: rGf917356f9ce0: Revert "[clang][cli] Port ObjCMTAction to new option parsing system".
Wed, Nov 11, 12:01 PM
mehdi_amini committed rGf917356f9ce0: Revert "[clang][cli] Port ObjCMTAction to new option parsing system" (authored by mehdi_amini).
Revert "[clang][cli] Port ObjCMTAction to new option parsing system"
Wed, Nov 11, 12:01 PM