Skip to content

fix: reject intrinsic op variants with immediate when literal argument value exceeds its range#711

Open
boblat wants to merge 5 commits intomainfrom
fix/op-selection
Open

fix: reject intrinsic op variants with immediate when literal argument value exceeds its range#711
boblat wants to merge 5 commits intomainfrom
fix/op-selection

Conversation

@boblat
Copy link
Copy Markdown
Contributor

@boblat boblat commented Apr 20, 2026

Fixes #709.

  • when building IR from AWST, always pick the stack-arg variant and let the IR optimiser promote stack args to immediates when the values actually fit.

  • add checks on literal value <= 255 to immediate promotions for gitxnas, itxnas, loads/stores in IR optimiser

  • add immediate promotion cases in IR optimiser for gaids, gloads, gloadss, txnas, gtxns, gtxnas, gtxnsa, gtxnsas so the new AWST behavior still produces the compact form when possible.

Note: intrincis simplification optimiser does not run for O0 so O0 stats are up.

@engineering-ci
Copy link
Copy Markdown

engineering-ci Bot commented Apr 20, 2026

Name Status O0 bytes O1 bytes O2 bytes O0 ops O1 ops O2 ops
box_storage 🟢 - - - +1 - -
calculator 🟢 - - - +3 - -
hello_world 🟢 +1 - - +1 - -
local_state/LocalStateContract 🟢 - - - +3 - -
local_state/LocalStateWithOffsets 🟢 - - - +4 - -
abi_routing/CustomApproval 🟢 - - - +1 - -
application 🟢 - - - +1 - -
arc4_conversions/TestContract 🟢 - - - +4 - -
arc4_dynamic_arrays 🟢 - - - +4 - -
arc4_types/Arc4BoolEvalContract 🟢 - - - +1 - -
arc4_types/Arc4BoolTypeContract 🟢 - - - +1 - -
arc4_types/Arc4DynamicBytesContract 🟢 - - - +1 - -
arc4_types/Arc4MutationContract 🟢 - - - +10 - -
arc4_types/Arc4StringTypesContract 🟢 - - - +8 - -
arc4_types/Arc4StructsTypeContract 🟢 - - - +1 - -
arc4_types/Arc4TuplesTypeContract 🟢 - - - +1 - -
arc4_types/MutableParams2 🟢 - - - +4 - -
array/Contract 🟢 - - - +2 - -
array/ImmutableArrayContract 🟢 - - - +22 - -
array/StaticSizeContract 🟢 - - - +2 - -
asset 🟢 - - - +4 - -
bytes_backed_ops 🟢 +2 - - +6 - -
dup2_optimization_bug 🟢 +2 - - +2 - -
edverify 🟢 +2 - - +4 - -
everything 🟢 - - - +2 - -
fixed_bytes_ops/FixedBytesOps 🟢 - - - +10 - -
group_side_effects/AppExpectingEffects 🟢 - -1🟢 -1 - -1🟢 -1
inner_transactions/MyContract 🟢 - - - +1 - -
intrinsics/ImmediateVariants 🟢 - -3🟢 -3 +9 -7🟢 -7
large_box_operations/NestedItemArrayUInt64 🟢 - - - +4 - -
logic_signature/args_complex 🟢 +21 - - +22 - -
logic_signature/args_complex_no_validation 🟢 +11 - - +17 - -
logic_signature/args_simple 🟢 +3 - - +3 - -
logic_signature/dont_use_this 🟢 +9 - - +5 - -
logic_signature/pre_approved_sale 🟢 - - - - -11🟢 -11
match/MyContract 🟢 - - - +1 - -
mutable_native_types/Case1WithTups 🟢 - - - +1 - -
mutable_native_types/Case2WithImmStruct 🟢 - - - +1 - -
mutable_native_types/Case3WithStruct 🟢 - - - +1 - -
mutable_native_types/Contract 🟢 - - - +5 - -
regression_tests/ExtractOpSelection 🆕 +604 +8 +8 +21 +4 +4
regression_tests/GTxnArrayGroupOpSelection 🆕 +25 +18 +18 +12 +9 +9
regression_tests/GTxnArrayIndexOpSelection 🆕 +25 +18 +18 +12 +9 +9
regression_tests/GTxnOpSelection 🆕 +17 +11 +11 +7 +5 +5
regression_tests/GaidOpSelection 🆕 +16 +10 +10 +7 +5 +5
regression_tests/GloadIOpSelection 🆕 +18 +12 +12 +8 +5 +5
regression_tests/GloadTOpSelection 🆕 +18 +11 +11 +8 +5 +5
regression_tests/ReplaceFoldOOB 🟢 +1 - - +1 - -
regression_tests/ReplaceOpSelection 🆕 +549 +543 +543 +13 +11 +11
regression_tests/SubstringEndOOB 🟢 +2 - - +2 - -
regression_tests/SubstringOpSelection 🆕 +605 +8 +8 +21 +4 +4
regression_tests/TxnArrayOpSelection 🆕 +23 +17 +17 +11 +9 +9
regression_tests/arg_op_selection 🆕 +15 +12 +12 +8 +7 +7
simplish 🟢 - - - +3 - -
state_mutations 🟢 - - - +2 - -
too_many_permutations 🟢 +2 - - +4 - -
transaction 🟢 - -1🟢 -1 - -7🟢 -7
tuple_support/NestedTuples 🟢 - - - +4 - -
tuple_support/NestedTuplesStorage 🟢 - - - +4 - -
typed_abi_call/Logger 🟢 +3 - - +9 - -
typed_itxn_abi_call/Logger 🟢 +3 - - +9 - -
undefined_phi_args 🟢 - - - +2 - -
native_array 🟢 - - - +13 - -
Total 🟢 +62 -5🟢 -5 +226 -26🟢 -26

