Skip to content

Commit

Permalink
[mlir][LLVM] Support opaque pointers in llvm.mlir.addressof
Browse files Browse the repository at this point in the history
The verifier of llvm.mlir.addressof did not properly account for opaque pointers, that is, the pointer type not having an element type equal to the type of the referenced global or function. This patch fixes that by skipping the test for the element type if the pointer is opaque.

Differential Revision: https://reviews.llvm.org/D124333
  • Loading branch information
zero9178 committed Apr 25, 2022
1 parent 042dc3c commit 12a2716
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 11 deletions.
4 changes: 2 additions & 2 deletions mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def LLVM_AnyFloat : Type<

// Type constraint accepting any LLVM pointer type.
def LLVM_AnyPointer : Type<CPred<"$_self.isa<::mlir::LLVM::LLVMPointerType>()">,
"LLVM pointer type">;
"LLVM pointer type", "::mlir::LLVM::LLVMPointerType">;

// Type constraint accepting LLVM pointer type with an additional constraint
// on the element type.
Expand All @@ -103,7 +103,7 @@ class LLVM_PointerTo<Type pointee> : Type<
"$_self",
"$_self.cast<::mlir::LLVM::LLVMPointerType>().getElementType()",
pointee.predicate>]>]>,
"LLVM pointer to " # pointee.summary>;
"LLVM pointer to " # pointee.summary, "::mlir::LLVM::LLVMPointerType">;

// Type constraints accepting LLVM pointer type to integer of a specific width.
class LLVM_IntPtrBase<int width, int addressSpace = 0> : Type<
Expand Down
2 changes: 1 addition & 1 deletion mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ def UnnamedAddr : LLVM_EnumAttr<

def LLVM_AddressOfOp : LLVM_Op<"mlir.addressof", [NoSideEffect]> {
let arguments = (ins FlatSymbolRefAttr:$global_name);
let results = (outs LLVM_Type:$res);
let results = (outs LLVM_AnyPointer:$res);

let summary = "Creates a pointer pointing to a global or a function";

Expand Down
15 changes: 10 additions & 5 deletions mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1654,14 +1654,19 @@ LogicalResult AddressOfOp::verify() {
return emitOpError(
"must reference a global defined by 'llvm.mlir.global' or 'llvm.func'");

if (global &&
LLVM::LLVMPointerType::get(global.getType(), global.getAddrSpace()) !=
getResult().getType())
LLVMPointerType type = getType();
if (global && global.getAddrSpace() != type.getAddressSpace())
return emitOpError("pointer address space must match address space of the "
"referenced global");

if (type.isOpaque())
return success();

if (global && type.getElementType() != global.getType())
return emitOpError(
"the type must be a pointer to the type of the referenced global");

if (function && LLVM::LLVMPointerType::get(function.getFunctionType()) !=
getResult().getType())
if (function && type.getElementType() != function.getFunctionType())
return emitOpError(
"the type must be a pointer to the type of the referenced function");

Expand Down
20 changes: 18 additions & 2 deletions mlir/test/Dialect/LLVMIR/global.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ llvm.mlir.global weak_odr @weak_odr() : i64
llvm.mlir.global external @has_thr_local(42 : i64) {thr_local} : i64
// CHECK: llvm.mlir.global external @has_dso_local(42 : i64) {dso_local} : i64
llvm.mlir.global external @has_dso_local(42 : i64) {dso_local} : i64
// CHECK: llvm.mlir.global external @has_addr_space(32 : i64) {addr_space = 3 : i32} : i64
llvm.mlir.global external @has_addr_space(32 : i64) {addr_space = 3: i32} : i64

// CHECK-LABEL: references
func.func @references() {
Expand All @@ -70,6 +72,12 @@ func.func @references() {
// CHECK: llvm.mlir.addressof @".string" : !llvm.ptr<array<6 x i8>>
%1 = llvm.mlir.addressof @".string" : !llvm.ptr<array<6 x i8>>

// CHECK: llvm.mlir.addressof @global : !llvm.ptr
%2 = llvm.mlir.addressof @global : !llvm.ptr

// CHECK: llvm.mlir.addressof @has_addr_space : !llvm.ptr<3>
%3 = llvm.mlir.addressof @has_addr_space : !llvm.ptr<3>

llvm.return
}

Expand Down Expand Up @@ -201,17 +209,25 @@ llvm.mlir.global internal @g(43 : i64) : i64 {

llvm.mlir.global internal @g(32 : i64) {addr_space = 3: i32} : i64
func.func @mismatch_addr_space_implicit_global() {
// expected-error @+1 {{op the type must be a pointer to the type of the referenced global}}
// expected-error @+1 {{pointer address space must match address space of the referenced global}}
llvm.mlir.addressof @g : !llvm.ptr<i64>
}

// -----

llvm.mlir.global internal @g(32 : i64) {addr_space = 3: i32} : i64
func.func @mismatch_addr_space() {
// expected-error @+1 {{op the type must be a pointer to the type of the referenced global}}
// expected-error @+1 {{pointer address space must match address space of the referenced global}}
llvm.mlir.addressof @g : !llvm.ptr<i64, 4>
}
// -----

llvm.mlir.global internal @g(32 : i64) {addr_space = 3: i32} : i64

func.func @mismatch_addr_space_opaque() {
// expected-error @+1 {{pointer address space must match address space of the referenced global}}
llvm.mlir.addressof @g : !llvm.ptr<4>
}

// -----

Expand Down
2 changes: 1 addition & 1 deletion mlir/test/Dialect/LLVMIR/invalid.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ func.func @invalid_vector_type_5(%a : vector<4xf32>, %idx : i32) -> vector<4xf32
// -----

func.func @null_non_llvm_type() {
// expected-error@+1 {{must be LLVM pointer type, but got 'i32'}}
// expected-error@+1 {{custom op 'llvm.mlir.null' invalid kind of type specified}}
llvm.mlir.null : i32
}

Expand Down

0 comments on commit 12a2716

Please sign in to comment.