Page MenuHomePhabricator

Benoit (Benoit Jacob)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 8 2021, 11:11 AM (68 w, 3 d)

Recent Activity

Yesterday

Benoit committed rG030b36a44c35: Useful error when input dim is unused by LHS/RHS. (authored by Benoit).
Useful error when input dim is unused by LHS/RHS.
Thu, Jun 30, 10:46 AM · Restricted Project, Restricted Project
Benoit closed D128925: Useful error when input dim is unused by LHS/RHS..
Thu, Jun 30, 10:46 AM · Restricted Project, Restricted Project
Benoit added inline comments to D128925: Useful error when input dim is unused by LHS/RHS..
Thu, Jun 30, 9:11 AM · Restricted Project, Restricted Project
Benoit updated the diff for D128925: Useful error when input dim is unused by LHS/RHS..

review comments, test, lint

Thu, Jun 30, 9:10 AM · Restricted Project, Restricted Project
Benoit added a comment to D128925: Useful error when input dim is unused by LHS/RHS..

Generates this message:

shell
benoitjacob@cloud:~/mlir-build$ cmake --build . --target mlir-opt && bin/mlir-opt /tmp/x.mlir
[6/6] Linking CXX executable bin/mlir-opt
/tmp/x.mlir:2:13: error: 'vector.contract' op expected all input dimensions to be used by either the LHS or the RHS.
  %result = vector.contract {
            ^
/tmp/x.mlir:2:13: note: see current operation: %0 = "vector.contract"(%arg0, %arg1, %arg2) {indexing_maps = [affine_map<(d0, d1, d2) -> (d0, d2)>, affine_map<(d0, d1, d2) -> (d2)>, affine_map<(d0, d1, d2) -> (d1)>], iterator_types = ["reduction", "parallel", "reduction"], kind = #vector.kind<add>} : (vector<1x2xi32>, vector<2xi32>, vector<1xi32>) -> vector<1xi32>
Thu, Jun 30, 9:01 AM · Restricted Project, Restricted Project
Benoit updated the diff for D128925: Useful error when input dim is unused by LHS/RHS..

better message

Thu, Jun 30, 9:00 AM · Restricted Project, Restricted Project
Benoit requested review of D128925: Useful error when input dim is unused by LHS/RHS..
Thu, Jun 30, 8:57 AM · Restricted Project, Restricted Project

Wed, Jun 29

Benoit committed rG694ad3eaef7a: Fix CombineContractBroadcast folding reduction iterators. (authored by Benoit).
Fix CombineContractBroadcast folding reduction iterators.
Wed, Jun 29, 11:06 AM · Restricted Project, Restricted Project
Benoit closed D128739: Fix CombineContractBroadcast folding reduction iterators..
Wed, Jun 29, 11:06 AM · Restricted Project, Restricted Project
Benoit updated the diff for D128739: Fix CombineContractBroadcast folding reduction iterators..

more review comments

Wed, Jun 29, 9:48 AM · Restricted Project, Restricted Project
Benoit added a comment to D128739: Fix CombineContractBroadcast folding reduction iterators..

Addressed review comments.

Wed, Jun 29, 8:35 AM · Restricted Project, Restricted Project
Benoit added inline comments to D128739: Fix CombineContractBroadcast folding reduction iterators..
Wed, Jun 29, 8:34 AM · Restricted Project, Restricted Project
Benoit updated the diff for D128739: Fix CombineContractBroadcast folding reduction iterators..

addressed review comments

Wed, Jun 29, 8:31 AM · Restricted Project, Restricted Project

Tue, Jun 28

Benoit updated the summary of D128739: Fix CombineContractBroadcast folding reduction iterators..
Tue, Jun 28, 10:03 AM · Restricted Project, Restricted Project
Benoit requested review of D128739: Fix CombineContractBroadcast folding reduction iterators..
Tue, Jun 28, 9:54 AM · Restricted Project, Restricted Project

Apr 13 2022

Benoit added a comment to D123684: [Scalarizer] Improve extractelement handling when returned type is vector pointer (PR54469).

Is this related to https://github.com/llvm/llvm-project/issues/54469 ?

Apr 13 2022, 10:14 AM · Restricted Project, Restricted Project

Mar 20 2022

Benoit abandoned D122026: just resize the cache, dont assert equality of sizes.

The above 2 lines really were a full testcase -- just needed to wrap that in a function!

Mar 20 2022, 6:56 PM · Restricted Project, Restricted Project

Mar 19 2022

Benoit added a comment to D122026: just resize the cache, dont assert equality of sizes.

OK, I have dug deeper now :-)

Mar 19 2022, 7:53 PM · Restricted Project, Restricted Project

Mar 18 2022

Benoit added a comment to D122026: just resize the cache, dont assert equality of sizes.

This looks like a consistency assertion that an <n x %ty> vector has an n-element scatter in the cache -- so it's not obvious to me that just dropping the assertion is fine, I think you might be hiding a legitimate issue here.

Mar 18 2022, 7:14 PM · Restricted Project, Restricted Project
Benoit removed a reviewer for D122026: just resize the cache, dont assert equality of sizes: bjope.
Mar 18 2022, 7:01 PM · Restricted Project, Restricted Project
Benoit added a reviewer for D122026: just resize the cache, dont assert equality of sizes: bjope.
Mar 18 2022, 12:31 PM · Restricted Project, Restricted Project
Benoit updated the summary of D122026: just resize the cache, dont assert equality of sizes.
Mar 18 2022, 12:31 PM · Restricted Project, Restricted Project
Benoit updated the summary of D122026: just resize the cache, dont assert equality of sizes.
Mar 18 2022, 12:30 PM · Restricted Project, Restricted Project
Benoit requested review of D122026: just resize the cache, dont assert equality of sizes.
Mar 18 2022, 12:04 PM · Restricted Project, Restricted Project

Mar 10 2022

Benoit added a comment to D121182: Expose ScalarizerPass options to C++ (not just commandline).

I don't have write access. Can someone please push this for me?

Mar 10 2022, 8:18 AM · Restricted Project, Restricted Project
Benoit updated the diff for D121182: Expose ScalarizerPass options to C++ (not just commandline).

review comments

Mar 10 2022, 8:17 AM · Restricted Project, Restricted Project

Mar 9 2022

Benoit updated the summary of D121182: Expose ScalarizerPass options to C++ (not just commandline).
Mar 9 2022, 1:37 PM · Restricted Project, Restricted Project
Benoit updated the diff for D121182: Expose ScalarizerPass options to C++ (not just commandline).

rebase

Mar 9 2022, 9:58 AM · Restricted Project, Restricted Project
Benoit updated the diff for D121293: Fix linking error, undefined class static constants..

rebased

Mar 9 2022, 9:57 AM · Restricted Project, Restricted Project
Benoit added a comment to D121293: Fix linking error, undefined class static constants..

I don't have write access to the repository. Can someone push this for me?

Mar 9 2022, 7:47 AM · Restricted Project, Restricted Project
Benoit added inline comments to D121182: Expose ScalarizerPass options to C++ (not just commandline).
Mar 9 2022, 7:33 AM · Restricted Project, Restricted Project
Benoit updated the diff for D121182: Expose ScalarizerPass options to C++ (not just commandline).

give the cl opts precedence. per review comment

Mar 9 2022, 7:32 AM · Restricted Project, Restricted Project
Benoit added reviewers for D121293: Fix linking error, undefined class static constants.: spupyrev, wenlei, hoy.
Mar 9 2022, 7:24 AM · Restricted Project, Restricted Project
Benoit requested review of D121293: Fix linking error, undefined class static constants..
Mar 9 2022, 7:22 AM · Restricted Project, Restricted Project

Mar 8 2022

Benoit updated the summary of D121182: Expose ScalarizerPass options to C++ (not just commandline).
Mar 8 2022, 6:45 PM · Restricted Project, Restricted Project
Benoit retitled D121182: Expose ScalarizerPass options to C++ (not just commandline) from (WIP - DO NOT REVIEW YET) Expose ScalarizerPass options to C++ (not just commandline) to Expose ScalarizerPass options to C++ (not just commandline).
Mar 8 2022, 6:43 PM · Restricted Project, Restricted Project
Benoit updated the diff for D121182: Expose ScalarizerPass options to C++ (not just commandline).

clang-format

Mar 8 2022, 6:35 PM · Restricted Project, Restricted Project
Benoit updated the diff for D121182: Expose ScalarizerPass options to C++ (not just commandline).

fixes

Mar 8 2022, 6:33 PM · Restricted Project, Restricted Project

Mar 7 2022

Benoit retitled D121182: Expose ScalarizerPass options to C++ (not just commandline) from Expose ScalarizerPass options to C++ (not just commandline) to (WIP - DO NOT REVIEW YET) Expose ScalarizerPass options to C++ (not just commandline).
Mar 7 2022, 9:01 PM · Restricted Project, Restricted Project
Benoit added a comment to D121182: Expose ScalarizerPass options to C++ (not just commandline).

Context: I needed this for https://github.com/google/iree/pull/8474 . I found that TSan instrumentation expects vector sizes to be <= 16, and in my project (IREE) we have tests with higher vector sizes. That left some tests function uninstrumented, resulting in crashes as instrumented called into them.

Mar 7 2022, 9:01 PM · Restricted Project, Restricted Project
Benoit requested review of D121182: Expose ScalarizerPass options to C++ (not just commandline).
Mar 7 2022, 8:45 PM · Restricted Project, Restricted Project

Mar 3 2022

Benoit abandoned D120964: Fix a data race reported by TSan on allowsUnregisteredDialects.
Mar 3 2022, 8:07 PM · Restricted Project, Restricted Project
Benoit added a comment to D120964: Fix a data race reported by TSan on allowsUnregisteredDialects.

Thanks for the quick reply! Great to hear this is going to be resolved in the principled way. I'll switch to fixing the faulty call sites in IREE, then.

Mar 3 2022, 8:07 PM · Restricted Project, Restricted Project
Benoit updated the diff for D120964: Fix a data race reported by TSan on allowsUnregisteredDialects.

clarify comment

Mar 3 2022, 8:03 PM · Restricted Project, Restricted Project
Benoit updated subscribers of D120964: Fix a data race reported by TSan on allowsUnregisteredDialects.

FYI @benvanik @GMNGeoffrey @mravishankar

Mar 3 2022, 7:58 PM · Restricted Project, Restricted Project
Benoit requested review of D120964: Fix a data race reported by TSan on allowsUnregisteredDialects.
Mar 3 2022, 7:57 PM · Restricted Project, Restricted Project

Feb 24 2022

Benoit added a comment to D120358: Add a lowering of quantized_matmul to matmul..

For what it's worth: the new home of this is https://github.com/google/iree/pull/8409

Feb 24 2022, 7:35 PM · Restricted Project
Benoit abandoned D120358: Add a lowering of quantized_matmul to matmul..

Abandoning: per discussion with @mravishankar , quantized_matmul may be an oddity not worthy of a new MLIR core pattern. Going to take this to IREE for now. Feel free to reopen / take over this if there is interest in this on the MLIR core side.

Feb 24 2022, 1:36 PM · Restricted Project
Benoit removed reviewers for D120358: Add a lowering of quantized_matmul to matmul.: nicolasvasilache, mravishankar.
Feb 24 2022, 1:34 PM · Restricted Project
Benoit updated the summary of D120358: Add a lowering of quantized_matmul to matmul..
Feb 24 2022, 11:23 AM · Restricted Project
Benoit updated the diff for D120358: Add a lowering of quantized_matmul to matmul..

remove unneeded using namespace

Feb 24 2022, 11:23 AM · Restricted Project
Benoit retitled D120358: Add a lowering of quantized_matmul to matmul. from [WIP][DRAFT]quantized-matmul-to-matmul to Add a lowering of quantized_matmul to matmul..
Feb 24 2022, 11:12 AM · Restricted Project
Benoit updated the diff for D120358: Add a lowering of quantized_matmul to matmul..

Ready for review!

Feb 24 2022, 11:09 AM · Restricted Project

Feb 22 2022

Benoit retitled D120358: Add a lowering of quantized_matmul to matmul. from quantized-matmul-to-matmul to [WIP][DRAFT]quantized-matmul-to-matmul.
Feb 22 2022, 2:20 PM · Restricted Project
Benoit requested review of D120358: Add a lowering of quantized_matmul to matmul..
Feb 22 2022, 2:18 PM · Restricted Project

Feb 8 2022

Benoit added a comment to D119202: Add case to handle 0-D vectors in FlattenContiguousRowMajorTransferWritePattern and FlattenContiguousRowMajorTransferReadPattern..

@Benoit could you take a look as I believe you added those patterns. Why doesn't the memref has to be rank 1 for the pattern to skip? Do you know why the unit tests above are disaled?

Better ask @nicolasvasilache ! In https://reviews.llvm.org/D114993 I had originally written a larger number of tests. Nicolas took over and disabled/removed tests.

ok, what about the failure condition, why do we have to care about the memref rank?

Feb 8 2022, 10:02 AM · Restricted Project

Feb 7 2022

Benoit added a comment to D119202: Add case to handle 0-D vectors in FlattenContiguousRowMajorTransferWritePattern and FlattenContiguousRowMajorTransferReadPattern..

@Benoit could you take a look as I believe you added those patterns. Why doesn't the memref has to be rank 1 for the pattern to skip? Do you know why the unit tests above are disaled?

Feb 7 2022, 7:49 PM · Restricted Project
Benoit accepted D119202: Add case to handle 0-D vectors in FlattenContiguousRowMajorTransferWritePattern and FlattenContiguousRowMajorTransferReadPattern..
Feb 7 2022, 7:43 PM · Restricted Project

Jan 13 2022

Benoit updated the diff for D114808: Enable ReassociatingReshapeOpConversion with "non-identity" layouts..

rebased and confirmed it is still needed to use vector transfer flattening.

Jan 13 2022, 9:32 AM · Restricted Project

Dec 15 2021

Benoit added a comment to D114808: Enable ReassociatingReshapeOpConversion with "non-identity" layouts..

I don't have permissions to submit this myself.

Dec 15 2021, 10:12 AM · Restricted Project

Dec 14 2021

Benoit updated the diff for D114808: Enable ReassociatingReshapeOpConversion with "non-identity" layouts..

rebased

Dec 14 2021, 1:03 PM · Restricted Project

Dec 13 2021

Benoit updated the diff for D114993: Patterns flattening vector transfers to 1D.

rebased

Dec 13 2021, 8:24 AM · Restricted Project
Benoit added a comment to D114808: Enable ReassociatingReshapeOpConversion with "non-identity" layouts..

Thanks for the review and explanation. I have updated the code to apply your inline comment i.e. restrict the change to the static shape case.
I haven't updated docs as you suggested because I'm out of my depth here, sorry.

Dec 13 2021, 8:08 AM · Restricted Project
Benoit updated the diff for D114808: Enable ReassociatingReshapeOpConversion with "non-identity" layouts..

restrict to static shape case

Dec 13 2021, 8:06 AM · Restricted Project

Dec 12 2021

Benoit added a comment to D114993: Patterns flattening vector transfers to 1D.

Hi Nicolas, thanks for the kind supportive words here regarding the usefulness of these patterns/helpers.

Dec 12 2021, 8:25 PM · Restricted Project
Benoit updated the diff for D114993: Patterns flattening vector transfers to 1D.

More review comments.

Dec 12 2021, 8:10 PM · Restricted Project
Benoit updated the diff for D114993: Patterns flattening vector transfers to 1D.

Apply Mahesh's suggestion to use inferRankReducedResultType.

Dec 12 2021, 7:17 PM · Restricted Project

Dec 10 2021

Benoit added inline comments to D114993: Patterns flattening vector transfers to 1D.
Dec 10 2021, 8:19 AM · Restricted Project
Benoit updated the diff for D114993: Patterns flattening vector transfers to 1D.

more review comments

Dec 10 2021, 8:15 AM · Restricted Project
Benoit added inline comments to D114993: Patterns flattening vector transfers to 1D.
Dec 10 2021, 7:32 AM · Restricted Project
Benoit updated the diff for D114993: Patterns flattening vector transfers to 1D.

split into 2 patterns

Dec 10 2021, 7:12 AM · Restricted Project
Benoit updated the diff for D114993: Patterns flattening vector transfers to 1D.

apply some review comments

Dec 10 2021, 5:55 AM · Restricted Project

Dec 3 2021

Benoit abandoned D114821: isReshapableDimBand: ignore strides of unit dims..

Thank you all for weighing in. After discussion with @nicolasvasilache and @ThomasRaoux today I switch to dropping the unit dims, which caused my need for the present diff, by inserting rank-reducing memref.subview in the pattern that is what generating the problematic memref.collapse_shape, see https://reviews.llvm.org/D114993 . So I can now close this diff, but thanks all for the conversation.

Dec 3 2021, 8:19 PM · Restricted Project
Benoit updated subscribers of D114993: Patterns flattening vector transfers to 1D.

Thanks for the review comments! I'll apply them on monday. For now I've just updated this diff with the new idea to use rank reducing subviews to drop unit dims, (thanks @ThomasRaoux for the suggestion), that removes my need for https://reviews.llvm.org/D114821 so I can drop it now.

Dec 3 2021, 8:16 PM · Restricted Project
Benoit updated the diff for D114993: Patterns flattening vector transfers to 1D.

add rank reducing subview to drop unit dims

Dec 3 2021, 8:15 PM · Restricted Project
Benoit accepted D115078: Split the locking of the queue and the threads vector in the ThreadPool implementation.
Dec 3 2021, 7:24 PM · Restricted Project
Benoit added a comment to D115019: ThreadPool: grow the pool only as needed.

Sorry! Pushed a fix in b28f317c8156 and will monitor the bot tonight.

Dec 3 2021, 7:16 PM · Restricted Project
Benoit added a comment to D115019: ThreadPool: grow the pool only as needed.

This should be good to submit now.

Dec 3 2021, 1:31 PM · Restricted Project
Benoit updated the diff for D115019: ThreadPool: grow the pool only as needed.

fix preexisting race condition in isWorkerThread found by MLIR tests with TSan

Dec 3 2021, 1:29 PM · Restricted Project
Benoit added a comment to D115019: ThreadPool: grow the pool only as needed.

Wait a bit more before submitting... we're still debugging CI failures.

Dec 3 2021, 12:34 PM · Restricted Project
Benoit updated the diff for D114821: isReshapableDimBand: ignore strides of unit dims..

Rebase

Dec 3 2021, 7:01 AM · Restricted Project

Dec 2 2021

Benoit added a comment to D115019: ThreadPool: grow the pool only as needed.

LG, thanks!

Dec 2 2021, 8:29 PM · Restricted Project
Benoit updated the diff for D115019: ThreadPool: grow the pool only as needed.

review comment

Dec 2 2021, 8:27 PM · Restricted Project
Benoit added a comment to D115019: ThreadPool: grow the pool only as needed.

I'm fairly sure I fixed this one or two months ago, at least the C++ API allows to setup a project without starting a ThreadPool at all. This was a bottleneck in TensorFlow for some small MLIR work we're doing there.

There might be some similar plumbing we could do in how we handle --mlir-disable-threading with mlir-opt as well, since it is a testing tool we haven't tried to "optimize" this kind of things (you're making a good case for it though!).

