[mlir][LLVM] Rework the API of GEPOp

[zero9178]

Summary

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.

---

Some notes of things I'd like to have done or still need to be done that this patch doesn't address:

• I would have really like to have required callers to pass constant indices to GEPArg for struct indices, instead of allowing SSA values that are constants. When implementing this I hit some nasty cases in flang however and I imagine other downstream projects would also struggle with such a vast breaking change.
• Verification is still quite incomplete, but is not regressed by this patch either.

[ftynse]

Thanks! I am very favorable of this direction. The encoding scheme with offset is a bit too clever IMO, I would prefer using the original scheme.

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

88

Nit: if these are intentionally implicit, consider adding a prefix comment /*implicit*/

122–126

This encoding is clever but makes it significantly harder to reason about raw IR (e.g., in the generic form while debugging): one has to visually a decimal value to 2^31-1, and then do large subtractions. I would much prefer the original convention of having one magic value and counting the number of these values to get the index in the list of dynamic values. The original is also consistent with subview/insertslice operations that are pretty pervasive in MLIR. The adaptor class would still work and hide the logic from the code, even if it is slightly less performant

129

Nit: some comments use adaptEr but the code uses adaptOr. The latter spelling is used elsewhere in MLIR.

161–165

I suppose something like

using GEPStaticSize = llvm::PointerEmbeddedInt<int32_t>;
using GEPArg = llvm::PointerUnion<Value, GEPStaticSize>;

will have this covered.

173

Nit: return rawConstantIndices.empty()

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

427

Using i64 here may be confusing because LLVM specifically requires constant indices to be i32.

[ftynse]

I would have really like to have required callers to pass constant indices to GEPArg for struct indices, instead of allowing SSA values that are constants. When implementing this I hit some nasty cases in flang however and I imagine other downstream projects would also struggle with such a vast breaking change.

There are plenty of those indeed. I would suggest sending out the patch + a PSA on the forum, then given some time to downstream projects to adapt.

[zero9178]

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

122–126

Do you think that GEPIndicesAdaptor should then still offer an operator[]? I think it'd then be misleading as such an operator somewhat applies random access and the associated constant time complexity.
There currently aren't really any uses of operator[] that can't just be rewritten with iterators at the moment so it should be fine in that regard.

If changing it back to the original encoding I'd make the iterators forward iterators and remove the operator[].

[ftynse]

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

122–126

I don't mind keeping random access as long as its documentation explains why it is not zero-cost. This is not a generic utility, and we are unlikely to have cases that actually hit performance problems here. GEPs with more than a dozen indices are rather rare.

[zero9178]

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

161–165

That is very nice! Didn't know about PointerEmbeddedInt and was close to doing it myself via void*.

That said, this could be problematic on 32 bit platforms right? Since we essentially do need the whole 32 bit, if we are on a platform with 32 bit pointers, we'd have to cut a few bits from the integer to make it fit. Is this okay?

[zero9178]

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

161–165

Alterntaively we could also just shave off a bits for that purpose. A 28 bit integer would eg. still be practically enough for any struct.

[ftynse]

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

161–165

I actually don't know if MLIR even builds on a 32-bit platform given the amount of int64_t and friends floating around the codebase. I'd just reserve 2 bits (there is a second template argument); if we have a struct with 2^30 fields, I suppose we have bigger problems than this.

[mehdi_amini]

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

161–165

int64_t works fine on 32bits platforms: it's only the pointer size that is smaller (not sure I understood what you meant).
The main problem is more that unless you explicitly specify pointer alignment, you may get less bits to use when packing bits in the low part of the pointer.

[zero9178]

• Revert to the existing encoding scheme
• Address review comments

[zero9178]

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

96

I kept the class as the syntax I wanted (eg. ArrayRef<GEPArg>{0, value}) would only work if it allowed implicit conversion from int32_t. Since C++ doesn't allow more than one layers of implicit conversion (int32_t -> GEPConstantIndex -> GEPArg) I simply wrapped it and added the required constructors.

[ftynse]

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

451

Please document this method.

Also naming nit: I'd call this deconstruct or destructure simply unpack to avoid confusion with it being a destructor.

2712

2717

[zero9178]

Addressed review comments

mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.h

 #include "mlir/IR/OpImplementation.h"
 #include "mlir/IR/TypeSupport.h"
 #include "mlir/IR/Types.h"
 #include "mlir/Interfaces/CallInterfaces.h"
 #include "mlir/Interfaces/ControlFlowInterfaces.h"
 #include "mlir/Interfaces/InferTypeOpInterface.h"
 #include "mlir/Interfaces/SideEffectInterfaces.h"
 #include "mlir/Support/ThreadLocalCache.h"
+#include "llvm/ADT/PointerEmbeddedInt.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 
 #include "mlir/Dialect/LLVMIR/LLVMOpsEnums.h.inc"
 
 namespace mlir {

 struct LLVMDialectImpl;
 } // namespace detail
 } // namespace LLVM
 } // namespace mlir
 
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.h.inc"
 
