From 7b420a1ada0775b35c6600f58ac4d5775106a723 Mon Sep 17 00:00:00 2001 From: Tobias Gysi Date: Wed, 14 Dec 2022 10:44:11 +0100 Subject: [PATCH] [mlir][llvm] Add inbounds attriubte to the gep op. The revision adds an inbounds attribute to the LLVM dialect GEP operation. It extends the builders and the import and export to support the optional inbounds attribute. As all builders set inbounds to false by default, existing lowerings from higher-level dialects to LLVM dialect are not affected by the change. Canonicalization/folding remains untouched since it currently does not implement any simplifications in case of undefined behavior (the handling of undefined behavior is deferred to LLVM). Reviewed By: ftynse Differential Revision: https://reviews.llvm.org/D139821 --- mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 23 +++++++++++++++-------- mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 18 +++++++++++------- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp | 7 +++---- mlir/test/Dialect/LLVMIR/canonicalize.mlir | 4 ++-- mlir/test/Dialect/LLVMIR/roundtrip.mlir | 4 ++-- mlir/test/Target/LLVMIR/Import/instructions.ll | 4 ++-- mlir/test/Target/LLVMIR/Import/opaque.ll | 4 ++-- mlir/test/Target/LLVMIR/llvmir.mlir | 4 ++-- 8 files changed, 39 insertions(+), 29 deletions(-) diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td index 6e63a51..4abd193 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -257,7 +257,8 @@ def LLVM_GEPOp : LLVM_Op<"getelementptr", [Pure]> { let arguments = (ins LLVM_ScalarOrVectorOf:$base, Variadic>:$dynamicIndices, DenseI32ArrayAttr:$rawConstantIndices, - OptionalAttr:$elem_type); + OptionalAttr:$elem_type, + UnitAttr:$inbounds); let results = (outs LLVM_ScalarOrVectorOf:$res); let skipDefaultBuilders = 1; @@ -269,14 +270,17 @@ def LLVM_GEPOp : LLVM_Op<"getelementptr", [Pure]> { 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. + An optional 'inbounds' attribute specifies the low-level pointer arithmetic + overflow behavior that LLVM uses after lowering the operation to LLVM IR. + Examples: ```mlir // GEP with an SSA value offset %0 = llvm.getelementptr %1[%2] : (!llvm.ptr, i64) -> !llvm.ptr - // GEP with a constant offset - %0 = llvm.getelementptr %1[3] : (!llvm.ptr) -> !llvm.ptr + // GEP with a constant offset and the inbounds attribute set + %0 = llvm.getelementptr inbounds %1[3] : (!llvm.ptr) -> !llvm.ptr // GEP with constant offsets into a structure %0 = llvm.getelementptr %1[0, 1] @@ -286,14 +290,16 @@ def LLVM_GEPOp : LLVM_Op<"getelementptr", [Pure]> { let builders = [ OpBuilder<(ins "Type":$resultType, "Type":$basePtrType, "Value":$basePtr, - "ValueRange":$indices, + "ValueRange":$indices, CArg<"bool", "false">:$inbounds, CArg<"ArrayRef", "{}">:$attributes)>, - OpBuilder<(ins "Type":$resultType, "Value":$basePtr, "ValueRange":$indices, + OpBuilder<(ins "Type":$resultType, "Value":$basePtr, + "ValueRange":$indices, CArg<"bool", "false">:$inbounds, CArg<"ArrayRef", "{}">:$attributes)>, - OpBuilder<(ins "Type":$resultType, "Value":$basePtr, "ArrayRef":$indices, + OpBuilder<(ins "Type":$resultType, "Value":$basePtr, + "ArrayRef":$indices, CArg<"bool", "false">:$inbounds, CArg<"ArrayRef", "{}">:$attributes)>, OpBuilder<(ins "Type":$resultType, "Type":$basePtrType, "Value":$basePtr, - "ArrayRef":$indices, + "ArrayRef":$indices, CArg<"bool", "false">:$inbounds, CArg<"ArrayRef", "{}">:$attributes)>, ]; let llvmBuilder = [{ @@ -310,9 +316,10 @@ def LLVM_GEPOp : LLVM_Op<"getelementptr", [Pure]> { } Type baseElementType = op.getSourceElementType(); llvm::Type *elementType = moduleTranslation.convertType(baseElementType); - $res = builder.CreateGEP(elementType, $base, indices); + $res = builder.CreateGEP(elementType, $base, indices, "", $inbounds); }]; let assemblyFormat = [{ + (`inbounds` $inbounds^)? $base `[` custom($dynamicIndices, $rawConstantIndices) `]` attr-dict `:` functional-type(operands, results) (`,` $elem_type^)? }]; diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp index 1afd98f..0694f9b 100644 --- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp +++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp @@ -418,7 +418,7 @@ static Type extractVectorElementType(Type type) { } void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType, - Value basePtr, ArrayRef indices, + Value basePtr, ArrayRef indices, bool inbounds, ArrayRef attributes) { auto ptrType = extractVectorElementType(basePtr.getType()).cast(); @@ -426,7 +426,7 @@ void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType, "expected non-opaque pointer, provide elementType explicitly when " "opaque pointers are used"); build(builder, result, resultType, ptrType.getElementType(), basePtr, indices, - attributes); + inbounds, attributes); } /// Destructures the 'indices' parameter into 'rawConstantIndices' and @@ -481,7 +481,7 @@ static void destructureIndices(Type currType, ArrayRef indices, void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType, Type elementType, Value basePtr, ArrayRef indices, - ArrayRef attributes) { + bool inbounds, ArrayRef attributes) { SmallVector rawConstantIndices; SmallVector dynamicIndices; destructureIndices(elementType, indices, rawConstantIndices, dynamicIndices); @@ -490,6 +490,10 @@ void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType, result.addAttributes(attributes); result.addAttribute(getRawConstantIndicesAttrName(result.name), builder.getDenseI32ArrayAttr(rawConstantIndices)); + if (inbounds) { + result.addAttribute(getInboundsAttrName(result.name), + builder.getUnitAttr()); + } if (extractVectorElementType(basePtr.getType()) .cast() .isOpaque()) @@ -499,17 +503,17 @@ void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType, } void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType, - Value basePtr, ValueRange indices, + Value basePtr, ValueRange indices, bool inbounds, ArrayRef attributes) { build(builder, result, resultType, basePtr, SmallVector(indices), - attributes); + inbounds, attributes); } void GEPOp::build(OpBuilder &builder, OperationState &result, Type resultType, Type elementType, Value basePtr, ValueRange indices, - ArrayRef attributes) { + bool inbounds, ArrayRef attributes) { build(builder, result, resultType, elementType, basePtr, - SmallVector(indices), attributes); + SmallVector(indices), inbounds, attributes); } static ParseResult diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp index ec9bde2..12d6238 100644 --- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp +++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp @@ -1085,7 +1085,6 @@ LogicalResult Importer::convertOperation(OpBuilder &odsBuilder, return success(); } if (inst->getOpcode() == llvm::Instruction::GetElementPtr) { - // FIXME: Support inbounds GEPs. auto *gepInst = cast(inst); Type sourceElementType = convertType(gepInst->getSourceElementType()); FailureOr basePtr = convertValue(gepInst->getOperand(0)); @@ -1105,8 +1104,9 @@ LogicalResult Importer::convertOperation(OpBuilder &odsBuilder, } Type type = convertType(inst->getType()); - Value res = builder.create(loc, type, sourceElementType, - basePtr.value(), indices); + Value res = + builder.create(loc, type, sourceElementType, basePtr.value(), + indices, gepInst->isInBounds()); mapValue(inst, res); return success(); } @@ -1116,7 +1116,6 @@ LogicalResult Importer::convertOperation(OpBuilder &odsBuilder, LogicalResult Importer::processInstruction(llvm::Instruction *inst) { // FIXME: Support uses of SubtargetData. - // FIXME: Add support for inbounds GEPs. // FIXME: Add support for fast-math flags and call / operand attributes. // FIXME: Add support for the indirectbr, cleanupret, catchret, catchswitch, // callbr, vaarg, landingpad, catchpad, cleanuppad instructions. diff --git a/mlir/test/Dialect/LLVMIR/canonicalize.mlir b/mlir/test/Dialect/LLVMIR/canonicalize.mlir index 570226c..9432eda 100644 --- a/mlir/test/Dialect/LLVMIR/canonicalize.mlir +++ b/mlir/test/Dialect/LLVMIR/canonicalize.mlir @@ -102,11 +102,11 @@ llvm.func @fold_gep(%x : !llvm.ptr) -> !llvm.ptr { // CHECK-LABEL: fold_gep_neg // CHECK-SAME: %[[a0:arg[0-9]+]] -// CHECK-NEXT: %[[RES:.*]] = llvm.getelementptr %[[a0]][0, 1] +// CHECK-NEXT: %[[RES:.*]] = llvm.getelementptr inbounds %[[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)> + %0 = llvm.getelementptr inbounds %x[%c0, 1] : (!llvm.ptr, i32) -> !llvm.ptr, !llvm.struct<(i32, i32)> llvm.return %0 : !llvm.ptr } diff --git a/mlir/test/Dialect/LLVMIR/roundtrip.mlir b/mlir/test/Dialect/LLVMIR/roundtrip.mlir index 884a53d..515215b 100644 --- a/mlir/test/Dialect/LLVMIR/roundtrip.mlir +++ b/mlir/test/Dialect/LLVMIR/roundtrip.mlir @@ -184,8 +184,8 @@ llvm.func @gep(%ptr: !llvm.ptr)>>, %idx: i64, %ptr2: !llvm.ptr)>>) { // CHECK: llvm.getelementptr %{{.*}}[%{{.*}}, 1, 0] : (!llvm.ptr)>>, i64) -> !llvm.ptr llvm.getelementptr %ptr[%idx, 1, 0] : (!llvm.ptr)>>, i64) -> !llvm.ptr - // CHECK: llvm.getelementptr %{{.*}}[%{{.*}}, 0, %{{.*}}] : (!llvm.ptr)>>, i64, i64) -> !llvm.ptr - llvm.getelementptr %ptr2[%idx, 0, %idx] : (!llvm.ptr)>>, i64, i64) -> !llvm.ptr + // CHECK: llvm.getelementptr inbounds %{{.*}}[%{{.*}}, 0, %{{.*}}] : (!llvm.ptr)>>, i64, i64) -> !llvm.ptr + llvm.getelementptr inbounds %ptr2[%idx, 0, %idx] : (!llvm.ptr)>>, i64, i64) -> !llvm.ptr llvm.return } diff --git a/mlir/test/Target/LLVMIR/Import/instructions.ll b/mlir/test/Target/LLVMIR/Import/instructions.ll index ad4a981..bd44c28 100644 --- a/mlir/test/Target/LLVMIR/Import/instructions.ll +++ b/mlir/test/Target/LLVMIR/Import/instructions.ll @@ -440,8 +440,8 @@ define void @call_fn_ptr(void (i16) *%fn) { ; CHECK-SAME: %[[PTR:[a-zA-Z0-9]+]] define void @gep_static_idx(float* %ptr) { ; CHECK: %[[IDX:.+]] = llvm.mlir.constant(7 : i32) - ; CHECK: llvm.getelementptr %[[PTR]][%[[IDX]]] : (!llvm.ptr, i32) -> !llvm.ptr - %1 = getelementptr float, float* %ptr, i32 7 + ; CHECK: llvm.getelementptr inbounds %[[PTR]][%[[IDX]]] : (!llvm.ptr, i32) -> !llvm.ptr + %1 = getelementptr inbounds float, float* %ptr, i32 7 ret void } diff --git a/mlir/test/Target/LLVMIR/Import/opaque.ll b/mlir/test/Target/LLVMIR/Import/opaque.ll index f732697..2d20b2c 100644 --- a/mlir/test/Target/LLVMIR/Import/opaque.ll +++ b/mlir/test/Target/LLVMIR/Import/opaque.ll @@ -38,8 +38,8 @@ define ptr @opaque_ptr_alloca(i32 %0) { ; CHECK-LABEL: @opaque_ptr_gep define ptr @opaque_ptr_gep(ptr %0, i32 %1) { - ; CHECK: = llvm.getelementptr %{{.*}}[%{{.*}}] : (!llvm.ptr, i32) -> !llvm.ptr, f32 - %3 = getelementptr float, ptr %0, i32 %1 + ; CHECK: = llvm.getelementptr inbounds %{{.*}}[%{{.*}}] : (!llvm.ptr, i32) -> !llvm.ptr, f32 + %3 = getelementptr inbounds float, ptr %0, i32 %1 ret ptr %3 } diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir index c767d8e..fd2bd27 100644 --- a/mlir/test/Target/LLVMIR/llvmir.mlir +++ b/mlir/test/Target/LLVMIR/llvmir.mlir @@ -996,8 +996,8 @@ llvm.func @gep(%ptr: !llvm.ptr)>>, %idx: i64, %ptr2: !llvm.ptr)>>) { // CHECK: = getelementptr { i32, { i32, float } }, ptr %{{.*}}, i64 %{{.*}}, i32 1, i32 0 llvm.getelementptr %ptr[%idx, 1, 0] : (!llvm.ptr)>>, i64) -> !llvm.ptr - // CHECK: = getelementptr { [10 x float] }, ptr %{{.*}}, i64 %{{.*}}, i32 0, i64 %{{.*}} - llvm.getelementptr %ptr2[%idx, 0, %idx] : (!llvm.ptr)>>, i64, i64) -> !llvm.ptr + // CHECK: = getelementptr inbounds { [10 x float] }, ptr %{{.*}}, i64 %{{.*}}, i32 0, i64 %{{.*}} + llvm.getelementptr inbounds %ptr2[%idx, 0, %idx] : (!llvm.ptr)>>, i64, i64) -> !llvm.ptr llvm.return } -- 2.7.4