Dec 2 2021, 8:24 PM · Restricted Project
Benoit updated the diff for D115019: ThreadPool: grow the pool only as needed.

Address review comments.

Dec 2 2021, 8:19 PM · Restricted Project
Benoit updated subscribers of D114821: isReshapableDimBand: ignore strides of unit dims..

https://reviews.llvm.org/D114702 landed a few days ago and fixes the dim vs idx issue that I was reporting here! Thanks @herhut !

Dec 2 2021, 8:00 PM · Restricted Project
Benoit added inline comments to D115019: ThreadPool: grow the pool only as needed.
Dec 2 2021, 7:42 PM · Restricted Project
Benoit updated the diff for D115019: ThreadPool: grow the pool only as needed.

Got it, getThreadCount() was really meant to return the max number of threads, not the current number. Those two numbers used to be the same before this patch.

Dec 2 2021, 7:39 PM · Restricted Project
Benoit added inline comments to D115019: ThreadPool: grow the pool only as needed.
Dec 2 2021, 7:11 PM · Restricted Project
Benoit added inline comments to D115019: ThreadPool: grow the pool only as needed.
Dec 2 2021, 7:05 PM · Restricted Project
Benoit updated the diff for D115019: ThreadPool: grow the pool only as needed.

off by one mistake in max number of worker threads

