[Mlir-commits] [mlir] [mlir][linalg] Handle reassociationIndices correctly for 0D tensor (PR #121683)

Andrzej WarzyƄski llvmlistbot at llvm.org
Tue Jan 7 03:01:26 PST 2025


================
@@ -611,10 +611,13 @@ static Value expandRank(PatternRewriter &rewriter, Location loc, Value tensor,
   SmallVector<SmallVector<int64_t, 2>> reassociationIndices(
       shapedType.getRank());
   int64_t index = 0;
-  for (index = 0; index <= numExtraDims; index++)
-    reassociationIndices[0].push_back(index);
-  for (size_t position = 1; position < reassociationIndices.size(); position++)
-    reassociationIndices[position].push_back(index++);
+  if (shapedType.getRank() != 0) {
----------------
banach-space wrote:

A good name is the best documentation :)

`srcTensorType`  might not be perfect, but IMHO, it's still better than something like:
```cpp
ShapedType shapedType;
```

That's similar to:
```cpp
int Int;
float Float;
```

We're missing an opportunity to add meaningful context to the variable name here. Instead, we're repeating information already conveyed by the type name.

>From my perspective, when I see `shapedType` in GitHub (which lacks proper code highlighting), it can be cognitively taxing - am I looking at a variable or a type? While this is less of an issue in a proper editor that distinguishes between the two, it would be helpful if variable naming optimised for code reviews on GitHub as well.

Perhaps `tensorType`?

Anyway, like I mentioned earlier, this is just a nice-to-have.

https://github.com/llvm/llvm-project/pull/121683


More information about the Mlir-commits mailing list