[mlir][llvm] Fix elem type passing into getelementptr (#68136)

As was correctly pointed out by @azteca1998, the element type for a
`llvm.getelementptr` was only read when using an attribute and not when
using a type. As pointed out in
#63832 (comment),
the translation to LLVM would work for
```mlir
llvm.func @main(%0 : !llvm.ptr) -> !llvm.ptr {
    %1 = llvm.getelementptr %0[0] { elem_type = !llvm.ptr } : (!llvm.ptr) -> !llvm.ptr
    llvm.return %1 : !llvm.ptr
}
```
but not for
```mlir
llvm.func @main(%0 : !llvm.ptr) -> !llvm.ptr<ptr> {
    %1 = llvm.getelementptr %0[0] : (!llvm.ptr) -> !llvm.ptr<ptr>
    llvm.return %1 : !llvm.ptr<ptr>
}
```
This was caused by the `LLVM_GEPOp` builder only reading the type from
the attribute (`{ elem_type = !llvm.ptr }`), but not from the pointer
type (`!llvm.ptr<ptr>`).

Fixes #63832.

EDIT: During review Markus Böck pointed out that this bugfix adds new functionality for typed pointers, but this functionality shouldn't be there in the first place. In response, Oleksandr "Alex" Zinenko pointed out that this is okay for now since the typed pointers will be removed in an upcoming release anyway, so it's best to merge this PR and spend time on the removal instead.

Author: rikhuijzer

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

@@ -303,8 +303,13 @@ def LLVM_GEPOp : LLVM_Op<"getelementptr", [Pure,
         indices.push_back(
             builder.getInt32(valueOrAttr.get<IntegerAttr>().getInt()));
     }
-    Type baseElementType = op.getSourceElementType();
-    llvm::Type *elementType = moduleTranslation.convertType(baseElementType);
+
+    Type elemTypeFromAttr = op.getSourceElementType();
+    auto ptrType = ::llvm::cast<LLVMPointerType>(op.getType());
+    Type elemTypeFromPtrType = ptrType.getElementType();
+
+    llvm::Type *elementType = moduleTranslation.convertType(
+        elemTypeFromAttr ? elemTypeFromAttr : elemTypeFromPtrType);
     $res = builder.CreateGEP(elementType, $base, indices, "", $inbounds);
   }];
   let assemblyFormat = [{

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

@@ -287,14 +287,17 @@ ParseResult AllocaOp::parse(OpAsmParser &parser, OperationState &result) {
 }
 
 /// Checks that the elemental type is present in either the pointer type or
-/// the attribute, but not both.
+/// the attribute, but not in none or both.
 static LogicalResult verifyOpaquePtr(Operation *op, LLVMPointerType ptrType,
                                      std::optional<Type> ptrElementType) {
-  if (ptrType.isOpaque() && !ptrElementType.has_value()) {
+  bool typePresentInPointerType = !ptrType.isOpaque();
+  bool typePresentInAttribute = ptrElementType.has_value();
+
+  if (!typePresentInPointerType && !typePresentInAttribute) {
     return op->emitOpError() << "expected '" << kElemTypeAttrName
                              << "' attribute if opaque pointer type is used";
   }
-  if (!ptrType.isOpaque() && ptrElementType.has_value()) {
+  if (typePresentInPointerType && typePresentInAttribute) {
     return op->emitOpError()
            << "unexpected '" << kElemTypeAttrName
            << "' attribute when non-opaque pointer type is used";

mlir/test/Target/LLVMIR/opaque-ptr.mlir

@@ -42,6 +42,15 @@ llvm.func @opaque_ptr_gep_struct(%arg0: !llvm.ptr, %arg1: i32) -> !llvm.ptr {
   llvm.return %0 : !llvm.ptr
 }
 
+// CHECK-LABEL: @opaque_ptr_elem_type
+llvm.func @opaque_ptr_elem_type(%0: !llvm.ptr) -> !llvm.ptr {
+  // CHECK: getelementptr ptr, ptr
+  %1 = llvm.getelementptr %0[0] { elem_type = !llvm.ptr } : (!llvm.ptr) -> !llvm.ptr
+  // CHECK: getelementptr ptr, ptr
+  %2 = llvm.getelementptr %0[0] : (!llvm.ptr) -> !llvm.ptr<ptr>
+  llvm.return %1 : !llvm.ptr
+}
+
 // CHECK-LABEL: @opaque_ptr_matrix_load_store
 llvm.func @opaque_ptr_matrix_load_store(%ptr: !llvm.ptr, %stride: i64) -> vector<48 x f32> {
   // CHECK: call <48 x float> @llvm.matrix.column.major.load.v48f32.i64