+namespace mlir {
+namespace LLVM {
+template <typename Values>
+class GEPIndicesAdaptor;
+
+/// Bit-width of a 'GEPConstantIndex' within GEPArg.
+constexpr int kGEPConstantBitWidth = 29;
+/// Wrapper around a int32_t for use in a PointerUnion.
+using GEPConstantIndex =
+    llvm::PointerEmbeddedInt<int32_t, kGEPConstantBitWidth>;
+
+/// Class used for building a 'llvm.getelementptr'. A single instance represents
+/// a sum type that is either a 'Value' or a constant 'GEPConstantIndex' index.
+/// The former represents a dynamic index in a GEP operation, while the later is
+/// a constant index as is required for indices into struct types.
+class GEPArg : public PointerUnion<Value, GEPConstantIndex> {
+  using BaseT = PointerUnion<Value, GEPConstantIndex>;
+
+public:
+  /// Constructs a GEPArg with a constant index.
+  /*implicit*/ GEPArg(int32_t integer) : BaseT(integer) {}
+
+  /// Constructs a GEPArg with a dynamic index.
+  /*implicit*/ GEPArg(Value value) : BaseT(value) {}
+
+  using BaseT::operator=;
+};
+} // namespace LLVM
+} // namespace mlir
+
 ///// Ops /////
 #define GET_OP_CLASSES
 #include "mlir/Dialect/LLVMIR/LLVMOps.h.inc"
 #define GET_OP_CLASSES
 #include "mlir/Dialect/LLVMIR/LLVMIntrinsicOps.h.inc"
 
 #include "mlir/Dialect/LLVMIR/LLVMOpsDialect.h.inc"
 
 namespace mlir {
 namespace LLVM {
+
+/// Class used for convenient access and iteration over GEP indices.
+/// This class is templated to support not only retrieving the dynamic operands
+/// of a GEP operation, but also as an adaptor during folding or conversion to
+/// LLVM IR.
+///
+/// GEP indices may either be constant indices or dynamic indices. The
+/// 'rawConstantIndices' is specially encoded by GEPOp and contains either the
+/// constant index or the information that an index is a dynamic index.
+///
+/// When an access to such an index is made it is done through the
+/// 'DynamicRange' of this class. This way it can be used as getter in GEPOp via
+/// 'GEPIndicesAdaptor<ValueRange>' or during folding via
+/// 'GEPIndicesAdaptor<ArrayRef<Attribute>>'.
+template <typename DynamicRange>
+class GEPIndicesAdaptor {
+public:
+  /// Return type of 'operator[]' and the iterators 'operator*'. It is depended
+  /// upon the value type of 'DynamicRange'. If 'DynamicRange' contains
+  /// Attributes or subclasses thereof, then value_type is 'Attribute'. In
+  /// all other cases it is a pointer union between the value type of
+  /// 'DynamicRange' and IntegerAttr.
+  using value_type = std::conditional_t<
+      std::is_base_of<Attribute,
+                      llvm::detail::ValueOfRange<DynamicRange>>::value,
+      Attribute,
+      PointerUnion<IntegerAttr, llvm::detail::ValueOfRange<DynamicRange>>>;
+
+  /// Constructs a GEPIndicesAdaptor with the raw constant indices of a GEPOp
+  /// and the range that is indexed into for retrieving dynamic indices.
+  GEPIndicesAdaptor(DenseI32ArrayAttr rawConstantIndices, DynamicRange values)
+      : rawConstantIndices(rawConstantIndices), values(std::move(values)) {}
+
+  /// Returns the GEP index at the given position. Note that this operation has
+  /// a linear complexity in regards to the accessed position. To iterate over
+  /// all indices, use the iterators.
+  ///
+  /// This operation is invalid if the index is out of bounds.
+  value_type operator[](size_t index) const {
+    assert(index < size() && "index out of bounds");
+    return *std::next(begin(), index);
+  }
+
+  /// Returns whether the GEP index at the given position is a dynamic index.
+  bool isDynamicIndex(size_t index) const {
+    return rawConstantIndices[index] == GEPOp::kDynamicIndex;
+  }
+
+  /// Returns the amount of indices of the GEPOp.
+  size_t size() const { return rawConstantIndices.size(); }
+
+  /// Returns true if this GEPOp does not have any indices.
+  bool empty() const { return rawConstantIndices.empty(); }
+
+  class iterator
+      : public llvm::iterator_facade_base<iterator, std::forward_iterator_tag,
+                                          value_type, std::ptrdiff_t,
+                                          value_type *, value_type> {
+  public:
+    iterator(const GEPIndicesAdaptor *base,
+             ArrayRef<int32_t>::iterator rawConstantIter,
+             llvm::detail::IterOfRange<const DynamicRange> valuesIter)
+        : base(base), rawConstantIter(rawConstantIter), valuesIter(valuesIter) {
+    }
+
+    value_type operator*() const {
+      if (*rawConstantIter == GEPOp::kDynamicIndex)
+        return *valuesIter;
+
+      return IntegerAttr::get(
+          ElementsAttr::getElementType(base->rawConstantIndices),
+          *rawConstantIter);
+    }
+
+    iterator &operator++() {
+      if (*rawConstantIter == GEPOp::kDynamicIndex)
+        valuesIter++;
+      rawConstantIter++;
+      return *this;
+    }
+
+    bool operator==(const iterator &rhs) const {
+      return base == rhs.base && rawConstantIter == rhs.rawConstantIter &&
+             valuesIter == rhs.valuesIter;
+    }
+
+  private:
+    const GEPIndicesAdaptor *base;
+    ArrayRef<int32_t>::const_iterator rawConstantIter;
+    llvm::detail::IterOfRange<const DynamicRange> valuesIter;
+  };
+
+  /// Returns the begin iterator, iterating over all GEP indices.
+  iterator begin() const {
+    return iterator(this, rawConstantIndices.asArrayRef().begin(),
+                    values.begin());
+  }
+
+  /// Returns the end iterator, iterating over all GEP indices.
+  iterator end() const {
+    return iterator(this, rawConstantIndices.asArrayRef().end(), values.end());
+  }
+
+private:
+  DenseI32ArrayAttr rawConstantIndices;
+  DynamicRange values;
+};
+
 /// Create an LLVM global containing the string "value" at the module containing
 /// surrounding the insertion point of builder. Obtain the address of that
 /// global and use it to compute the address of the first character in the
 /// string (operations inserted at the builder insertion point).
 Value createGlobalString(Location loc, OpBuilder &builder, StringRef name,
                          StringRef value, Linkage linkage);
 
 /// LLVM requires some operations to be inside of a Module operation. This

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

let builders = [
     }]>
     ];
   let hasCustomAssemblyFormat = 1;
   let hasVerifier = 1;
 }
 
 def LLVM_GEPOp : LLVM_Op<"getelementptr", [NoSideEffect]> {
   let arguments = (ins LLVM_ScalarOrVectorOf<LLVM_AnyPointer>:$base,
-                   Variadic<LLVM_ScalarOrVectorOf<AnyInteger>>:$indices,
-                   I32ElementsAttr:$structIndices,
+                   Variadic<LLVM_ScalarOrVectorOf<AnyInteger>>:$dynamicIndices,
+                   DenseI32ArrayAttr:$rawConstantIndices,
                    OptionalAttr<TypeAttr>:$elem_type);
   let results = (outs LLVM_ScalarOrVectorOf<LLVM_AnyPointer>:$res);
   let skipDefaultBuilders = 1;
+
+  let description = [{
+    This operation mirrors LLVM IRs 'getelementptr' operation that is used to
+    perform pointer arithmetic.
+
+    Like in LLVM IR, it is possible to use both constants as well as SSA values
+    as indices. In the case of indexing within a structure, it is required to
+    either use constant indices directly, or supply a constant SSA value.
+
+    Examples:
+
+    ```mlir
+    // GEP with an SSA value offset
+    %0 = llvm.getelementptr %1[%2] : (!llvm.ptr<f32>, i64) -> !llvm.ptr<f32>
+
+    // GEP with a constant offset
+    %0 = llvm.getelementptr %1[3] : (!llvm.ptr<f32>) -> !llvm.ptr<f32>
+
+    // GEP with constant offsets into a structure
+    %0 = llvm.getelementptr %1[0, 1]
+       : (!llvm.ptr<struct(i32, f32)>) -> !llvm.ptr<f32>
+    ```
+  }];
+
   let builders = [
-    OpBuilder<(ins "Type":$resultType, "Value":$basePtr, "ValueRange":$indices,
+    OpBuilder<(ins "Type":$resultType, "Type":$basePtrType, "Value":$basePtr,
+               "ValueRange":$indices,
                CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes)>,
     OpBuilder<(ins "Type":$resultType, "Value":$basePtr, "ValueRange":$indices,
-               "ArrayRef<int32_t>":$structIndices,
                CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes)>,
-    OpBuilder<(ins "Type":$resultType, "Type":$basePtrType, "Value":$basePtr,
-               "ValueRange":$indices,
+    OpBuilder<(ins "Type":$resultType, "Value":$basePtr, "ArrayRef<GEPArg>":$indices,
                CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes)>,
-  OpBuilder<(ins "Type":$resultType, "Type":$basePtrType, "Value":$basePtr,
-               "ValueRange":$indices,
+    OpBuilder<(ins "Type":$resultType, "Type":$basePtrType, "Value":$basePtr,
+               "ArrayRef<GEPArg>":$indices,
                CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes)>,
-    OpBuilder<(ins "Type":$resultType, "Type":$basePtrType, "Value":$basePtr,
-               "ValueRange":$indices, "ArrayRef<int32_t>":$structIndices,
-               CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes)>
   ];
   let llvmBuilder = [{
     SmallVector<llvm::Value *> indices;
-    indices.reserve($structIndices.size());
-    unsigned operandIdx = 0;
-    for (int32_t structIndex : $structIndices.getValues<int32_t>()) {
-      if (structIndex == GEPOp::kDynamicIndex)
-        indices.push_back($indices[operandIdx++]);
+    indices.reserve($rawConstantIndices.size());
+    GEPIndicesAdaptor<decltype($dynamicIndices)>
+        gepIndices(op.getRawConstantIndicesAttr(), $dynamicIndices);
+    for (PointerUnion<IntegerAttr, llvm::Value*> valueOrAttr : gepIndices) {
+      if (llvm::Value* value = valueOrAttr.dyn_cast<llvm::Value*>())
+        indices.push_back(value);
       else
-        indices.push_back(builder.getInt32(structIndex));
+        indices.push_back(
+            builder.getInt32(valueOrAttr.get<IntegerAttr>().getInt()));
     }
     Type baseElementType = op.getSourceElementType();
     llvm::Type *elementType = moduleTranslation.convertType(baseElementType);
     $res = builder.CreateGEP(elementType, $base, indices);
   }];
   let assemblyFormat = [{
-    $base `[` custom<GEPIndices>($indices, $structIndices) `]` attr-dict
+    $base `[` custom<GEPIndices>($dynamicIndices, $rawConstantIndices) `]` attr-dict
     `:` functional-type(operands, results) (`,` $elem_type^)?
   }];
 
   let extraClassDeclaration = [{
-    constexpr static int kDynamicIndex = std::numeric_limits<int32_t>::min();
+    constexpr static int32_t kDynamicIndex = std::numeric_limits<int32_t>::min();
 
     /// Returns the type pointed to by the pointer argument of this GEP.
     Type getSourceElementType();
+
+    GEPIndicesAdaptor<ValueRange> getIndices();
   }];
   let hasFolder = 1;
   let hasVerifier = 1;
 }
 
 def LLVM_LoadOp : LLVM_Op<"load">, MemoryOpWithAlignmentAndAttributes {
   let arguments = (ins Arg<LLVM_PointerTo<LLVM_LoadableType>, "", [MemRead]>:$addr,
                    OptionalAttr<SymbolRefArrayAttr>:$access_groups,

mlir/include/mlir/IR/BuiltinAttributes.h

 /// supported element type below.
 template <typename T>
 class DenseArrayAttr : public DenseArrayBaseAttr {
 public:
   using DenseArrayBaseAttr::DenseArrayBaseAttr;
 
   /// Implicit conversion to ArrayRef<T>.
   operator ArrayRef<T>() const;
-  ArrayRef<T> asArrayRef() { return ArrayRef<T>{*this}; }
+  ArrayRef<T> asArrayRef() const { return ArrayRef<T>{*this}; }
+
+  /// Random access to elements.
+  T operator[](std::size_t index) const { return asArrayRef()[index]; }
 
   /// Builder from ArrayRef<T>.
   static DenseArrayAttr get(MLIRContext *context, ArrayRef<T> content);
 
   /// Print the short form `[42, 100, -1]` without any type prefix.
   void print(AsmPrinter &printer) const;
   void print(raw_ostream &os) const;
   /// Print the short form `42, 100, -1` without any braces or type prefix.

 struct PointerLikeTypeTraits<mlir::StringAttr>
     : public PointerLikeTypeTraits<mlir::Attribute> {
   static inline mlir::StringAttr getFromVoidPointer(void *p) {
     return mlir::StringAttr::getFromOpaquePointer(p);
   }
 };
 
 template <>
+struct PointerLikeTypeTraits<mlir::IntegerAttr>
+    : public PointerLikeTypeTraits<mlir::Attribute> {
+  static inline mlir::IntegerAttr getFromVoidPointer(void *p) {
+    return mlir::IntegerAttr::getFromOpaquePointer(p);
+  }
+};
+
+template <>
 struct PointerLikeTypeTraits<mlir::SymbolRefAttr>
     : public PointerLikeTypeTraits<mlir::Attribute> {
   static inline mlir::SymbolRefAttr getFromVoidPointer(void *ptr) {
     return mlir::SymbolRefAttr::getFromOpaquePointer(ptr);
   }
 };
 
 } // namespace llvm
 
 #endif // MLIR_IR_BUILTINATTRIBUTES_H

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

SuccessorOperands SwitchOp::getSuccessorOperands(unsigned index) {
   return SuccessorOperands(index == 0 ? getDefaultOperandsMutable()
                                       : getCaseOperandsMutable(index - 1));
 }
 
 //===----------------------------------------------------------------------===//
 // Code for LLVM::GEPOp.
 //===----------------------------------------------------------------------===//
 
-constexpr int GEPOp::kDynamicIndex;
+constexpr int32_t GEPOp::kDynamicIndex;
+
+GEPIndicesAdaptor<ValueRange> GEPOp::getIndices() {
+  return GEPIndicesAdaptor<ValueRange>(getRawConstantIndicesAttr(),
+                                       getDynamicIndices());
+}
+
+/// Returns the elemental type of any LLVM-compatible vector type or self.
+static Type extractVectorElementType(Type type) {
+  if (auto vectorType = type.dyn_cast<VectorType>())
+    return vectorType.getElementType();
+  if (auto scalableVectorType = type.dyn_cast<LLVMScalableVectorType>())
+    return scalableVectorType.getElementType();
+  if (auto fixedVectorType = type.dyn_cast<LLVMFixedVectorType>())
+    return fixedVectorType.getElementType();
+  return type;
+}
+
+void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
+                  Value basePtr, ArrayRef<GEPArg> indices,
+                  ArrayRef<NamedAttribute> attributes) {
+  auto ptrType =
+      extractVectorElementType(basePtr.getType()).cast<LLVMPointerType>();
+  assert(!ptrType.isOpaque() &&
+         "expected non-opaque pointer, provide elementType explicitly when "
+         "opaque pointers are used");
+  build(builder, result, resultType, ptrType.getElementType(), basePtr, indices,
+        attributes);
+}
+
+/// Destructures the 'indices' parameter into 'rawConstantIndices' and
+/// 'dynamicIndices', encoding the former in the process. In the process,
+/// dynamic indices which are used to index into a structure type are converted
+/// to constant indices when possible. To do this, the GEPs element type should
+/// be passed as first parameter.
+static void destructureIndices(Type currType, ArrayRef<GEPArg> indices,
+                               SmallVectorImpl<int32_t> &rawConstantIndices,
+                               SmallVectorImpl<Value> &dynamicIndices) {
+  for (const GEPArg &iter : indices) {
+    // If the thing we are currently indexing into is a struct we must turn
+    // any integer constants into constant indices. If this is not possible
+    // we don't do anything here. The verifier will catch it and emit a proper
+    // error. All other canonicalization is done in the fold method.
+    bool requiresConst = !rawConstantIndices.empty() &&
+                         currType.isa_and_nonnull<LLVMStructType>();
+    if (Value val = iter.dyn_cast<Value>()) {
+      APInt intC;
+      if (requiresConst && matchPattern(val, m_ConstantInt(&intC)) &&
+          intC.isSignedIntN(kGEPConstantBitWidth)) {
+        rawConstantIndices.push_back(intC.getSExtValue());
+      } else {
+        rawConstantIndices.push_back(GEPOp::kDynamicIndex);
+        dynamicIndices.push_back(val);
+      }
+    } else {
+      rawConstantIndices.push_back(iter.get<GEPConstantIndex>());
+    }
+
+    // Skip for very first iteration of this loop. First index does not index
+    // within the aggregates, but is just a pointer offset.
+    if (rawConstantIndices.size() == 1 || !currType)
+      continue;
+
+    currType =
+        TypeSwitch<Type, Type>(currType)
+            .Case<VectorType, LLVMScalableVectorType, LLVMFixedVectorType,
+                  LLVMArrayType>([](auto containerType) {
+              return containerType.getElementType();
+            })
+            .Case([&](LLVMStructType structType) -> Type {
+              int64_t memberIndex = rawConstantIndices.back();
+              if (memberIndex >= 0 && static_cast<size_t>(memberIndex) <
+                                          structType.getBody().size())
+                return structType.getBody()[memberIndex];
+              return nullptr;
+            })
+            .Default(Type(nullptr));
+  }
+}
+
+void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
+                  Type elementType, Value basePtr, ArrayRef<GEPArg> indices,
+                  ArrayRef<NamedAttribute> attributes) {
+  SmallVector<int32_t> rawConstantIndices;
+  SmallVector<Value> dynamicIndices;
+  destructureIndices(elementType, indices, rawConstantIndices, dynamicIndices);
+
+  result.addTypes(resultType);
+  result.addAttributes(attributes);
+  result.addAttribute(getRawConstantIndicesAttrName(result.name),
+                      builder.getDenseI32ArrayAttr(rawConstantIndices));
+  if (extractVectorElementType(basePtr.getType())
+          .cast<LLVMPointerType>()
+          .isOpaque())
+    result.addAttribute(kElemTypeAttrName, TypeAttr::get(elementType));
+  result.addOperands(basePtr);
+  result.addOperands(dynamicIndices);
+}
+
+void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
+                  Value basePtr, ValueRange indices,
+                  ArrayRef<NamedAttribute> attributes) {
+  build(builder, result, resultType, basePtr, SmallVector<GEPArg>(indices),
+        attributes);
+}
+
+void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
+                  Type elementType, Value basePtr, ValueRange indices,
+                  ArrayRef<NamedAttribute> attributes) {
+  build(builder, result, resultType, elementType, basePtr,
+        SmallVector<GEPArg>(indices), attributes);
+}
+
+static ParseResult
+parseGEPIndices(OpAsmParser &parser,
+                SmallVectorImpl<OpAsmParser::UnresolvedOperand> &indices,
+                DenseI32ArrayAttr &rawConstantIndices) {
+  SmallVector<int32_t> constantIndices;
+
+  auto idxParser = [&]() -> ParseResult {
+    int32_t constantIndex;
+    OptionalParseResult parsedInteger =
+        parser.parseOptionalInteger(constantIndex);
+    if (parsedInteger.hasValue()) {
+      if (failed(parsedInteger.getValue()))
+        return failure();
+      constantIndices.push_back(constantIndex);
+      return success();
+    }
+
+    constantIndices.push_back(LLVM::GEPOp::kDynamicIndex);
+    return parser.parseOperand(indices.emplace_back());
+  };
+  if (parser.parseCommaSeparatedList(idxParser))
+    return failure();
+
+  rawConstantIndices =
+      DenseI32ArrayAttr::get(parser.getContext(), constantIndices);
+  return success();
+}
+
+static void printGEPIndices(OpAsmPrinter &printer, LLVM::GEPOp gepOp,
+                            OperandRange indices,
+                            DenseI32ArrayAttr rawConstantIndices) {
+  llvm::interleaveComma(
+      GEPIndicesAdaptor<OperandRange>(rawConstantIndices, indices), printer,
+      [&](PointerUnion<IntegerAttr, Value> cst) {
+        if (Value val = cst.dyn_cast<Value>())
+          printer.printOperand(val);
+        else
+          printer << cst.get<IntegerAttr>().getInt();
+      });
+}
 
 namespace {
 /// Base class for llvm::Error related to GEP index.
 class GEPIndexError : public llvm::ErrorInfo<GEPIndexError> {
 protected:
   unsigned indexPos;
 
 public:

 };
 } // end anonymous namespace
 
 char GEPIndexError::ID = 0;
 char GEPIndexOutOfBoundError::ID = 0;
 char GEPStaticIndexError::ID = 0;
 
 /// For the given `structIndices` and `indices`, check if they're complied
-/// with `baseGEPType`, especially check against LLVMStructTypes nested within,
-/// and refine/promote struct index from `indices` to `updatedStructIndices`
-/// if the latter argument is not null.
-static llvm::Error
-recordStructIndices(Type baseGEPType, unsigned indexPos,
-                    ArrayRef<int32_t> structIndices, ValueRange indices,
-                    SmallVectorImpl<int32_t> *updatedStructIndices,
-                    SmallVectorImpl<Value> *remainingIndices) {
-  if (indexPos >= structIndices.size())
+/// with `baseGEPType`, especially check against LLVMStructTypes nested within.
+static llvm::Error verifyStructIndices(Type baseGEPType, unsigned indexPos,
+                                       GEPIndicesAdaptor<ValueRange> indices) {
+  if (indexPos >= indices.size())
     // Stop searching
     return llvm::Error::success();
 
-  int32_t gepIndex = structIndices[indexPos];
-  bool isStaticIndex = gepIndex != GEPOp::kDynamicIndex;
-
-  unsigned dynamicIndexPos = indexPos;
-  if (!isStaticIndex)
-    dynamicIndexPos = llvm::count(structIndices.take_front(indexPos + 1),
-                                  LLVM::GEPOp::kDynamicIndex) -
-                      1;
-
   return llvm::TypeSwitch<Type, llvm::Error>(baseGEPType)
       .Case<LLVMStructType>([&](LLVMStructType structType) -> llvm::Error {
-        // We don't always want to refine the index (e.g. when performing
-        // verification), so we only refine when updatedStructIndices is not
-        // null.
-        if (!isStaticIndex && updatedStructIndices) {
-          // Try to refine.
-          APInt staticIndexValue;
-          isStaticIndex = matchPattern(indices[dynamicIndexPos],
-                                       m_ConstantInt(&staticIndexValue));
-          if (isStaticIndex) {
-            assert(staticIndexValue.getBitWidth() <= 64 &&
-                   llvm::isInt<32>(staticIndexValue.getLimitedValue()) &&
-                   "struct index can't fit within int32_t");
-            gepIndex = static_cast<int32_t>(staticIndexValue.getSExtValue());
-          }
-        }
-        if (!isStaticIndex)
+        if (!indices[indexPos].is<IntegerAttr>())
           return llvm::make_error<GEPStaticIndexError>(indexPos);
 
+        int32_t gepIndex = indices[indexPos].get<IntegerAttr>().getInt();
         ArrayRef<Type> elementTypes = structType.getBody();
         if (gepIndex < 0 ||
             static_cast<size_t>(gepIndex) >= elementTypes.size())
           return llvm::make_error<GEPIndexOutOfBoundError>(indexPos);
 
-        if (updatedStructIndices)
-          (*updatedStructIndices)[indexPos] = gepIndex;
-
-        // Instead of recusively going into every children types, we only
+        // Instead of recursively going into every children types, we only
         // dive into the one indexed by gepIndex.
-        return recordStructIndices(elementTypes[gepIndex], indexPos + 1,
-                                   structIndices, indices, updatedStructIndices,
-                                   remainingIndices);
+        return verifyStructIndices(elementTypes[gepIndex], indexPos + 1,
+                                   indices);
       })
       .Case<VectorType, LLVMScalableVectorType, LLVMFixedVectorType,
             LLVMArrayType>([&](auto containerType) -> llvm::Error {
-        // Currently we don't refine non-struct index even if it's static.
-        if (remainingIndices)
-          remainingIndices->push_back(indices[dynamicIndexPos]);
-        return recordStructIndices(containerType.getElementType(), indexPos + 1,
-                                   structIndices, indices, updatedStructIndices,
-                                   remainingIndices);
+        return verifyStructIndices(containerType.getElementType(), indexPos + 1,
+                                   indices);
       })
       .Default(
           [](auto otherType) -> llvm::Error { return llvm::Error::success(); });
 }
 
 /// Driver function around `recordStructIndices`. Note that we always check
 /// from the second GEP index since the first one is always dynamic.
-static llvm::Error
-findStructIndices(Type baseGEPType, ArrayRef<int32_t> structIndices,
-                  ValueRange indices,
-                  SmallVectorImpl<int32_t> *updatedStructIndices = nullptr,
-                  SmallVectorImpl<Value> *remainingIndices = nullptr) {
-  if (remainingIndices)
-    // The first GEP index is always dynamic.
-    remainingIndices->push_back(indices[0]);
-  return recordStructIndices(baseGEPType, /*indexPos=*/1, structIndices,
-                             indices, updatedStructIndices, remainingIndices);
-}
-
-void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
-                  Value basePtr, ValueRange operands,
-                  ArrayRef<NamedAttribute> attributes) {
-  build(builder, result, resultType, basePtr, operands,
-        SmallVector<int32_t>(operands.size(), kDynamicIndex), attributes);
-}
-
-/// Returns the elemental type of any LLVM-compatible vector type or self.
-static Type extractVectorElementType(Type type) {
-  if (auto vectorType = type.dyn_cast<VectorType>())
-    return vectorType.getElementType();
-  if (auto scalableVectorType = type.dyn_cast<LLVMScalableVectorType>())
-    return scalableVectorType.getElementType();
-  if (auto fixedVectorType = type.dyn_cast<LLVMFixedVectorType>())
-    return fixedVectorType.getElementType();
-  return type;
-}
-
-void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
-                  Type elementType, Value basePtr, ValueRange indices,
-                  ArrayRef<NamedAttribute> attributes) {
-  build(builder, result, resultType, elementType, basePtr, indices,
-        SmallVector<int32_t>(indices.size(), kDynamicIndex), attributes);
-}
-
-void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
-                  Value basePtr, ValueRange indices,
-                  ArrayRef<int32_t> structIndices,
-                  ArrayRef<NamedAttribute> attributes) {
-  auto ptrType =
-      extractVectorElementType(basePtr.getType()).cast<LLVMPointerType>();
-  assert(!ptrType.isOpaque() &&
-         "expected non-opaque pointer, provide elementType explicitly when "
-         "opaque pointers are used");
-  build(builder, result, resultType, ptrType.getElementType(), basePtr, indices,
-        structIndices, attributes);
-}
-
-void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType,
-                  Type elementType, Value basePtr, ValueRange indices,
-                  ArrayRef<int32_t> structIndices,
-                  ArrayRef<NamedAttribute> attributes) {
-  SmallVector<Value> remainingIndices;
-  SmallVector<int32_t> updatedStructIndices(structIndices.begin(),
-                                            structIndices.end());
-  if (llvm::Error err =
-          findStructIndices(elementType, structIndices, indices,
-                            &updatedStructIndices, &remainingIndices))
-    llvm::report_fatal_error(StringRef(llvm::toString(std::move(err))));
-
-  assert(remainingIndices.size() == static_cast<size_t>(llvm::count(
-                                        updatedStructIndices, kDynamicIndex)) &&
-         "expected as many index operands as dynamic index attr elements");
-
-  result.addTypes(resultType);
-  result.addAttributes(attributes);
-  result.addAttribute("structIndices",
-                      builder.getI32TensorAttr(updatedStructIndices));
-  if (extractVectorElementType(basePtr.getType())
-          .cast<LLVMPointerType>()
-          .isOpaque())
-    result.addAttribute(kElemTypeAttrName, TypeAttr::get(elementType));
-  result.addOperands(basePtr);
-  result.addOperands(remainingIndices);
-}
-
-static ParseResult
-parseGEPIndices(OpAsmParser &parser,
-                SmallVectorImpl<OpAsmParser::UnresolvedOperand> &indices,
-                DenseIntElementsAttr &structIndices) {
-  SmallVector<int32_t> constantIndices;
-
-  auto idxParser = [&]() -> ParseResult {
-    int32_t constantIndex;
-    OptionalParseResult parsedInteger =
-        parser.parseOptionalInteger(constantIndex);
-    if (parsedInteger.hasValue()) {
-      if (failed(parsedInteger.getValue()))
-        return failure();
-      constantIndices.push_back(constantIndex);
-      return success();
-    }
-
-    constantIndices.push_back(LLVM::GEPOp::kDynamicIndex);
-    return parser.parseOperand(indices.emplace_back());
-  };
-  if (parser.parseCommaSeparatedList(idxParser))
-    return failure();
-
-  structIndices = parser.getBuilder().getI32TensorAttr(constantIndices);
-  return success();
-}
-
-static void printGEPIndices(OpAsmPrinter &printer, LLVM::GEPOp gepOp,
-                            OperandRange indices,
-                            DenseIntElementsAttr structIndices) {
-  unsigned operandIdx = 0;
-  llvm::interleaveComma(structIndices.getValues<int32_t>(), printer,
-                        [&](int32_t cst) {
-                          if (cst == LLVM::GEPOp::kDynamicIndex)
-                            printer.printOperand(indices[operandIdx++]);
-                          else
-                            printer << cst;
-                        });
+static llvm::Error verifyStructIndices(Type baseGEPType,
+                                       GEPIndicesAdaptor<ValueRange> indices) {
+  return verifyStructIndices(baseGEPType, /*indexPos=*/1, indices);
 }
 
 LogicalResult LLVM::GEPOp::verify() {
   if (failed(verifyOpaquePtr(
           getOperation(),
           extractVectorElementType(getType()).cast<LLVMPointerType>(),
           getElemType())))
     return failure();
 
-  auto structIndexRange = getStructIndices().getValues<int32_t>();
-  // structIndexRange is a kind of iterator, which cannot be converted
-  // to ArrayRef directly.
-  SmallVector<int32_t> structIndices(structIndexRange.size());
-  for (unsigned i : llvm::seq<unsigned>(0, structIndexRange.size()))
-    structIndices[i] = structIndexRange[i];
-  if (llvm::Error err = findStructIndices(getSourceElementType(), structIndices,
-                                          getIndices()))
+  if (static_cast<size_t>(
+          llvm::count(getRawConstantIndices(), kDynamicIndex)) !=
+      getDynamicIndices().size())
+    return emitOpError("expected as many dynamic indices as specified in '")
+           << getRawConstantIndicesAttrName().getValue() << "'";
+
+  if (llvm::Error err =
+          verifyStructIndices(getSourceElementType(), getIndices()))
     return emitOpError() << llvm::toString(std::move(err));
 
   return success();
 }
 
 Type LLVM::GEPOp::getSourceElementType() {
   if (Optional<Type> elemType = getElemType())
     return *elemType;
OpFoldResult LLVM::AddrSpaceCastOp::fold(ArrayRef<Attribute> operands) {
   return {};
 }
 
 //===----------------------------------------------------------------------===//
 // Folder for LLVM::GEPOp
 //===----------------------------------------------------------------------===//
 
 OpFoldResult LLVM::GEPOp::fold(ArrayRef<Attribute> operands) {
+  GEPIndicesAdaptor<ArrayRef<Attribute>> indices(getRawConstantIndicesAttr(),
+                                                 operands.drop_front());
+
   // gep %x:T, 0 -> %x
-  if (getBase().getType() == getType() && getIndices().size() == 1 &&
-      getStructIndices().size() == 1 && matchPattern(getIndices()[0], m_Zero()))
+  if (getBase().getType() == getType() && indices.size() == 1)
+    if (auto integer = indices[0].dyn_cast_or_null<IntegerAttr>())
+      if (integer.getValue().isZero())
         return getBase();
+
+  // Canonicalize any dynamic indices of constant value to constant indices.
+  bool changed = false;
+  SmallVector<GEPArg> gepArgs;
+  for (auto &iter : llvm::enumerate(indices)) {
+    auto integer = iter.value().dyn_cast_or_null<IntegerAttr>();
+    // Constant indices can only be int32_t, so if integer does not fit we
+    // are forced to keep it dynamic, despite being a constant.
+    if (!indices.isDynamicIndex(iter.index()) || !integer ||
+        !integer.getValue().isSignedIntN(kGEPConstantBitWidth)) {
+
+      PointerUnion<IntegerAttr, Value> existing = getIndices()[iter.index()];
+      if (Value val = existing.dyn_cast<Value>())
+        gepArgs.emplace_back(val);
+      else
+        gepArgs.emplace_back(existing.get<IntegerAttr>().getInt());
+
+      continue;
+    }
+
+    changed = true;
+    gepArgs.emplace_back(integer.getInt());
+  }
+  if (changed) {
+    SmallVector<int32_t> rawConstantIndices;
+    SmallVector<Value> dynamicIndices;
+    destructureIndices(getSourceElementType(), gepArgs, rawConstantIndices,
+                       dynamicIndices);
+
+    getDynamicIndicesMutable().assign(dynamicIndices);
+    setRawConstantIndicesAttr(
+        DenseI32ArrayAttr::get(getContext(), rawConstantIndices));
+    return Value{*this};
+  }
+
   return {};
 }
 
 //===----------------------------------------------------------------------===//
 // LLVMDialect initialization, type parsing, and registration.
 //===----------------------------------------------------------------------===//
 
 void LLVMDialect::initialize() {

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

case llvm::Instruction::Fence: {
     return success();
   }
   case llvm::Instruction::GetElementPtr: {
     // FIXME: Support inbounds GEPs.
     llvm::GetElementPtrInst *gep = cast<llvm::GetElementPtrInst>(inst);
     Value basePtr = processValue(gep->getOperand(0));
     Type sourceElementType = processType(gep->getSourceElementType());
 
-    SmallVector<Value> indices;
-    for (llvm::Value *operand : llvm::drop_begin(gep->operand_values())) {
-      indices.push_back(processValue(operand));
-      if (!indices.back())
-        return failure();
-    }
     // Treat every indices as dynamic since GEPOp::build will refine those
     // indices into static attributes later. One small downside of this
     // approach is that many unused `llvm.mlir.constant` would be emitted
     // at first place.
-    SmallVector<int32_t> structIndices(indices.size(),
-                                       LLVM::GEPOp::kDynamicIndex);
+    SmallVector<GEPArg> indices;
+    for (llvm::Value *operand : llvm::drop_begin(gep->operand_values())) {
+      Value val = processValue(operand);
+      if (!val)
+        return failure();
+      indices.push_back(val);
+    }
 
     Type type = processType(inst->getType());
     if (!type)
       return failure();
-    instMap[inst] = b.create<GEPOp>(loc, type, sourceElementType, basePtr,
-                                    indices, structIndices);
+    instMap[inst] =
+        b.create<GEPOp>(loc, type, sourceElementType, basePtr, indices);
     return success();
   }
   case llvm::Instruction::InsertValue: {
     auto *ivInst = cast<llvm::InsertValueInst>(inst);
     Value inserted = processValue(ivInst->getInsertedValueOperand());
     if (!inserted)
       return failure();
     Value aggOperand = processValue(ivInst->getAggregateOperand());

mlir/test/Dialect/LLVMIR/canonicalize.mlir

 llvm.func @fold_gep(%x : !llvm.ptr<i8>) -> !llvm.ptr<i8> {
   %c0 = arith.constant 0 : i32
   %c = llvm.getelementptr %x[%c0] : (!llvm.ptr<i8>, i32) -> !llvm.ptr<i8>
   llvm.return %c : !llvm.ptr<i8>
 }
 
 // CHECK-LABEL: fold_gep_neg
 // CHECK-SAME: %[[a0:arg[0-9]+]]
-// CHECK-NEXT: %[[C:.*]] = arith.constant 0
-// CHECK-NEXT: %[[RES:.*]] = llvm.getelementptr %[[a0]][%[[C]], 1]
+// CHECK-NEXT: %[[RES:.*]] = llvm.getelementptr %[[a0]][0, 1]
 // CHECK-NEXT: llvm.return %[[RES]]
 llvm.func @fold_gep_neg(%x : !llvm.ptr) -> !llvm.ptr {
   %c0 = arith.constant 0 : i32
   %0 = llvm.getelementptr %x[%c0, 1] : (!llvm.ptr, i32) -> !llvm.ptr, !llvm.struct<(i32, i32)>
   llvm.return %0 : !llvm.ptr
 }
 
+// CHECK-LABEL: fold_gep_canon
+// CHECK-SAME: %[[a0:arg[0-9]+]]
+// CHECK-NEXT: %[[RES:.*]] = llvm.getelementptr %[[a0]][2]
+// CHECK-NEXT: llvm.return %[[RES]]
+llvm.func @fold_gep_canon(%x : !llvm.ptr<i8>) -> !llvm.ptr<i8> {
+  %c2 = arith.constant 2 : i32
+  %c = llvm.getelementptr %x[%c2] : (!llvm.ptr<i8>, i32) -> !llvm.ptr<i8>
+  llvm.return %c : !llvm.ptr<i8>
+}
+
+
 // -----
 
 // Check that LLVM constants participate in cross-dialect constant folding. The
 // resulting constant is created in the arith dialect because the last folded
 // operation belongs to it.
 // CHECK-LABEL: llvm_constant
 func.func @llvm_constant() -> i32 {
   // CHECK-NOT: llvm.mlir.constant

mlir/test/Dialect/LLVMIR/dynamic-gep-index.mlir

 // RUN: mlir-opt %s | FileCheck %s
 
 module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i1, dense<8> : vector<2xi32>>, #dlti.dl_entry<i8, dense<8> : vector<2xi32>>, #dlti.dl_entry<i16, dense<16> : vector<2xi32>>, #dlti.dl_entry<i32, dense<32> : vector<2xi32>>, #dlti.dl_entry<i64, dense<[32, 64]> : vector<2xi32>>, #dlti.dl_entry<f16, dense<16> : vector<2xi32>>, #dlti.dl_entry<f64, dense<64> : vector<2xi32>>, #dlti.dl_entry<f128, dense<128> : vector<2xi32>>>} {
   // CHECK: llvm.func @foo(%[[ARG0:.+]]: !llvm.ptr<struct<"my_struct", {{.+}}>>, %[[ARG1:.+]]: i32)
   llvm.func @foo(%arg0: !llvm.ptr<struct<"my_struct", (struct<"sub_struct", (i32, i8)>, array<4 x i32>)>>, %arg1: i32) {
     // CHECK: %[[C0:.+]] = llvm.mlir.constant(0 : i32)
     %0 = llvm.mlir.constant(0 : i32) : i32
     // CHECK: llvm.getelementptr %[[ARG0]][%[[C0]], 1, %[[ARG1]]]
-    %1 = "llvm.getelementptr"(%arg0, %0, %arg1) {structIndices = dense<[-2147483648, 1, -2147483648]> : tensor<3xi32>} : (!llvm.ptr<struct<"my_struct", (struct<"sub_struct", (i32, i8)>, array<4 x i32>)>>, i32, i32) -> !llvm.ptr<i32>
+    %1 = "llvm.getelementptr"(%arg0, %0, %arg1) {rawConstantIndices = [:i32 -2147483648, 1, -2147483648]} : (!llvm.ptr<struct<"my_struct", (struct<"sub_struct", (i32, i8)>, array<4 x i32>)>>, i32, i32) -> !llvm.ptr<i32>
     llvm.return
   }
 }

mlir/test/Dialect/LLVMIR/invalid.mlir

 
 func.func @gep_non_function_type(%pos : i64, %base : !llvm.ptr<f32>) {
   // expected-error@+1 {{invalid kind of type specified}}
   llvm.getelementptr %base[%pos] : !llvm.ptr<f32>
 }
 
 // -----
 
+func.func @gep_too_few_dynamic(%base : !llvm.ptr<f32>) {
+  // expected-error@+1 {{expected as many dynamic indices as specified in 'rawConstantIndices'}}
+  %1 = "llvm.getelementptr"(%base) {rawConstantIndices = [:i32 -2147483648]} : (!llvm.ptr<f32>) -> !llvm.ptr<f32>
+}
+
+// -----
+
 func.func @load_non_llvm_type(%foo : memref<f32>) {
   // expected-error@+1 {{expected LLVM pointer type}}
   llvm.load %foo : memref<f32>
 }
 
 // -----
 
 func.func @load_non_ptr_type(%foo : f32) {