[Mlir-commits] [mlir] [mlir][Vector] Refactor VectorEmulateNarrowType.cpp (PR #123529)

[Andrzej Warzyński]

================
@@ -1127,7 +1159,8 @@ struct ConvertVectorTransferRead final
     auto origElements = op.getVectorType().getNumElements();
 
     // Note, per-element-alignment was already verified above.
-    bool isFullyAligned = origElements % emulatedPerContainerElem == 0;
+    bool isFullyAligned =
----------------
banach-space wrote:

Thanks for the suggestion!

That's a good name, but captures the "local" condition computed here rather than the "global" one I had in mind. Let me illustrate:
```cpp
 bool isPerElementAlligned = (containerBits % emulatedBits == 0);
 if (!isPerElementAlligned) {
    return failure();
 }
 
 // (...)
 
 // "local" condition
 bool isDivisibleInSize = origElements % emulatedPerContainerElem == 0;
 // "global" condition
 bool isFullyAlligned = isPerElementAlligned && isDivisibleInSize;
```

So, I see two options:
**OPTION 1**
```cpp
// Check per-element allignment
if (containerBits % emulatedBits != 0) {
    return failure();
}

// (...)
// Note, per-element-alignment was already verified above
bool isFullyAligned = (origElements % emulatedPerContainerElem == 0);
```

**OPTION 2**
```cpp
 bool isPerElementAlligned = (containerBits % emulatedBits == 0);
 if (!isPerElementAlligned) {
    return failure();
 }
 
 // (...)
 
 // "local" condition
 bool isDivisibleInSize = origElements % emulatedPerContainerElem == 0;
 // "global" condition
 bool isFullyAlligned = isPerElementAlligned && isDivisibleInSize;
```

What do you reckon? There's also ...

**OPTION 3**
```cpp
// Check per-element allignment
if (containerBits % emulatedBits != 0) {
    return failure();
}

// (...)
// Note, per-element-alignment was already verified above
bool isDivisibleInSize = (origElements % emulatedPerContainerElem == 0);
```
... but it feels half-way through OPTION 1 and OPTION 2. Not my preference.

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