- User Since
- Jun 28 2018, 2:24 AM (145 w, 5 d)
I have expressed by concerns about the dialect AVX here and in the previous discussion. We will have to rename it the moment somebody wants SSE! I don't understand what is blocking here, the renamed dialect intends to contain ops related to X86 vectors. It does not yet, but this should not preclude us from stating the intent and proceeding. If somebody fills super-strongly about the name, I can waste my time to add a random SSE op once its landed...
Sat, Apr 10
Fri, Apr 9
Thu, Apr 8
Wed, Apr 7
Tue, Apr 6
Would you mind providing a commit description in addition to the one-line summary? A good description explains _why_ this new functionality is useful, rather than what the change does.
Fri, Apr 2
Thu, Apr 1
Wed, Mar 31
Looks good. It would be nice to have DialectLinalg in a separate extension and generally start structuring the bindings more than the current flat representation, e.g. by adding lib/Bindings/Python/Dialect/Linalg. This could be a separate restructuring commit though.
Sat, Mar 27
Fri, Mar 26
Wed, Mar 24
Tue, Mar 23
Let me get a bit more practical here. Do you think this patch would have been more acceptable if it also included a test that exposed the buggy behavior in that Linalg pass?
This is tested, although indirectly, by the linalg fusion-on-tensors pass that was changed to use the functionality. The buggy functionality is in upstream code, not exposing the bug upstream is justification enough for me. We can argue whether the linalg code was actually buggy, or was just relying on the behavior of the rewriter that was not specified as either part of the contract or not part of it (also, https://www.hyrumslaw.com/). I think minimizing the test that exposes the problem and adding it upstream would be beneficial for everyone, and is reasonable request, post-commit or otherwise. Suggestions on how to test the order-of-traversal directly are welcome. And I agree that we can do better on explaining our changes and our requests.
Mon, Mar 22
Please fix the linter messages and consider folding RootKind into some pointer.
Looks like this was already fixed, but thanks for the patch!
Fri, Mar 19
Thu, Mar 18
Wed, Mar 17
forgot to git-add a test
I don't understand this change. What are the other, non-float types that are passed here? Why shouldn't we just assert here (having a FloatAttr expect a FloatType sounds reasonable)?
Tue, Mar 16
I wonder if this is something that should be instead implemented as an "expansion" pass on math dialect, converting it to std.constant, std.addf and math.log that are already handled by further lowerings.
Mon, Mar 15
LLVMAVX512 no longer exists, please just use AVX512 in names
Here's the list of broken tests
This introduced a memory error and had to be rolled back. The error is visible under ASAN on several tests, here's mlir-opt -split-input-file -convert-linalg-to-spirv -canonicalize -verify-diagnostics mlir/test/Conversion/LinalgToSPIRV/linalg-to-spirv.mlir for example.
Mar 13 2021
Mar 12 2021