[mlir][vector] Standarise operand naming

[banach-space]

Task

Standarise operand naming for Vector dialect operations.

Context

Below is a list of vector dialect operations that move values from an abstract source to an abstract destination, i.e. "read"/"write" operations:

• vector.load, vector.store, vector.transfer_read,
  vector.transfer_write, vector.gather, vector.scatter,
  vector.compressstore, vector.expandload, vector.maskedload,
  vector.maskedstore, vector.extract, vector.insert,
  vector.scalable_extract, vector.scalable_insert,
  vector.extract_strided_slice, vector.insert_strided_slice.

| **Vector Dialect Op**          | **Operand Names**        | **Operand Types**             | **Result Name**  | **Result Type**      |
|--------------------------------|--------------------------|-------------------------------|------------------|----------------------|
| `vector.load`                  | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.store`                 | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.transfer_read`         | `source`                 | `memref` / `tensor`           | `vector`         | `vector`             |
| `vector.transfer_write`        | `vector`, `source`       | `vector`, `memref`/ `tensor`  | `result`         | `vector`             |
| `vector.gather`                | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.scatter`               | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.expandload`            | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.compressstore`         | `valueToStore`,`base`    | `vector`, `memref`            | -                | -                    |
| `vector.maskedload`            | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.maskedstore`           | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.extract`               | `vector`                 | `vector`                      | `result`         | `scalar` / `vector`  |
| `vector.insert`                | `source`, `dest`         | `scalar` / `vector`, `vector` | `result`         | `vector`             |
| `vector.scalable_extract`      | `source`                 | `vector`                      | `res`            | `scalar` / `vector`  |
| `vector.scalable_insert`       | `source`, `dest`         | `scalar` / `vector`, `vector` | `res`            | `vector`             |
| `vector.extract_strided_slice` | `vector`                 | `vector`                      | (missing name)   | `vector`             |
| `vector.insert_strided_slice`  | `source`, `dest`         | `vector`                      | `res`            | `vector`             |

Note that "read" operations take one operand ("from"), whereas "write"
operations require two ("value-to-store" and "to").

Observations

Each "read" operation has a "from" argument, while each "write" operation has a
"to" and a "value-to-store" operand. However, the naming conventions are
inconsistent, making it difficult to extract common patterns or determine
operand roles. Here are some inconsistencies:

• getBase() in vector.load refers to the "from" operand (source).
• getBase() in vector.store refers to the "to" operand (destination).
• vector.transfer_read and vector.transfer_write use getSource(), which:
  • Conflicts with the vector.load / vector.store naming pattern.
  • Does not clearly indicate whether the operand represents a source
    or destination.
• vector.insert defines getSource() and getDest(), making the distinction
  between "to" and "from" operands clear. However, its sibling operation,
  vector.extract, only defines getVector(), making it unclear whether it
  represents a source or destination.
• vector.store uses getValueToStore(), whereas
  vector.insert_strided_slice does not.

There is no consistent way to identify:

• "from" (read operand)
• "to" (write operand)
• "value-to-store" (written value)

Proposed re-naming

To address these inconsistencies, we propose adopting the following standardised naming conventions:

| **Vector Dialect Op**          | **Operand Names**        | **Operand Types**             | **Result Name**  | **Result Type**      |
|--------------------------------|--------------------------|-------------------------------|------------------|----------------------|
| `vector.load`                  | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.store`                 | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.transfer_read`         | `base`                   | `memref` / `tensor`           | `result`         | `vector`             |
| `vector.transfer_write`        | `valueToStore`, `base`   | `vector`, `memref`/ `tensor`  | `result`         | `vector`             |
| `vector.gather`                | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.scatter`               | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.expandload`            | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.compressstore`         | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.maskedload`            | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.maskedstore`           | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.extract`               | `source`                 | `vector`                      | `result`         | `scalar` / `vector`  |
| `vector.insert`                | `valueToStore`, `dest`   | `scalar` / `vector`, `vector` | `result`         | `vector`             |
| `vector.scalable_extract`      | `source`                 | `vector`                      | `result`         | `scalar` / `vector`  |
| `vector.scalable_insert`       | `valueToStore`, `dest`   | `scalar` / `vector`, `vector` | `result`         | `vector`             |
| `vector.extract_strided_slice` | `source`                 | `vector`                      | `result`         | `vector`             |
| `vector.insert_strided_slice`  | `valueToStore`, `dest`   | `vector`                      | `result`         | `vector`             |

Next steps

I will leave this open for 1-2 weeks to gather feedback. If there is broad support, I will prepare a patch to implement these changes.

UPDATE 3/4/24

List of PRs that implement this (updated whenever a new PR is sent):

• [mlir][vector] Standardise valueToStore Naming Across Vector Ops (NFC) #134206

Thanks!

[llvmbot]

@llvm/issue-subscribers-mlir-vectorops

Author: Andrzej Warzyński (banach-space)

Below is a list of vector dialect operations that move values from an abstract **source** to an abstract **destination**, i.e. "read"/"write" operations:

• vector.load, vector.store, vector.transfer_read,
  vector.transfer_write, vector.gather, vector.scatter,
  vector.compressstore, vector.expandload, vector.maskedload,
  vector.maskedstore, vector.extract, vector.insert,
  vector.scalable_extract, vector.scalable_insert,
  vector.extract_strided_slice, vector.insert_strided_slice.

| **Vector Dialect Op**          | **Operand Names**        | **Operand Types**             | **Result Name**  | **Result Type**      |
|--------------------------------|--------------------------|-------------------------------|------------------|----------------------|
| `vector.load`                  | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.store`                 | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.transfer_read`         | `source`                 | `memref` / `tensor`           | `vector`         | `vector`             |
| `vector.transfer_write`        | `vector`, `source`       | `vector`, `memref`/ `tensor`  | `result`         | `vector`             |
| `vector.gather`                | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.scatter`               | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.expandload`            | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.compressstore`         | `valueToStore`,`base`    | `vector`, `memref`            | -                | -                    |
| `vector.maskedload`            | `base`                   | `memref`                      | `result`         | `vector`             |
| `vector.maskedstore`           | `valueToStore`, `base`   | `vector`, `memref`            | -                | -                    |
| `vector.extract`               | `vector`                 | `vector`                      | `result`         | `scalar` / `vector`  |
| `vector.insert`                | `source`, `dest`         | `scalar` / `vector`, `vector` | `result`         | `vector`             |
| `vector.scalable_extract`      | `source`                 | `vector`                      | `res`            | `scalar` / `vector`  |
| `vector.scalable_insert`       | `source`, `dest`         | `scalar` / `vector`, `vector` | `res`            | `vector`             |
| `vector.extract_strided_slice` | `vector`                 | `vector`                      | (missing name)   | `vector`             |
| `vector.insert_strided_slice`  | `source`, `dest`         | `vector`                      | `res`            | `vector`             |

