From 009fc4ca3b4476826c89de05cb74d0afe2b5d609 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Markus=20B=C3=B6ck?= Date: Sun, 12 Feb 2023 23:55:47 +0100 Subject: [PATCH] [mlir][LLVM] Verify correct pointer casts with `llvm.bitcast` `llvm.bitcast` has so far not had a verifier and this allowed various bugs to sneak into the codebase (including within tests!) which could only be caught once translated to actual LLVM IR. This patch fixes those problematic cases by now verifying bitcasts on pointers are done correctly. Specifically, it verifies that if pointers are involved, that both result and source types are pointers, that this also applies to vector of pointers and that pointer casts are of the same address space. The only thing left unverified is the general case of "source type size does not match result type size". I think this case is less trivial and more prone to false positives, so I did not yet implement it. Differential Revision: https://reviews.llvm.org/D143868 --- mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 1 + mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 37 ++++++++++++++++++++++++- mlir/test/Dialect/LLVMIR/invalid.mlir | 43 +++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td index d79e511..739f973 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -446,6 +446,7 @@ class LLVM_CastOp { let hasFolder = 1; + let hasVerifier = 1; } def LLVM_AddrSpaceCastOp : LLVM_CastOp<"addrspacecast", "AddrSpaceCast", LLVM_ScalarOrVectorOf, diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index 8d654df7..5de6dac 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -2307,7 +2307,7 @@ LogicalResult FenceOp::verify() { } //===----------------------------------------------------------------------===// -// Folder for LLVM::BitcastOp +// Folder and verifier for LLVM::BitcastOp //===----------------------------------------------------------------------===// OpFoldResult LLVM::BitcastOp::fold(FoldAdaptor adaptor) { @@ -2321,6 +2321,41 @@ OpFoldResult LLVM::BitcastOp::fold(FoldAdaptor adaptor) { return {}; } +LogicalResult LLVM::BitcastOp::verify() { + auto resultType = extractVectorElementType(getResult().getType()) + .dyn_cast(); + auto sourceType = + extractVectorElementType(getArg().getType()).dyn_cast(); + + // If one of the types is a pointer (or vector of pointers), then + // both source and result type have to be pointers. + if (static_cast(resultType) != static_cast(sourceType)) + return emitOpError("can only cast pointers from and to pointers"); + + if (!resultType) + return success(); + + auto isVector = [](Type type) { + return type.isa(); + }; + + // Due to bitcast requiring both operands to be of the same size, it is not + // possible for only one of the two to be a pointer of vectors. + if (isVector(getResult().getType()) && !isVector(getArg().getType())) + return emitOpError("cannot cast pointer to vector of pointers"); + + if (!isVector(getResult().getType()) && isVector(getArg().getType())) + return emitOpError("cannot cast vector of pointers to pointer"); + + // Bitcast cannot cast between pointers of different address spaces. + // 'llvm.addrspacecast' must be used for this purpose instead. + if (resultType.getAddressSpace() != sourceType.getAddressSpace()) + return emitOpError("cannot cast pointers of different address spaces, " + "use 'llvm.addrspacecast' instead"); + + return success(); +} + //===----------------------------------------------------------------------===// // Folder for LLVM::AddrSpaceCastOp //===----------------------------------------------------------------------===// diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir index 4547d13..dccc02f 100644 --- a/mlir/test/Dialect/LLVMIR/invalid.mlir +++ b/mlir/test/Dialect/LLVMIR/invalid.mlir @@ -1292,3 +1292,46 @@ func.func @extract_scalable_from_fixed_length_vector(%arg0 : vector<16xf32>) { #void = #llvm.di_void_result_type // expected-error@below {{expected subroutine to have non-void argument types}} #void_argument_type = #llvm.di_subroutine_type + +// ----- + +func.func @invalid_bitcast_ptr_to_i64(%arg : !llvm.ptr) { + // expected-error@+1 {{can only cast pointers from and to pointers}} + %1 = llvm.bitcast %arg : !llvm.ptr to i64 +} + +// ----- + +func.func @invalid_bitcast_i64_to_ptr() { + %0 = llvm.mlir.constant(2 : i64) : i64 + // expected-error@+1 {{can only cast pointers from and to pointers}} + %1 = llvm.bitcast %0 : i64 to !llvm.ptr +} + +// ----- + +func.func @invalid_bitcast_vec_to_ptr(%arg : !llvm.vec<4 x ptr>) { + // expected-error@+1 {{cannot cast vector of pointers to pointer}} + %0 = llvm.bitcast %arg : !llvm.vec<4 x ptr> to !llvm.ptr +} + +// ----- + +func.func @invalid_bitcast_ptr_to_vec(%arg : !llvm.ptr) { + // expected-error@+1 {{cannot cast pointer to vector of pointers}} + %0 = llvm.bitcast %arg : !llvm.ptr to !llvm.vec<4 x ptr> +} + +// ----- + +func.func @invalid_bitcast_addr_cast(%arg : !llvm.ptr<1>) { + // expected-error@+1 {{cannot cast pointers of different address spaces, use 'llvm.addrspacecast' instead}} + %0 = llvm.bitcast %arg : !llvm.ptr<1> to !llvm.ptr +} + +// ----- + +func.func @invalid_bitcast_addr_cast_vec(%arg : !llvm.vec<4 x ptr<1>>) { + // expected-error@+1 {{cannot cast pointers of different address spaces, use 'llvm.addrspacecast' instead}} + %0 = llvm.bitcast %arg : !llvm.vec<4 x ptr<1>> to !llvm.vec<4 x ptr> +} -- 2.7.4