[ODS][NFC] Cast range.size() to int32_t in accumulation #85629
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
|
@llvm/pr-subscribers-mlir-core Author: Andrei Golubev (andrey-golubev) ChangesUsing 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. Full diff: https://github.com/llvm/llvm-project/pull/85629.diff 1 Files Affected:
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 0d81912afb6158..3a697520dfad57 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -3057,7 +3057,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())";
|
|
@llvm/pr-subscribers-mlir Author: Andrei Golubev (andrey-golubev) ChangesUsing 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. Full diff: https://github.com/llvm/llvm-project/pull/85629.diff 1 Files Affected:
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 0d81912afb6158..3a697520dfad57 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -3057,7 +3057,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())";
|
|
@joker-eph @Mogball @jpienaar please take a look! |
|
a gentle reminder, ping. |
joker-eph
approved these changes
Mar 25, 2024
|
and - as already became a usual thing - could you please also click merge? :) |
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Mar 26, 2024
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)
tstellar
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Mar 27, 2024
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)
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 .size() to 'int32_t' so following this practice seems safe enough.