Skip to content

[mlir] Add tests for the bytesN bitwise/shift ops fix#143

Merged
PavelKopyl merged 1 commit into
mainfrom
kpv-fix-fixedbytes-ops
Jun 8, 2026
Merged

[mlir] Add tests for the bytesN bitwise/shift ops fix#143
PavelKopyl merged 1 commit into
mainfrom
kpv-fix-fixedbytes-ops

Conversation

@PavelKopyl

@PavelKopyl PavelKopyl commented Jun 5, 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 MLIR codegen and lit coverage to validate the corrected handling of bytesN bitwise (& | ^ ~) and shift (<< >>) operations, and removes a couple of semantic tests from the “expected failures” list now that they pass.

Changes:

  • Update SolidityToMLIRPass to emit sol.{and,or,xor,shl,shr,not} directly on !sol.fixedbytes<N> operands (and allow shift RHS to stay in its “mobile” integer type).
  • Refresh existing FileCheck expectations for bytes/fixedbytes bitwise and shift lowering (both Sol and EVM pipelines).
  • Add new fixedbytes.sol lit tests (Sol + EVM) covering comparisons, bitwise ops, unary ~, shifts, and mixed-size fixed-bytes operands; remove now-passing semtests from the MLIR failure list.

Reviewed changes

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

Show a summary per file
File Description
test/mlirSemtestFailures.txt Removes two tests from the expected-MLIR-failures list because they now pass.
test/lit/mlirCodegen/shift-bytes.sol Updates checks to expect direct sol.shl/sol.shr on !sol.fixedbytes<20> rather than cast-to-int shims.
test/lit/mlirCodegen/fixedbytes.sol New lit test covering bytesN comparisons/bitwise/shift/unary-not (Sol pipeline).
test/lit/mlirCodegen/EVM/shift-bytes.sol Updates EVM-lowering checks for fixed-bytes shifts to match new lowering pattern.
test/lit/mlirCodegen/EVM/fixedbytes.sol New lit test covering bytesN comparisons/bitwise/shift/unary-not (EVM pipeline).
test/lit/mlirCodegen/EVM/arith.sol Updates expected lowering for bytes4 bitwise-not and some shift-related masking behavior.
test/lit/mlirCodegen/bytes.sol Updates checks to expect sol.and/or/xor directly on !sol.fixedbytes<5> rather than int casts.
test/lit/mlirCodegen/arith.sol Updates checks to reflect direct sol.not on !sol.fixedbytes<4> and updated shift op typing.
libsolidity/codegen/mlir/SolidityToMLIR.cpp Core codegen change: stop converting bytesN to ints for bitwise/shift ops; emit ops directly with appropriate result/RHS typing.

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

@PavelKopyl PavelKopyl requested review from abinavpp and vladimirradosavljevic and removed request for vladimirradosavljevic June 8, 2026 07:08

@abinavpp abinavpp 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.

thank you!

Comment thread libsolidity/codegen/mlir/SolidityToMLIR.cpp Outdated

@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. Can we add tests for ByteType?

@PavelKopyl PavelKopyl force-pushed the kpv-fix-fixedbytes-ops branch from 3f86f2e to 2cb9f36 Compare June 8, 2026 20:31
@PavelKopyl

Copy link
Copy Markdown
Contributor Author

LGTM. Can we add tests for ByteType?

Yes, but it doesn’t seem easy to generate such a test from Solidity. I’ve created a test directly from the Sol dialect: matter-labs/solx-llvm#87

@PavelKopyl PavelKopyl merged commit f8d6a96 into main Jun 8, 2026
2 checks passed
@PavelKopyl PavelKopyl deleted the kpv-fix-fixedbytes-ops branch June 8, 2026 20:59
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.

4 participants