[all-commits] [llvm/llvm-project] 37da2a: [mlir][LLVM] Rework the API of GEPOp

zero9178 via All-commits all-commits at lists.llvm.org
Fri Jul 29 09:32:50 PDT 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 37da2a141c6aa02e3ef86a9010ef2b2d793a2c66
      https://github.com/llvm/llvm-project/commit/37da2a141c6aa02e3ef86a9010ef2b2d793a2c66
  Author: Markus Böck <markus.boeck02 at gmail.com>
  Date:   2022-07-29 (Fri, 29 Jul 2022)

  Changed paths:
    M mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h
    M mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
    M mlir/include/mlir/IR/BuiltinAttributes.h
    M mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
    M mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
    M mlir/test/Dialect/LLVMIR/canonicalize.mlir
    M mlir/test/Dialect/LLVMIR/dynamic-gep-index.mlir
    M mlir/test/Dialect/LLVMIR/invalid.mlir

  Log Message:
  -----------
  [mlir][LLVM] Rework the API of GEPOp

The implementation and API of GEP Op has gotten a bit convoluted over the time. Issues with it are:
* Misleading naming: `indices` actually only contains the dynamic indices, not all of them. To get the amount of indices you need to query the size of `structIndices`
* Very difficult to iterate over all indices properly: One had to iterate over `structIndices`, check whether it contains the magic constant `kDynamicIndex`, if it does, access the next value in `index` etc.
* Inconvenient to build: One either has create lots of constant ops for every index or have an odd split of passing both a `ValueRange` as well as a `ArrayRef<int32_t>` filled with `kDynamicIndex` at the correct places.
* Implementation doing verification in the build method
and more.

This patch attempts to address all these issues via convenience classes and reworking the way GEP Op works:
* Adds `GEPArg` class which is a sum type of a `int32_t` and `Value` and is used to have a single convenient easy to use `ArrayRef<GEPArg>` in the builders instead of the previous `ValueRange` + `ArrayRef<int32_t>` builders.
* Adds `GEPIndicesAdapter` which is a class used for easy random access and iteration over the indices of a GEP. It is generic and flexible enough to also instead return eg. a corresponding `Attribute` for an operand inside of `fold`.
*  Rename `structIndices` to `rawConstantIndices` and `indices` to `dynamicIndices`: `rawConstantIndices` signifies one shouldn't access it directly as it is encoded, and `dynamicIndices` is more accurate and also frees up the `indices` name.
* Add `getIndices` returning a `GEPIndicesAdapter` to easily iterate over the GEP Ops indices.
* Move the verification/asserts out of the build method and into the `verify` method emitting op error messages.
* Add convenient builder methods making use of `GEPArg`.
* Add canonicalizer turning dynamic indices with constant values into constant indices to have a canonical representation.

The only breaking change is for any users building GEPOps that have so far used the old `ValueRange` + `ArrayRef<int32_t>` builder as well as those using the generic syntax.

Another follow up patch then goes through upstream and makes use of the new `ArrayRef<GEPArg>` builder to remove a lot of code building constants for GEP indices.

Differential Revision: https://reviews.llvm.org/D130730




More information about the All-commits mailing list