[mlir][emitc] Fix bug in dereference translation #171028
Merged
Conversation
This file contains hidden or 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
The op was not added to `hasDeferredEmission()` when introduced by f17abc2, causing incorrect translation.
Member
|
@llvm/pr-subscribers-mlir-emitc @llvm/pr-subscribers-mlir Author: Gil Rapaport (aniragil) ChangesThe op was not added to Full diff: https://github.com/llvm/llvm-project/pull/171028.diff 2 Files Affected:
diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index 55b9c3dc0a355..15c23c60d0b86 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -333,7 +333,8 @@ struct CppEmitter {
/// Determine whether expression \p op should be emitted in a deferred way.
static bool hasDeferredEmission(Operation *op) {
- return isa_and_nonnull<emitc::GetGlobalOp, emitc::LiteralOp, emitc::MemberOp,
+ return isa_and_nonnull<emitc::DereferenceOp, emitc::GetGlobalOp,
+ emitc::LiteralOp, emitc::MemberOp,
emitc::MemberOfPtrOp, emitc::SubscriptOp,
emitc::GetFieldOp>(op);
}
diff --git a/mlir/test/Target/Cpp/common-cpp.mlir b/mlir/test/Target/Cpp/common-cpp.mlir
index abf85c8e9a359..f397a4ae9709f 100644
--- a/mlir/test/Target/Cpp/common-cpp.mlir
+++ b/mlir/test/Target/Cpp/common-cpp.mlir
@@ -118,7 +118,7 @@ func.func @address_of() {
// CHECK-LABEL: void dereference
// CHECK-SAME: (int32_t* [[ARG0:[^ ]*]]) {
func.func @dereference(%arg0: !emitc.ptr<i32>) {
- // CHECK: int32_t [[V1:[^ ]*]] = *[[ARG0]];
+ // CHECK-NEXT: int32_t [[V1:[^ ]*]] = *[[ARG0]];
%2 = emitc.dereference %arg0 : !emitc.ptr<i32>
emitc.load %2 : !emitc.lvalue<i32>
return
|
Contributor
Author
|
@simon-camp , @marbre - I pushed this PR promptly to hopefully get this fix to downstream users before they hit the bug, but post-commit review is welcome. |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/13397 Here is the relevant piece of the build log for the reference |
honeygoyal
pushed a commit
to honeygoyal/llvm-project
that referenced
this pull request
Dec 9, 2025
The op was not added to `hasDeferredEmission()` when introduced by f17abc2, causing incorrect translation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The op was not added to
hasDeferredEmission()when introduced by f17abc2, causing incorrect translation.