Skip to content

[mlir][Sol] Support all event arg types in EmitOp lowering#66

Merged
vladimirradosavljevic merged 1 commit into
mainfrom
app-event-arg-types
May 22, 2026
Merged

[mlir][Sol] Support all event arg types in EmitOp lowering#66
vladimirradosavljevic merged 1 commit into
mainfrom
app-event-arg-types

Conversation

@abinavpp

@abinavpp abinavpp commented May 6, 2026

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Solidity→Yul lowering of sol.emit to better match EVM log topic rules by transforming indexed event arguments into the form expected by yul.log topics, including hashing reference-typed indexed arguments. It also tightens normalizeABIScalarForEncoding to hard-fail when invoked on unsupported (non-scalar) types.

Changes:

  • Lower indexed reference-typed event args (string/array/struct) to keccak256(abiPackedEncode(arg)) before passing as log topics.
  • Normalize indexed value-typed args to i256 via normalizeABIScalarForEncoding (topics are i256).
  • Replace the previous fallback return val; in normalizeABIScalarForEncoding with an llvm_unreachable for non-scalar types.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
mlir/lib/Conversion/SolToStandard/SolToYul.cpp Updates EmitOp lowering to hash reference-type indexed args and normalize scalar indexed args to i256 topics.
mlir/lib/Conversion/SolToStandard/EVMUtil.cpp Makes normalizeABIScalarForEncoding explicitly unreachable on non-scalar inputs (no silent passthrough).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mlir/lib/Conversion/SolToYul/SolToYul.cpp
Comment thread mlir/lib/Conversion/SolToStandard/SolToYul.cpp Outdated
@github-actions

github-actions Bot commented May 14, 2026

Copy link
Copy Markdown

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 0af903a14710e4a1151d6a9dce67542d0f815f9a d558d0d7a2c0470185ac3f2183f60ca6bd3b162e -- mlir/lib/Conversion/SolToYul/SolToYul.cpp
View the diff from clang-format here.
diff --git a/mlir/lib/Conversion/SolToYul/SolToYul.cpp b/mlir/lib/Conversion/SolToYul/SolToYul.cpp
index 00fc4bf837dc..e251e1ac1c75 100644
--- a/mlir/lib/Conversion/SolToYul/SolToYul.cpp
+++ b/mlir/lib/Conversion/SolToYul/SolToYul.cpp
@@ -3393,8 +3393,7 @@ struct EmitOpLowering : public OpConversionPattern<sol::EmitOp> {
         Value scratchStart = evmB.genFreePtr();
         Value scratchEnd =
             evmB.genABIPackedEncoding(origTy, val, scratchStart, loc);
-        Value scratchSize =
-            r.create<yul::SubOp>(loc, scratchEnd, scratchStart);
+        Value scratchSize = r.create<yul::SubOp>(loc, scratchEnd, scratchStart);
         indexedArgs.push_back(
             r.create<yul::Keccak256Op>(loc, scratchStart, scratchSize));
         continue;

@vladimirradosavljevic vladimirradosavljevic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@abinavpp abinavpp force-pushed the app-event-arg-types branch from 699727b to 07da935 Compare May 21, 2026 12:00
Indexed reference args hash via keccak256 of the packed encoding;
indexed value-type args go through normalizeABIScalarForEncoding.

Also tighten normalizeABIScalarForEncoding to assert on non-scalar types.
@abinavpp abinavpp force-pushed the app-event-arg-types branch from 07da935 to d558d0d Compare May 22, 2026 11:08
@vladimirradosavljevic vladimirradosavljevic merged commit 46ececa into main May 22, 2026
7 of 9 checks passed
@vladimirradosavljevic vladimirradosavljevic deleted the app-event-arg-types branch May 22, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants