release/18.x: [ODS][NFC] Cast range.size() to int32_t in accumulation (#85629) #86677
Conversation
|
@joker-eph What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: None (llvmbot) ChangesBackport bce1703 Requested by: @EugeneZelenko Full diff: https://github.com/llvm/llvm-project/pull/86677.diff 1 Files Affected:
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 71326049af0579..7f748cfbd31ad4 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -3058,7 +3058,7 @@ void OpEmitter::genCodeForAddingArgAndRegionForBuilder(
body << llvm::formatv(
"static_cast<int32_t>(std::accumulate({0}.begin(), {0}.end(), 0, "
"[](int32_t curSum, ::mlir::ValueRange range) {{ return curSum + "
- "range.size(); }))",
+ "static_cast<int32_t>(range.size()); }))",
operandName);
} else {
body << "static_cast<int32_t>(" << getArgumentName(op, i) << ".size())";
|
|
Hi @EugeneZelenko (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. |
Using range.size() "as is" means we accumulate 'size_t' values into 'int32_t' variable. This may produce narrowing conversion warnings (particularly, on MSVC). The surrounding code seems to cast <x>.size() to 'int32_t' so following this practice seems safe enough. Co-authored-by: Ovidiu Pintican <ovidiu.pintican@intel.com> (cherry picked from commit bce1703)
|
@andrey-golubev: Please answer @tstellar question. I just added proper cherry-pick command in issue. |
hi! @tstellar I don't think it's worth it: this is a tiny narrowing conversion warnings suppression (by adding explicit static cast), so not worth it to make it into release notes. |
|
@andrey-golubev OK, no problem. I don't expect a release note entry for every change. |
Backport bce1703
Requested by: @EugeneZelenko