Dec 2 2021, 6:54 PM · Restricted Project
Benoit added a reviewer for D115019: ThreadPool: grow the pool only as needed: mehdi_amini.
Dec 2 2021, 6:48 PM · Restricted Project
Benoit requested review of D115019: ThreadPool: grow the pool only as needed.
Dec 2 2021, 6:47 PM · Restricted Project
Benoit added a reviewer for D114993: Patterns flattening vector transfers to 1D: mravishankar.
Dec 2 2021, 1:18 PM · Restricted Project
Benoit requested review of D114993: Patterns flattening vector transfers to 1D.
Dec 2 2021, 1:18 PM · Restricted Project

Dec 1 2021

Benoit updated the diff for D114821: isReshapableDimBand: ignore strides of unit dims..

newline at end of file; minor wording update

Dec 1 2021, 7:34 AM · Restricted Project
Benoit updated the summary of D114821: isReshapableDimBand: ignore strides of unit dims..
Dec 1 2021, 7:30 AM · Restricted Project
Benoit added reviewers for D114821: isReshapableDimBand: ignore strides of unit dims.: mravishankar, pifon2a.
Dec 1 2021, 7:30 AM · Restricted Project

Nov 30 2021

Benoit requested review of D114821: isReshapableDimBand: ignore strides of unit dims..
Nov 30 2021, 2:25 PM · Restricted Project
Benoit updated the diff for D114808: Enable ReassociatingReshapeOpConversion with "non-identity" layouts..

Rename the test function - was not match what it actually did

Nov 30 2021, 1:19 PM · Restricted Project