[Mlir-commits] [mlir] [mlir][acc] Support lazy remark message construction (PR #180665)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon Feb 9 17:59:52 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-openacc
Author: Razvan Lupusoru (razvanlupusoru)
<details>
<summary>Changes</summary>
The OpenACC remark emission utilities previously only accepted Twine for message construction. However, complex remarks often require additional logic to build messages, such as resolving variable names. This results in unnecessary work when remarks are disabled.
Add an overload that accepts a lambda for message generation, which is only invoked when remark emission is enabled. Update ACCLoopTiling to use this lazy API for tile size reporting.
Additionally, getVariableName now returns numeric strings for constant integer values. This is also being used by ACCLoopTiling along with the lazy remark update.
---
Full diff: https://github.com/llvm/llvm-project/pull/180665.diff
6 Files Affected:
- (modified) mlir/include/mlir/Dialect/OpenACC/Analysis/OpenACCSupport.h (+30-9)
- (modified) mlir/include/mlir/Dialect/OpenACC/OpenACCUtils.h (+22-3)
- (modified) mlir/lib/Dialect/OpenACC/Analysis/OpenACCSupport.cpp (+4-3)
- (modified) mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp (+27-18)
- (modified) mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp (+8-2)
- (modified) mlir/unittests/Dialect/OpenACC/OpenACCUtilsTest.cpp (+24)
``````````diff
diff --git a/mlir/include/mlir/Dialect/OpenACC/Analysis/OpenACCSupport.h b/mlir/include/mlir/Dialect/OpenACC/Analysis/OpenACCSupport.h
index c49452556f0c4..6cceb704bca4e 100644
--- a/mlir/include/mlir/Dialect/OpenACC/Analysis/OpenACCSupport.h
+++ b/mlir/include/mlir/Dialect/OpenACC/Analysis/OpenACCSupport.h
@@ -55,6 +55,7 @@
#include "mlir/IR/Value.h"
#include "mlir/Pass/AnalysisManager.h"
#include "llvm/ADT/StringRef.h"
+#include <functional>
#include <memory>
#include <string>
@@ -85,7 +86,7 @@ struct OpenACCSupportTraits {
// emitted. When not provided, in the default implementation, the category
// is "openacc".
virtual remark::detail::InFlightRemark
- emitRemark(Operation *op, const Twine &message,
+ emitRemark(Operation *op, std::function<std::string()> messageFn,
llvm::StringRef category) = 0;
/// Check if a symbol use is valid for use in an OpenACC region.
@@ -120,8 +121,9 @@ struct OpenACCSupportTraits {
decltype(std::declval<ImplT>().emitRemark(std::declval<Args>()...));
template <typename ImplT>
- using has_emitRemark = llvm::is_detected<emitRemark_t, ImplT, Operation *,
- const Twine &, llvm::StringRef>;
+ using has_emitRemark =
+ llvm::is_detected<emitRemark_t, ImplT, Operation *,
+ std::function<std::string()>, llvm::StringRef>;
/// This class wraps a concrete OpenACCSupport implementation and forwards
/// interface calls to it. This provides type erasure, allowing different
@@ -146,13 +148,13 @@ struct OpenACCSupportTraits {
return impl.emitNYI(loc, message);
}
- remark::detail::InFlightRemark emitRemark(Operation *op,
- const Twine &message,
- llvm::StringRef category) final {
+ remark::detail::InFlightRemark
+ emitRemark(Operation *op, std::function<std::string()> messageFn,
+ llvm::StringRef category) final {
if constexpr (has_emitRemark<ImplT>::value)
- return impl.emitRemark(op, message, category);
+ return impl.emitRemark(op, std::move(messageFn), category);
else
- return acc::emitRemark(op, message, category);
+ return acc::emitRemark(op, messageFn(), category);
}
bool isValidSymbolUse(Operation *user, SymbolRefAttr symbol,
@@ -222,6 +224,21 @@ class OpenACCSupport {
/// unsupported case.
InFlightDiagnostic emitNYI(Location loc, const Twine &message);
+ /// Emit an OpenACC remark with lazy message generation.
+ ///
+ /// The messageFn is only invoked if remarks are enabled for the given
+ /// operation, allowing callers to avoid constructing expensive messages
+ /// when remarks are disabled.
+ ///
+ /// \param op The operation to emit the remark for.
+ /// \param messageFn A callable that returns the remark message.
+ /// \param category Optional category for the remark. Defaults to "openacc".
+ /// \return An in-flight remark object that can be used to append
+ /// additional information to the remark.
+ remark::detail::InFlightRemark
+ emitRemark(Operation *op, std::function<std::string()> messageFn,
+ llvm::StringRef category = "openacc");
+
/// Emit an OpenACC remark.
///
/// \param op The operation to emit the remark for.
@@ -231,7 +248,11 @@ class OpenACCSupport {
/// additional information to the remark.
remark::detail::InFlightRemark
emitRemark(Operation *op, const Twine &message,
- llvm::StringRef category = "openacc");
+ llvm::StringRef category = "openacc") {
+ return emitRemark(
+ op, std::function<std::string()>([msg = message.str()]() { return msg; }),
+ category);
+ }
/// Check if a symbol use is valid for use in an OpenACC region.
///
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCUtils.h b/mlir/include/mlir/Dialect/OpenACC/OpenACCUtils.h
index a436050babce2..aac3bf7ed67c8 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCUtils.h
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCUtils.h
@@ -13,6 +13,7 @@
#include "mlir/IR/Remarks.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include <functional>
namespace mlir {
class DominanceInfo;
@@ -100,6 +101,20 @@ getDominatingDataClauses(mlir::Operation *computeConstructOp,
mlir::DominanceInfo &domInfo,
mlir::PostDominanceInfo &postDomInfo);
+/// Emit an OpenACC remark with lazy message generation.
+///
+/// The messageFn is only invoked if remarks are enabled, allowing callers
+/// to avoid constructing expensive messages when remarks are disabled.
+///
+/// \param op The operation to emit the remark for.
+/// \param messageFn A callable that returns the remark message.
+/// \param category Optional category for the remark. Defaults to "openacc".
+/// \return An in-flight remark object that can be used to append
+/// additional information to the remark.
+remark::detail::InFlightRemark
+emitRemark(mlir::Operation *op, const std::function<std::string()> &messageFn,
+ llvm::StringRef category = "openacc");
+
/// Emit an OpenACC remark for the given operation with the given message.
///
/// \param op The operation to emit the remark for.
@@ -107,9 +122,13 @@ getDominatingDataClauses(mlir::Operation *computeConstructOp,
/// \param category Optional category for the remark. Defaults to "openacc".
/// \return An in-flight remark object that can be used to append
/// additional information to the remark.
-remark::detail::InFlightRemark emitRemark(mlir::Operation *op,
- const llvm::Twine &message,
- llvm::StringRef category = "openacc");
+inline remark::detail::InFlightRemark
+emitRemark(mlir::Operation *op, const llvm::Twine &message,
+ llvm::StringRef category = "openacc") {
+ return emitRemark(
+ op, std::function<std::string()>([msg = message.str()]() { return msg; }),
+ category);
+}
} // namespace acc
} // namespace mlir
diff --git a/mlir/lib/Dialect/OpenACC/Analysis/OpenACCSupport.cpp b/mlir/lib/Dialect/OpenACC/Analysis/OpenACCSupport.cpp
index 4fbe3e06c2532..f204ace4609ab 100644
--- a/mlir/lib/Dialect/OpenACC/Analysis/OpenACCSupport.cpp
+++ b/mlir/lib/Dialect/OpenACC/Analysis/OpenACCSupport.cpp
@@ -42,11 +42,12 @@ InFlightDiagnostic OpenACCSupport::emitNYI(Location loc, const Twine &message) {
}
remark::detail::InFlightRemark
-OpenACCSupport::emitRemark(Operation *op, const Twine &message,
+OpenACCSupport::emitRemark(Operation *op,
+ std::function<std::string()> messageFn,
llvm::StringRef category) {
if (impl)
- return impl->emitRemark(op, message, category);
- return acc::emitRemark(op, message, category);
+ return impl->emitRemark(op, std::move(messageFn), category);
+ return acc::emitRemark(op, messageFn(), category);
}
bool OpenACCSupport::isValidSymbolUse(Operation *user, SymbolRefAttr symbol,
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp
index 23bec6a295061..495d0247d86d3 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp
@@ -63,6 +63,7 @@
#include "mlir/IR/PatternMatch.h"
#include "mlir/Support/LLVM.h"
#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Debug.h"
namespace mlir {
@@ -117,30 +118,38 @@ struct ACCLoopTilingImpl : public OpRewritePattern<acc::LoopOp> {
void emitTilingRemarks(acc::LoopOp loop, ArrayRef<Value> tileSizes) const {
// Emit remarks for loop tiling
- size_t tileLevel = tileSizes.size();
- std::string msg =
- "Tiling " + std::to_string(tileLevel) + "-level loop nest with tile(";
- for (size_t i = 0; i < tileSizes.size(); ++i) {
- std::optional<int64_t> val = getConstantIntValue(tileSizes[i]);
- if (*val == -1)
- msg += "*";
- else
- msg += std::to_string(*val);
- if (i < tileSizes.size() - 1)
- msg += ",";
- }
- msg += ")";
- accSupport.emitRemark(loop, llvm::Twine(msg), DEBUG_TYPE);
+ accSupport.emitRemark(
+ loop,
+ [&]() {
+ auto getTileSizeStr = [&](Value v) -> std::string {
+ std::string name = accSupport.getVariableName(v);
+ // Use "*" for unknown tile sizes (represented as -1 or empty)
+ if (name.empty() || name == "-1")
+ return "*";
+ return name;
+ };
+ SmallVector<std::string> tileStrs;
+ for (Value v : tileSizes)
+ tileStrs.push_back(getTileSizeStr(v));
+ return "Tiling " + std::to_string(tileSizes.size()) +
+ "-level loop nest with tile(" + llvm::join(tileStrs, ",") +
+ ")";
+ },
+ DEBUG_TYPE);
// Emit remarks for unknown tile sizes that will be resolved to default
// TODO: Need to base the default tile size on some heuristics.
for (Value tileSize : tileSizes) {
std::optional<int64_t> val = getConstantIntValue(tileSize);
if (val && *val < 0) {
- std::string unknownMsg = "Picking default tile size " +
- std::to_string(defaultTileSize) +
- " for unknown tile size '*'";
- accSupport.emitRemark(loop, llvm::Twine(unknownMsg), DEBUG_TYPE);
+ accSupport.emitRemark(
+ loop,
+ [&]() {
+ return "Picking default tile size " +
+ std::to_string(defaultTileSize) +
+ " for unknown tile size '*'";
+ },
+ DEBUG_TYPE);
}
}
}
diff --git a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp
index 26243b2ae84be..6e5ec0ccd1210 100644
--- a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp
+++ b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp
@@ -9,6 +9,7 @@
#include "mlir/Dialect/OpenACC/OpenACCUtils.h"
#include "mlir/Dialect/OpenACC/OpenACC.h"
+#include "mlir/Dialect/Utils/StaticValueUtils.h"
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/Dominance.h"
#include "mlir/IR/SymbolTable.h"
@@ -92,6 +93,10 @@ std::string mlir::acc::getVariableName(mlir::Value v) {
// Walk through view operations until a name is found or can't go further
while (Operation *definingOp = current.getDefiningOp()) {
+ // For integer constants, return their value as a string.
+ if (std::optional<int64_t> constVal = getConstantIntValue(current))
+ return std::to_string(*constVal);
+
// Check for `acc.var_name` attribute
if (auto varNameAttr =
definingOp->getAttrOfType<VarNameAttr>(getVarNameAttrName()))
@@ -331,7 +336,8 @@ mlir::acc::getDominatingDataClauses(mlir::Operation *computeConstructOp,
}
mlir::remark::detail::InFlightRemark
-mlir::acc::emitRemark(mlir::Operation *op, const llvm::Twine &message,
+mlir::acc::emitRemark(mlir::Operation *op,
+ const std::function<std::string()> &messageFn,
llvm::StringRef category) {
using namespace mlir::remark;
mlir::Location loc = op->getLoc();
@@ -351,6 +357,6 @@ mlir::acc::emitRemark(mlir::Operation *op, const llvm::Twine &message,
auto remark = engine->emitOptimizationRemark(loc, opts);
if (remark)
- remark << message.str();
+ remark << messageFn();
return remark;
}
diff --git a/mlir/unittests/Dialect/OpenACC/OpenACCUtilsTest.cpp b/mlir/unittests/Dialect/OpenACC/OpenACCUtilsTest.cpp
index 56cc2b9d9ba0b..489fe9108c04e 100644
--- a/mlir/unittests/Dialect/OpenACC/OpenACCUtilsTest.cpp
+++ b/mlir/unittests/Dialect/OpenACC/OpenACCUtilsTest.cpp
@@ -488,6 +488,30 @@ TEST_F(OpenACCUtilsTest, getVariableNameFromCopyin) {
EXPECT_EQ(varName, name);
}
+TEST_F(OpenACCUtilsTest, getVariableNameConstantInt) {
+ // Create a constant integer value
+ OwningOpRef<arith::ConstantOp> constOp =
+ arith::ConstantOp::create(b, loc, b.getI64IntegerAttr(42));
+
+ Value constVal = constOp->getResult();
+
+ // Test that getVariableName returns the constant value as a string
+ std::string varName = getVariableName(constVal);
+ EXPECT_EQ(varName, "42");
+}
+
+TEST_F(OpenACCUtilsTest, getVariableNameNegativeConstantInt) {
+ // Create a negative constant integer value
+ OwningOpRef<arith::ConstantOp> constOp =
+ arith::ConstantOp::create(b, loc, b.getI64IntegerAttr(-123));
+
+ Value constVal = constOp->getResult();
+
+ // Test that getVariableName returns the negative constant value as a string
+ std::string varName = getVariableName(constVal);
+ EXPECT_EQ(varName, "-123");
+}
+
//===----------------------------------------------------------------------===//
// getRecipeName Tests
//===----------------------------------------------------------------------===//
``````````
</details>
https://github.com/llvm/llvm-project/pull/180665
More information about the Mlir-commits
mailing list