Skip to content
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] move loadIfRef to FIRBuilder #84306

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Conversation

tblah
Copy link
Contributor

@tblah tblah commented Mar 7, 2024

This will be useful for OpenMP too.

I changed the definition slightly to use fir::isa_ref_type (which also includes llvm pointers) because I think it reads better using the common type helpers. There shouldn't be any llvm pointers in lowering so this isn't a functional change.

Commit series for by-ref openmp reductions: 1/3

This will be useful for OpenMP too.

I changed the definition slightly to use `fir::isa_ref_type` (which also
includes llvm pointers) because I think it reads better using the common
type helpers. There shouldn't be any llvm pointers in lowering so this
isn't a functional change.

Commit series for by-ref openmp reductions: 1/3
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir openacc labels Mar 7, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 7, 2024

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-openacc

Author: Tom Eccles (tblah)

Changes

This will be useful for OpenMP too.

I changed the definition slightly to use fir::isa_ref_type (which also includes llvm pointers) because I think it reads better using the common type helpers. There shouldn't be any llvm pointers in lowering so this isn't a functional change.

Commit series for by-ref openmp reductions: 1/3


Full diff: https://github.com/llvm/llvm-project/pull/84306.diff

3 Files Affected:

  • (modified) flang/include/flang/Optimizer/Builder/FIRBuilder.h (+4)
  • (modified) flang/lib/Lower/OpenACC.cpp (+2-10)
  • (modified) flang/lib/Optimizer/Builder/FIRBuilder.cpp (+7)
diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
index bd9b67b14b966c..d61bf681be6194 100644
--- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -309,6 +309,10 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
   void createStoreWithConvert(mlir::Location loc, mlir::Value val,
                               mlir::Value addr);
 
+  /// Create a fir.load if \p val is a reference or pointer type. Return the
+  /// result of the load if it was created, otherwise return \p val
+  mlir::Value loadIfRef(mlir::Location loc, mlir::Value val);
+
   /// Create a new FuncOp. If the function may have already been created, use
   /// `addNamedFunction` instead.
   mlir::func::FuncOp createFunction(mlir::Location loc, llvm::StringRef name,
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index 151077d81ba14a..d2c6006ecf914a 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -1041,14 +1041,6 @@ static mlir::Value genLogicalCombiner(fir::FirOpBuilder &builder,
   return builder.create<fir::ConvertOp>(loc, value1.getType(), combined);
 }
 
-static mlir::Value loadIfRef(fir::FirOpBuilder &builder, mlir::Location loc,
-                             mlir::Value value) {
-  if (mlir::isa<fir::ReferenceType, fir::PointerType, fir::HeapType>(
-          value.getType()))
-    return builder.create<fir::LoadOp>(loc, value);
-  return value;
-}
-
 static mlir::Value genComparisonCombiner(fir::FirOpBuilder &builder,
                                          mlir::Location loc,
                                          mlir::arith::CmpIPredicate pred,
@@ -1066,8 +1058,8 @@ static mlir::Value genScalarCombiner(fir::FirOpBuilder &builder,
                                      mlir::acc::ReductionOperator op,
                                      mlir::Type ty, mlir::Value value1,
                                      mlir::Value value2) {
-  value1 = loadIfRef(builder, loc, value1);
-  value2 = loadIfRef(builder, loc, value2);
+  value1 = builder.loadIfRef(loc, value1);
+  value2 = builder.loadIfRef(loc, value2);
   if (op == mlir::acc::ReductionOperator::AccAdd) {
     if (ty.isIntOrIndex())
       return builder.create<mlir::arith::AddIOp>(loc, value1, value2);
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index 788c99e40105a2..12da7412888a3b 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -16,6 +16,7 @@
 #include "flang/Optimizer/Builder/Todo.h"
 #include "flang/Optimizer/Dialect/FIRAttr.h"
 #include "flang/Optimizer/Dialect/FIROpsSupport.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/Support/FatalError.h"
 #include "flang/Optimizer/Support/InternalNames.h"
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
@@ -404,6 +405,12 @@ void fir::FirOpBuilder::createStoreWithConvert(mlir::Location loc,
   create<fir::StoreOp>(loc, cast, addr);
 }
 
+mlir::Value fir::FirOpBuilder::loadIfRef(mlir::Location loc, mlir::Value val) {
+  if (fir::isa_ref_type(val.getType()))
+    return create<fir::LoadOp>(loc, val);
+  return val;
+}
+
 fir::StringLitOp fir::FirOpBuilder::createStringLitOp(mlir::Location loc,
                                                       llvm::StringRef data) {
   auto type = fir::CharacterType::get(getContext(), 1, data.size());

@tblah
Copy link
Contributor Author

tblah commented Mar 7, 2024

Next PR in the series: #84305

@tblah tblah merged commit 860a400 into main Mar 8, 2024
8 checks passed
@tblah tblah deleted the users/tblah/omp_byref_1 branch March 8, 2024 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category openacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants