[Mlir-commits] [mlir] 009fc4c - [mlir][LLVM] Verify correct pointer casts with `llvm.bitcast`

[Markus Böck]

Author: Markus Böck
Date: 2023-02-13T22:24:20+01:00
New Revision: 009fc4ca3b4476826c89de05cb74d0afe2b5d609

URL: https://github.com/llvm/llvm-project/commit/009fc4ca3b4476826c89de05cb74d0afe2b5d609
DIFF: https://github.com/llvm/llvm-project/commit/009fc4ca3b4476826c89de05cb74d0afe2b5d609.diff

LOG: [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

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
    mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
    mlir/test/Dialect/LLVMIR/invalid.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index d79e511b210d5..739f973778ec6 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -446,6 +446,7 @@ class LLVM_CastOp<string mnemonic, string instName, Type type,
 def LLVM_BitcastOp : LLVM_CastOp<"bitcast", "BitCast",
                                  LLVM_AnyNonAggregate, LLVM_AnyNonAggregate> {
   let hasFolder = 1;
+  let hasVerifier = 1;
 }
 def LLVM_AddrSpaceCastOp : LLVM_CastOp<"addrspacecast", "AddrSpaceCast",
                                        LLVM_ScalarOrVectorOf<LLVM_AnyPointer>,

diff  --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 8d654df7d2179..5de6dac18f51c 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<LLVMPointerType>();
+  auto sourceType =
+      extractVectorElementType(getArg().getType()).dyn_cast<LLVMPointerType>();
+
+  // 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<bool>(resultType) != static_cast<bool>(sourceType))
+    return emitOpError("can only cast pointers from and to pointers");
+
+  if (!resultType)
+    return success();
+
+  auto isVector = [](Type type) {
+    return type.isa<VectorType, LLVMScalableVectorType, LLVMFixedVectorType>();
+  };
+
+  // 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 
diff erent address spaces.
+  // 'llvm.addrspacecast' must be used for this purpose instead.
+  if (resultType.getAddressSpace() != sourceType.getAddressSpace())
+    return emitOpError("cannot cast pointers of 
diff erent 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 4547d13dc38b3..dccc02fcfcf7a 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 at below {{expected subroutine to have non-void argument types}}
 #void_argument_type = #llvm.di_subroutine_type<types = #void, #void>
+
+// -----
+
+func.func @invalid_bitcast_ptr_to_i64(%arg : !llvm.ptr) {
+  // expected-error at +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 at +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 at +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 at +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 at +1 {{cannot cast pointers of 
diff erent 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 at +1 {{cannot cast pointers of 
diff erent address spaces, use 'llvm.addrspacecast' instead}}
+  %0 = llvm.bitcast %arg : !llvm.vec<4 x ptr<1>> to !llvm.vec<4 x ptr>
+}