-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[flang,nfc] Initial changes needed to use llvm intrinsics instead of regular calls #134170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flang,nfc] Initial changes needed to use llvm intrinsics instead of regular calls #134170
Conversation
@llvm/pr-subscribers-flang-fir-hlfir Author: Asher Mancinelli (ashermancinelli) ChangesFlang uses Full diff: https://github.com/llvm/llvm-project/pull/134170.diff 4 Files Affected:
diff --git a/flang/include/flang/Optimizer/Builder/LowLevelIntrinsics.h b/flang/include/flang/Optimizer/Builder/LowLevelIntrinsics.h
index 9be051632f93d..be106f7ea33b7 100644
--- a/flang/include/flang/Optimizer/Builder/LowLevelIntrinsics.h
+++ b/flang/include/flang/Optimizer/Builder/LowLevelIntrinsics.h
@@ -24,9 +24,6 @@ class FirOpBuilder;
namespace fir::factory {
-/// Get the LLVM intrinsic for `memcpy`. Use the 64 bit version.
-mlir::func::FuncOp getLlvmMemcpy(FirOpBuilder &builder);
-
/// Get the LLVM intrinsic for `memmove`. Use the 64 bit version.
mlir::func::FuncOp getLlvmMemmove(FirOpBuilder &builder);
diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp
index b677a136a74aa..2d61c2ee8dd8e 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -6184,17 +6184,16 @@ class ArrayExprLowering {
/// Get the function signature of the LLVM memcpy intrinsic.
mlir::FunctionType memcpyType() {
- return fir::factory::getLlvmMemcpy(builder).getFunctionType();
+ auto ptrTy = mlir::LLVM::LLVMPointerType::get(builder.getContext());
+ llvm::SmallVector<mlir::Type> args = {ptrTy, ptrTy, builder.getI64Type()};
+ return mlir::FunctionType::get(builder.getContext(), args, std::nullopt);
}
/// Create a call to the LLVM memcpy intrinsic.
- void createCallMemcpy(llvm::ArrayRef<mlir::Value> args) {
+ void createCallMemcpy(llvm::ArrayRef<mlir::Value> args, bool isVolatile) {
mlir::Location loc = getLoc();
- mlir::func::FuncOp memcpyFunc = fir::factory::getLlvmMemcpy(builder);
- mlir::SymbolRefAttr funcSymAttr =
- builder.getSymbolRefAttr(memcpyFunc.getName());
- mlir::FunctionType funcTy = memcpyFunc.getFunctionType();
- builder.create<fir::CallOp>(loc, funcSymAttr, funcTy.getResults(), args);
+ builder.create<mlir::LLVM::MemcpyOp>(loc, args[0], args[1], args[2],
+ isVolatile);
}
// Construct code to check for a buffer overrun and realloc the buffer when
@@ -6306,9 +6305,8 @@ class ArrayExprLowering {
auto buff = builder.createConvert(loc, fir::HeapType::get(resTy), mem);
mlir::Value buffi = computeCoordinate(buff, off);
llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments(
- builder, loc, memcpyType(), buffi, v.getAddr(), byteSz,
- /*volatile=*/builder.createBool(loc, false));
- createCallMemcpy(args);
+ builder, loc, memcpyType(), buffi, v.getAddr(), byteSz);
+ createCallMemcpy(args, /*isVolatile=*/false);
// Save the incremented buffer position.
builder.create<fir::StoreOp>(loc, endOff, buffPos);
@@ -6357,9 +6355,8 @@ class ArrayExprLowering {
builder.createConvert(loc, fir::HeapType::get(resTy), mem);
mlir::Value buffi = computeCoordinate(buff, off);
llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments(
- builder, loc, memcpyType(), buffi, v.getAddr(), eleSz,
- /*volatile=*/builder.createBool(loc, false));
- createCallMemcpy(args);
+ builder, loc, memcpyType(), buffi, v.getAddr(), eleSz);
+ createCallMemcpy(args, /*isVolatile=*/false);
builder.create<fir::StoreOp>(loc, plusOne, buffPos);
}
diff --git a/flang/lib/Optimizer/Builder/LowLevelIntrinsics.cpp b/flang/lib/Optimizer/Builder/LowLevelIntrinsics.cpp
index 411a48614af6c..e8547cf2b1e1b 100644
--- a/flang/lib/Optimizer/Builder/LowLevelIntrinsics.cpp
+++ b/flang/lib/Optimizer/Builder/LowLevelIntrinsics.cpp
@@ -21,16 +21,6 @@
#include "flang/Optimizer/Builder/LowLevelIntrinsics.h"
#include "flang/Optimizer/Builder/FIRBuilder.h"
-mlir::func::FuncOp fir::factory::getLlvmMemcpy(fir::FirOpBuilder &builder) {
- auto ptrTy = builder.getRefType(builder.getIntegerType(8));
- llvm::SmallVector<mlir::Type> args = {ptrTy, ptrTy, builder.getI64Type(),
- builder.getI1Type()};
- auto memcpyTy =
- mlir::FunctionType::get(builder.getContext(), args, std::nullopt);
- return builder.createFunction(builder.getUnknownLoc(),
- "llvm.memcpy.p0.p0.i64", memcpyTy);
-}
-
mlir::func::FuncOp fir::factory::getLlvmMemmove(fir::FirOpBuilder &builder) {
auto ptrTy = builder.getRefType(builder.getIntegerType(8));
llvm::SmallVector<mlir::Type> args = {ptrTy, ptrTy, builder.getI64Type(),
diff --git a/flang/test/Lower/array-constructor-2.f90 b/flang/test/Lower/array-constructor-2.f90
index ae75a3b425202..c026c0673fbbd 100644
--- a/flang/test/Lower/array-constructor-2.f90
+++ b/flang/test/Lower/array-constructor-2.f90
@@ -78,12 +78,12 @@ end function test3c
! CHECK-DAG: %[[rep:.*]] = fir.convert %{{.*}} : (!fir.heap<f32>) -> !fir.ref<i8>
! CHECK-DAG: %[[res:.*]] = fir.convert %{{.*}} : (index) -> i64
! CHECK: %{{.*}} = fir.call @realloc(%[[rep]], %[[res]]) {{.*}}: (!fir.ref<i8>, i64) -> !fir.ref<i8>
- ! CHECK: fir.call @llvm.memcpy.p0.p0.i64(%{{.*}}, %{{.*}}, %{{.*}}, %false{{.*}}) {{.*}}: (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()
+ ! CHECK: "llvm.intr.memcpy"(%{{.*}}, %{{.*}}, %{{.*}}) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
! CHECK: fir.call @_QPtest3c
! CHECK: fir.save_result
! CHECK: %[[tmp2:.*]] = fir.allocmem !fir.array<?xf32>, %{{.*}}#1 {uniq_name = ".array.expr"}
! CHECK: fir.call @realloc
- ! CHECK: fir.call @llvm.memcpy.p0.p0.i64(%
+ ! CHECK: "llvm.intr.memcpy"(%{{.*}}, %{{.*}}, %{{.*}}) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
! CHECK: fir.array_coor %[[tmp:.*]](%{{.*}}) %{{.*}} : (!fir.heap<!fir.array<?xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>
! CHECK-NEXT: fir.load
! CHECK-NEXT: fir.array_coor %arg0 %{{.*}} : (!fir.box<!fir.array<?xf32>>, index) -> !fir.ref<f32>
@@ -130,11 +130,11 @@ subroutine test5(a, array2)
! CHECK: %[[res:.*]] = fir.allocmem !fir.array<4xf32>
! CHECK: fir.address_of(@_QQro.2xr4.2) : !fir.ref<!fir.array<2xf32>>
! CHECK: %[[tmp1:.*]] = fir.allocmem !fir.array<2xf32>
- ! CHECK: fir.call @llvm.memcpy.p0.p0.i64(%{{.*}}, %{{.*}}, %{{.*}}, %false{{.*}}) {{.*}}: (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()
+ ! CHECK: "llvm.intr.memcpy"(%{{.*}}, %{{.*}}, %{{.*}}) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
! CHECK: %[[tmp2:.*]] = fir.allocmem !fir.array<2xf32>
! CHECK: = fir.array_coor %[[array2]](%{{.*}}) %{{.*}} : (!fir.ref<!fir.array<2xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>
! CHECK: = fir.array_coor %[[tmp2]](%{{.*}}) %{{.*}} : (!fir.heap<!fir.array<2xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>
- ! CHECK: fir.call @llvm.memcpy.p0.p0.i64(%{{.*}}, %{{.*}}, %{{.*}}, %false{{.*}}) {{.*}}: (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()
+ ! CHECK: "llvm.intr.memcpy"(%{{.*}}, %{{.*}}, %{{.*}}) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
! CHECK: = fir.array_coor %{{.*}}(%{{.*}}) %{{.*}} : (!fir.heap<!fir.array<4xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>
! CHECK: = fir.array_coor %[[a]] %{{.*}} : (!fir.box<!fir.array<?xf32>>, index) -> !fir.ref<f32>
! CHECK-DAG: fir.freemem %{{.*}} : !fir.heap<!fir.array<4xf32>>
@@ -151,12 +151,12 @@ subroutine test6(c, d, e)
! CHECK: = fir.allocmem !fir.array<2x!fir.char<1,5>>
! CHECK: fir.call @realloc
! CHECK: %[[t:.*]] = fir.coordinate_of %{{.*}}, %{{.*}} : (!fir.heap<!fir.array<2x!fir.char<1,5>>>, index) -> !fir.ref<!fir.char<1,5>>
- ! CHECK: %[[to:.*]] = fir.convert %[[t]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<i8>
- ! CHECK: fir.call @llvm.memcpy.p0.p0.i64(%[[to]], %{{.*}}, %{{.*}}, %false) {{.*}}: (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()
+ ! CHECK: %[[to:.*]] = fir.convert %[[t]] : (!fir.ref<!fir.char<1,5>>) -> !llvm.ptr
+ ! CHECK: "llvm.intr.memcpy"(%[[to]], %{{.*}}, %{{.*}}) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
! CHECK: fir.call @realloc
! CHECK: %[[t:.*]] = fir.coordinate_of %{{.*}}, %{{.*}} : (!fir.heap<!fir.array<2x!fir.char<1,5>>>, index) -> !fir.ref<!fir.char<1,5>>
- ! CHECK: %[[to:.*]] = fir.convert %[[t]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<i8>
- ! CHECK: fir.call @llvm.memcpy.p0.p0.i64(%[[to]], %{{.*}}, %{{.*}}, %false) {{.*}}: (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()
+ ! CHECK: %[[to:.*]] = fir.convert %[[t]] : (!fir.ref<!fir.char<1,5>>) -> !llvm.ptr
+ ! CHECK: "llvm.intr.memcpy"(%[[to]], %{{.*}}, %{{.*}}) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
! CHECK: fir.freemem %{{.*}} : !fir.heap<!fir.array<2x!fir.char<1,5>>>
c = (/ d, e /)
end subroutine test6
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. That makes sense to use the op where we have one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this improvement. I just have one question about types :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Follow up to #134170. We should be using the LLVM intrinsics instead of plain fir.calls when we can. Existing code creates a declaration for the llvm intrinsic and a regular fir.call, which makes it hard for consumers of the IR to find all the intrinsic calls.
Flang uses
fir.call <llvm intrinsic>
in a few places. This means consumers of the IR need to strcmp every fir.call if they want to find a particular LLVM intrinsic, which does not seem ideal given we have intrinsics available to us. I'd like to do the same for the rest of the functions used in this source file if folks are ok with this.