@engineering-ci
Copy link
Copy Markdown

engineering-ci Bot commented Apr 20, 2026

Checking stubs for changes and corresponding version bump in origin/fix/op-selection

Last puyapy release: v5.8.1
This branch stubs version: 3.5.0
Last released stubs version: 3.5.0
Main stubs version: 3.5.0

✅ Stub files unchanged
✅ Stub check passed!

@engineering-ci
Copy link
Copy Markdown

engineering-ci Bot commented Apr 20, 2026

Coverage

Tests Skipped Failures Errors Time
1691 3 💤 0 ❌ 0 🔥 11m 41s ⏱️

@boblat boblat force-pushed the fix/op-selection branch 2 times, most recently from bc555c9 to 7de9d78 Compare April 20, 2026 07:47
Copy link
Copy Markdown
Contributor

@Argimirodelpozo Argimirodelpozo left a comment

Choose a reason for hiding this comment

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

So I think that for any op. that would unconditionally fail with something out of bounds for a certain type (gaid, gload, etc.) we should not let that compile but instead emit an error directly. The argument for the different handling vs. what happened with intrinsic folding is that with intrinsic folding it might not be obvious for a dev. that the folding is happening (something might end up foldable after some other opts.) while in this case it'd be a result of direct user input.
Also in this case we wouldn't have the -O0 vs -O1/-O2 divergent behavior, so I'd say lets be aggresive in those instances and fail compilation.

@daniel-makerx
Copy link
Copy Markdown
Contributor

So I think that for any op. that would unconditionally fail with something out of bounds for a certain type (gaid, gload, etc.) we should not let that compile but instead emit an error directly.

When you say out of bounds here, are you referring to the protocol limit of 16 for number of transactions in a group? If so is that a bound that could ever realistically be changed at the protocol level?

@boblat boblat force-pushed the fix/op-selection branch 2 times, most recently from a7b58ec to 9a4180c Compare April 21, 2026 07:32
@Argimirodelpozo
Copy link
Copy Markdown
Contributor

When you say out of bounds here, are you referring to the protocol limit of 16 for number of transactions in a group? If so is that a bound that could ever realistically be changed at the protocol level?

Yes (the T param. for gaid and gload) and also talking about the scratch limit in gload (the I param bounded by 255).
I'd say both of these are very stable protocol bounds. Not that they absolutely couldn't, but if they were to be changed it'd constitute a major breaking change (and I don't think they have ever changed since their introduction), so IMO they are safe to assume hard constants (unlike something like txn fields that I'd say is more subject to change from time to time e.g. when RejectVersion was introduced recently).

@boblat boblat force-pushed the fix/op-selection branch from 9a4180c to 3a619e0 Compare April 22, 2026 01:31
@boblat boblat force-pushed the fix/op-selection branch 3 times, most recently from 9a8d6a0 to b5fd110 Compare April 23, 2026 11:00
@boblat boblat force-pushed the fix/op-selection branch from b5fd110 to 7176a92 Compare April 23, 2026 11:08
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.

Variants of op codes with unit8 immediates are incorrectly used even when the parameter exceeds uint8 bound

3 participants