Use gt(calldatasize(), 3) for the function dispatch check in IR codegen

[philippecyberian]

Summary

The IR dispatcher emits a guard that skips the selector switch when calldata is too short to hold a function selector. It was spelled as a negated less-than:

if iszero(lt(calldatasize(), 4))

This PR spells the same condition directly:

if gt(calldatasize(), 3)

Both mean calldatasize() >= 4. The second form drops the ISZERO, so the guard assembles to four instructions instead of five.

This is a one-line change in IRGenerator::dispatchRoutine(). The bulk of the diff is updated test expectations.

Note: this supersedes the original approach in this PR, which added a FunctionSelectorGuard peephole rule to the EVM assembly optimizer. Following review feedback from @nikola-matic, the pattern is now fixed at its source in Yul codegen instead of being matched and rewritten afterwards. The peephole rule has been removed entirely.

---

Motivation

The guard is emitted in the dispatcher of every contract compiled via IR that has at least one external function, so the saving applies broadly:

	Before	After
Instructions	PUSH1 0x04 / CALLDATASIZE / LT / ISZERO	PUSH1 0x03 / CALLDATASIZE / GT
Runtime bytes	5	4
Static gas	11	8

Measured with evmone across a 22-contract corpus: 3 gas per external call and 1 byte
of runtime code, uniformly, in every compilation unit that emits a dispatcher. Contracts
with no external function cases — libraries, internal-only contracts, and contracts with
only receive()/fallback() — are bit-identical between the two builds, since the guard
sits inside the <?+cases> conditional. See the measurement comment below for the
breakdown and method.

Equivalence

calldatasize() is unsigned, so over the full domain:

!(cds < 4)  ≡  (cds >= 4)  ≡  (cds > 3)

Checked exhaustively with Z3:

(assert (>= cds 0))
(assert (not (= (ite (< cds 4) 0 1)
                (ite (> cds 3) 1 0))))
(check-sat) ; unsat

Testing

Existing expectations across cmdlineTests, semanticTests and gasTests are updated to reflect the new instruction sequence.

One new test, semanticTests/fallback/short_data_reverts_without_fallback.sol, pins the boundary the change moves. It walks 0/1/2/3-byte calldata (must revert), 4 bytes with a valid selector (must dispatch), and 4 bytes with an unknown selector (must revert). It fails in both directions if the bound slips: gt(calldatasize(), 4) stops the 4-byte call dispatching, and gt(calldatasize(), 2) lets a 3-byte call through, because calldataload zero-pads and the truncated selector reads back as a valid one.

The pre-existing fallback/short_data_calls_fallback.sol covers only the contract-with-fallback path, and revertStrings/unknown_sig_no_fallback.sol is a single 1-byte case on a contract with a receive(), so the no-fallback boundary was previously uncovered.

[github-actions]

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

[philippecyberian]

I fixed the genuine CI check failure due to coding style. The remaining failures are due to possibly sporadic (the failing checks change each run) troubles with accessing external resources

[nikola-matic]

Hi @philippecyberian and thanks for this. There's also likely a significantly easier approach to achieve the same effect by changing the Yul codegen directly. Do you want to join our next Wednesday public call so we can discuss it?

[philippecyberian]

Thanks @nikola-matic for reviewing and for the call invitation. I can't join this week but am looking forward to discussing it on next week's call.

[philippecyberian]

Thanks again @nikola-matic . This PR is a consequence of my work on a term paper for a Graduate class I'm taking. I was hoping for a quick review and merge to impress the grader :-) I absolutely want to to see the change through but I will de-prioritize it just a little given that I'm also scheduled to go help out the Math teacher in my daughter's class atelier at the same time as the call this week. I will join the call right after the New Year to discuss enabling the improvement by changing the Yul codegen directly. I'm all onboard making the right change in the right place, and then possibly looking at further, subsequent contributions. Happy Holidays!

[philippecyberian]

Hi again @nikola-matic . I've reworked the change as suggested and look forward to your further review.

Approach reworked: dispatch fixed at the IR level instead of assembly peepholes

I've dropped the peephole-based approach entirely in favor of a one-line change at the source of the pattern: IRGenerator::dispatchRoutine now emits gt(calldatasize(), 3) instead of iszero(lt(calldatasize(), 4)) as the function dispatch guard. The two are equivalent, but the new form needs one instruction less; every via-IR contract saves 3 gas per incoming external call and 1 byte of deployed code, with no pattern matching in the assembly optimizer.