Note that "read" operations take one operand ("from"), whereas "write"
operations require two ("value-to-store" and "to").

Observations

Each "read" operation has a "from" argument, while each "write" operation has a
"to" and a "value-to-store" operand. However, the naming conventions are
inconsistent, making it difficult to extract common patterns or determine
operand roles. Here are some inconsistencies:

• getBase() in vector.load refers to the "from" operand (source).
• getBase() in vector.store refers to the "to" operand (destination).
• vector.transfer_read and vector.transfer_write use getSource(), which:
  • Conflicts with the vector.load / vector.store naming pattern.
  • Does not clearly indicate whether the operand represents a source
    or destination.
• vector.insert defines getSource() and getDest(), making the distinction
  between "to" and "from" operands clear. However, its sibling operation,
  vector.extract, only defines getVector(), making it unclear whether it
  represents a source or destination.
• vector.store uses getValueToStore(), whereas
  vector.insert_strided_slice does not.

There is no consistent way to identify:

• "from" (read operand)
• "to" (write operand)
• "value-to-store" (written value)

[dcaballe]

Thanks, SGTM! I would perhaps try to align valueToStore in store operations with the source in vector insert operations as they are pretty much representing the same. Perhaps using source for both of them? (I really don't have an opinion...)

[banach-space]

I would perhaps try to align valueToStore in store operations with the source in vector insert operations as they are pretty much representing the same.

Good point! I have update the table accordingly .

Perhaps using source for both of them? (I really don't have an opinion...)

I feel that source is already used for many other purposes, so I'd like to avoid overloading it further. That's why I proposed valueToStore, which is already in use. The name is secondary - once we standardise it, changing it across all ops would be relatively easy if we identify a more fitting name later.

[banach-space]

I've sent my first PR for this (I intend to do it in a few smaller steps). I will keep an up-to-date list of PRs in the summary.

Thanks!