New semantic test fallback/short_data_reverts_without_fallback.sol pins the dispatch boundary for a contract without a fallback (0–3 byte calldata reverts, the exact 4-byte selector dispatches, an unknown 4-byte selector reverts), complementing the existing short_data_calls_fallback.sol.

Test expectations regenerated: command-line tests (IR text, bytecode, source maps, CFG JSON) and semantic gas expectations (irOptimized and ssaCFGOptimized)

[philippecyberian]

t_native_test_ext_zeppelin is red on develop itself at the merged base (reviewers can verify via develop build 2084073)

[msooseth]

iszero(lt(x),4) means NOT(x < 4). gt (x, 3) means x>3.
So one is x >= 4, the other x > 3

LGTM

[blishko]

Looks good to me. This is an excellent candidate for the runtime gas benchmarking @rodiazet is building. We should wait with merging till we get results from that.

[philippecyberian]

Sounds good, no rush on my end — happy to wait for @rodiazet's harness.

For reference, the numbers currently in the description came from evmone: the guard drops from 11 to 8 gas statically (PUSH1/CALLDATASIZE/LT/ISZERO → PUSH1/CALLDATASIZE/GT), 2 gas observed per external call, plus one byte less deployed runtime code. Glad to re-measure with the new benchmarking once it lands so there's a directly comparable number.

In the meantime I've tidied the branch up while it waits: it's now rebased onto current develop as a single commit (the merge commits and conflict-resolution commits are gone, per the review checklist), and the title and description have been rewritten to describe the IR codegen change rather than the original peephole approach. CI is re-running after the rebase.

[philippecyberian]

@blishko — happy to wait for @rodiazet's harness before merging. In the meantime, here are measurements that may be useful as a cross-check when it lands.

Method. Built solc twice from this branch, once with the change and once with the single line reverted. Compiled a 22-contract corpus with both (--via-ir --optimize --no-cbor-metadata), then executed every resulting runtime object under evmone 0.18 via evmc run, on two paths: empty calldata (guard rejects) and the first function selector (guard accepts).

Result — uniform, not merely typical:

Compilation units	Count	Δ bytes	Δ gas, rejected call	Δ gas, dispatched call
With a selector dispatcher	19	−1 (all)	−3 (18 of 19)	−3 (19 of 19)
Without one (controls)	4	0 (all)	0 (all)	n/a

The corpus spans minimal contracts, an ERC-20, a proxy, libraries, inheritance, interfaces, overloads, custom errors, and string/struct/array parameters.

The four controls are the interesting part: a library, a contract with only internal functions, one with only receive(), and one with only fallback(). All four are bit-identical between the two builds, which is what we want — the guard sits inside the <?+cases> conditional, so it is simply not emitted when there are no cases to dispatch to.

One gap: the proxy contract on the rejected-call path shows no difference. That is the harness, not the change. evmc run executes without host state, so the delegatecall in that contract's fallback isn't modelled and both builds report an identical figure near 63/64 of the gas limit. The same contract's dispatched path shows the expected −3, and its code is one byte shorter like every other dispatcher.

---

Correction to the PR description. It previously claimed 2 gas per external call. That was a measurement error on my part; the correct figure is 3, which is exactly what the instruction costs predict:

before:  PUSH1 (3) + CALLDATASIZE (2) + LT (3) + ISZERO (3) = 11
after:   PUSH1 (3) + CALLDATASIZE (2) + GT (3)              =  8

I had also invented an explanation for the 1-gas gap ("real-world dispatcher structure absorbs it"), which was wrong — the guard is straight-line code with nothing available to absorb a gas unit. Description now updated.

One note for the benchmarking harness. Comparing bytecode sizes across two builds of the compiler needs --no-cbor-metadata, or metadata stripping. A dirty working tree makes solc stamp its version as ...+commit.<sha>.mod instead of ...+commit.<sha>, and since that string is embedded in the CBOR metadata appended to every contract, the "before" binary carries 4 extra bytes unrelated to the code. It produced a convincing "5 bytes saved" for me before I caught it; the real figure is 1.