Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent error that occurs when removing a series of redundant Load and Store ops #317

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/sizes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
biguint_binary_ops/BiguintBinaryOps 189 77 112 77 0
boolean_binary_ops/BooleanBinaryOps 1154 471 683 471 0
box_storage/Box 1860 1435 425 1435 0
bug_load_store_load_store 78 73 5 73 0
bytes_ops/BiguintBinaryOps 139 139 0 139 0
calculator 349 317 32 315 2
callsub 32 32 0 32 0
Expand Down
10 changes: 7 additions & 3 deletions src/puya/mir/stack_allocation/peephole.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,16 @@ def optimize_pair(
match a, b:
case mir.LoadLStack(copy=False) as load, mir.StoreLStack(copy=True):
return attrs.evolve(load, copy=True), *maybe_virtuals
# consider this sequence Load*, Virtual(Store*), Virtual(Load*), Store*
# can't just remove outer virtuals because inner virtual ops assume "something"
# loaded a value onto the stack, so need to keep entire sequence around as
# virtual ops
case mir.LoadXStack(), mir.StoreXStack(copy=False):
return maybe_virtuals
return mir.VirtualStackOp(a), *maybe_virtuals, mir.VirtualStackOp(b)
case mir.LoadFStack(), mir.StoreFStack():
return maybe_virtuals
return mir.VirtualStackOp(a), *maybe_virtuals, mir.VirtualStackOp(b)
case mir.LoadVirtual(), mir.StoreVirtual():
return maybe_virtuals
return mir.VirtualStackOp(a), *maybe_virtuals, mir.VirtualStackOp(b)
else:
match a, b:
case (
Expand Down
21 changes: 21 additions & 0 deletions test_cases/bug_load_store_load_store/contract.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from algopy import Contract, UInt64, subroutine, urange


@subroutine
def get_bool() -> bool:
return True


class MyContract(Contract):
def approval_program(self) -> UInt64:
val = UInt64(0)
for _idx in urange(2):
if get_bool():
pass
elif get_bool(): # noqa: SIM102
if not get_bool():
val += UInt64(123)
return val

def clear_state_program(self) -> bool:
return True
90 changes: 90 additions & 0 deletions test_cases/bug_load_store_load_store/out/MyContract.approval.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Op // Op Description Stack (out) Source code Source line

#pragma version 10

// test_cases.bug_load_store_load_store.contract.MyContract.approval_program() -> uint64:
main:
byte "" // allocate 1 to stack (𝕗) val#11 |

main_block@0:
int 0 // (𝕗) val#11 | 0 UInt64(0) bug_load_store_load_store/contract.py:11
// virtual: store val#0 to f-stack (𝕗) val#11,val#0 | val = UInt64(0) bug_load_store_load_store/contract.py:11
int 0 // (𝕗) val#11,val#0 | 0 urange(2) bug_load_store_load_store/contract.py:12
// virtual: store _idx#0 to f-stack (𝕗) val#11,val#0,_idx#0 | _idx bug_load_store_load_store/contract.py:12
// Implicit fall through to main_for_header@1 // (𝕗) val#11,val#0,_idx#0 |

main_for_header@1:
dup // load _idx#0 from f-stack (𝕗) val#11,val#0,_idx#0 | _idx#0 urange(2) bug_load_store_load_store/contract.py:12
int 2 // (𝕗) val#11,val#0,_idx#0 | _idx#0,2 2 bug_load_store_load_store/contract.py:12
< // (𝕗) val#11,val#0,_idx#0 | {<} urange(2) bug_load_store_load_store/contract.py:12
// virtual: store continue_looping%0#0 to l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | continue_looping%0#0 urange(2) bug_load_store_load_store/contract.py:12
// virtual: load continue_looping%0#0 from l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | continue_looping%0#0 for _idx in urange(2): bug_load_store_load_store/contract.py:12
bz main_after_for@11 // (𝕗) val#11,val#0,_idx#0 | for _idx in urange(2): bug_load_store_load_store/contract.py:12
// Implicit fall through to main_for_body@2 // (𝕗) val#11,val#0,_idx#0 | for _idx in urange(2): bug_load_store_load_store/contract.py:12

main_for_body@2:
callsub get_bool // (𝕗) val#11,val#0,_idx#0 | {get_bool} get_bool() bug_load_store_load_store/contract.py:13
// virtual: store tmp%0#0 to l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | tmp%0#0 get_bool() bug_load_store_load_store/contract.py:13
// virtual: load tmp%0#0 from l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | tmp%0#0 if get_bool(): bug_load_store_load_store/contract.py:13
bnz main_after_if_else@9 // (𝕗) val#11,val#0,_idx#0 | if get_bool(): bug_load_store_load_store/contract.py:13
// Implicit fall through to main_else_body@4 // (𝕗) val#11,val#0,_idx#0 | if get_bool(): bug_load_store_load_store/contract.py:13

main_else_body@4:
callsub get_bool // (𝕗) val#11,val#0,_idx#0 | {get_bool} get_bool() bug_load_store_load_store/contract.py:15
// virtual: store tmp%1#0 to l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | tmp%1#0 get_bool() bug_load_store_load_store/contract.py:15
dig 2 // load val#0 from f-stack (𝕗) val#11,val#0,_idx#0 | tmp%1#0,val#0
bury 4 // store val#11 to f-stack (𝕗) val#11,val#0,_idx#0 | tmp%1#0
// virtual: load tmp%1#0 from l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | tmp%1#0 elif get_bool(): # noqa: SIM102 bug_load_store_load_store/contract.py:15
bz main_after_if_else@8 // (𝕗) val#11,val#0,_idx#0 | elif get_bool(): # noqa: SIM102 bug_load_store_load_store/contract.py:15
// Implicit fall through to main_if_body@5 // (𝕗) val#11,val#0,_idx#0 | elif get_bool(): # noqa: SIM102 bug_load_store_load_store/contract.py:15

main_if_body@5:
callsub get_bool // (𝕗) val#11,val#0,_idx#0 | {get_bool} get_bool() bug_load_store_load_store/contract.py:16
// virtual: store tmp%2#0 to l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | tmp%2#0 get_bool() bug_load_store_load_store/contract.py:16
dig 2 // load val#0 from f-stack (𝕗) val#11,val#0,_idx#0 | tmp%2#0,val#0
bury 4 // store val#11 to f-stack (𝕗) val#11,val#0,_idx#0 | tmp%2#0
// virtual: load tmp%2#0 from l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | tmp%2#0 not get_bool() bug_load_store_load_store/contract.py:16
bnz main_after_if_else@7 // (𝕗) val#11,val#0,_idx#0 | not get_bool() bug_load_store_load_store/contract.py:16
// Implicit fall through to main_if_body@6 // (𝕗) val#11,val#0,_idx#0 | not get_bool() bug_load_store_load_store/contract.py:16

main_if_body@6:
dig 1 // load val#0 from f-stack (𝕗) val#11,val#0,_idx#0 | val#0 val += UInt64(123) bug_load_store_load_store/contract.py:17
int 123 // (𝕗) val#11,val#0,_idx#0 | val#0,123 UInt64(123) bug_load_store_load_store/contract.py:17
+ // (𝕗) val#11,val#0,_idx#0 | {+} val += UInt64(123) bug_load_store_load_store/contract.py:17
// virtual: store val#0 to l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | val#0 val += UInt64(123) bug_load_store_load_store/contract.py:17
// virtual: load val#0 from l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | val#0
bury 3 // store val#11 to f-stack (𝕗) val#11,val#0,_idx#0 |
// Implicit fall through to main_after_if_else@7 // (𝕗) val#11,val#0,_idx#0 |

main_after_if_else@7:
// virtual: load val#11 (𝕗) val#11,val#0,_idx#0 | val#11
// virtual: store val#0 to l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | val#0
// virtual: load val#0 from l-stack (no copy) (𝕗) val#11,val#0,_idx#0 | val#0
// virtual: store val#11 (𝕗) val#11,val#0,_idx#0 |
// Implicit fall through to main_after_if_else@8 // (𝕗) val#11,val#0,_idx#0 |

main_after_if_else@8:
dig 2 // load val#11 from f-stack (𝕗) val#11,val#0,_idx#0 | val#11
bury 2 // store val#0 to f-stack (𝕗) val#11,val#0,_idx#0 |
// Implicit fall through to main_after_if_else@9 // (𝕗) val#11,val#0,_idx#0 |

main_after_if_else@9:
dup // load _idx#0 from f-stack (𝕗) val#11,val#0,_idx#0 | _idx#0 urange(2) bug_load_store_load_store/contract.py:12
int 1 // (𝕗) val#11,val#0,_idx#0 | _idx#0,1 urange(2) bug_load_store_load_store/contract.py:12
+ // (𝕗) val#11,val#0,_idx#0 | {+} urange(2) bug_load_store_load_store/contract.py:12
bury 1 // store _idx#0 to f-stack (𝕗) val#11,val#0,_idx#0 | urange(2) bug_load_store_load_store/contract.py:12
b main_for_header@1 // (𝕗) val#11,val#0,_idx#0 |

main_after_for@11:
dig 1 // load val#0 from f-stack (𝕗) val#11,val#0,_idx#0 | val#0 return val bug_load_store_load_store/contract.py:18
return // (𝕗) val#11,val#0,_idx#0 | return val bug_load_store_load_store/contract.py:18


// test_cases.bug_load_store_load_store.contract.get_bool() -> uint64:
get_bool:
proto 0 1 // @subroutine\ndef get_bool() -> bool: bug_load_store_load_store/contract.py:4-5

get_bool_block@0:
int 1 // 1 True bug_load_store_load_store/contract.py:6
retsub // 1 return True bug_load_store_load_store/contract.py:6

73 changes: 73 additions & 0 deletions test_cases/bug_load_store_load_store/out/MyContract.approval.teal
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
#pragma version 10

test_cases.bug_load_store_load_store.contract.MyContract.approval_program:
byte ""
// bug_load_store_load_store/contract.py:11
// val = UInt64(0)
int 0
// bug_load_store_load_store/contract.py:12
// for _idx in urange(2):
dup

main_for_header@1:
// bug_load_store_load_store/contract.py:12
// for _idx in urange(2):
dup
int 2
<
bz main_after_for@11
// bug_load_store_load_store/contract.py:13
// if get_bool():
callsub get_bool
bnz main_after_if_else@9
// bug_load_store_load_store/contract.py:15
// elif get_bool(): # noqa: SIM102
callsub get_bool
dig 2
bury 4
bz main_after_if_else@8
// bug_load_store_load_store/contract.py:16
// if not get_bool():
callsub get_bool
dig 2
bury 4
bnz main_after_if_else@7
// bug_load_store_load_store/contract.py:17
// val += UInt64(123)
dig 1
int 123
+
bury 3

main_after_if_else@7:

main_after_if_else@8:
dig 2
bury 2

main_after_if_else@9:
// bug_load_store_load_store/contract.py:12
// for _idx in urange(2):
dup
int 1
+
bury 1
b main_for_header@1

main_after_for@11:
// bug_load_store_load_store/contract.py:18
// return val
dig 1
return


// test_cases.bug_load_store_load_store.contract.get_bool() -> uint64:
get_bool:
// bug_load_store_load_store/contract.py:4-5
// @subroutine
// def get_bool() -> bool:
proto 0 1
// bug_load_store_load_store/contract.py:6
// return True
int 1
retsub
9 changes: 9 additions & 0 deletions test_cases/bug_load_store_load_store/out/MyContract.clear.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Op // Stack (out) Source code Source line

#pragma version 10

// test_cases.bug_load_store_load_store.contract.MyContract.clear_state_program() -> uint64:
main_block@0:
int 1 // 1 True bug_load_store_load_store/contract.py:21
return // return True bug_load_store_load_store/contract.py:21

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma version 10

test_cases.bug_load_store_load_store.contract.MyContract.clear_state_program:
// bug_load_store_load_store/contract.py:21
// return True
int 1
return
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
contract test_cases.bug_load_store_load_store.contract.MyContract:
program approval:
subroutine test_cases.bug_load_store_load_store.contract.MyContract.approval_program() -> uint64:
block@0: // L10
let val#0: uint64 = 0u
let _idx#0: uint64 = 0u
goto block@1
block@1: // for_header_L12
let continue_looping%0#0: bool = (< _idx#0 2u)
goto continue_looping%0#0 ? block@2 : block@11
block@2: // for_body_L13
let tmp%0#0: bool = test_cases.bug_load_store_load_store.contract.get_bool()
goto tmp%0#0 ? block@9 : block@4
block@4: // else_body_L15
let tmp%1#0: bool = test_cases.bug_load_store_load_store.contract.get_bool()
let val#11: uint64 = val#0
goto tmp%1#0 ? block@5 : block@8
block@5: // if_body_L16
let tmp%2#0: bool = test_cases.bug_load_store_load_store.contract.get_bool()
let val#11: uint64 = val#0
goto tmp%2#0 ? block@7 : block@6
block@6: // if_body_L17
let val#0: uint64 = (+ val#0 123u)
let val#11: uint64 = val#0
goto block@7
block@7: // after_if_else_L16
let val#0: uint64 = val#11
let val#11: uint64 = val#0
goto block@8
block@8: // after_if_else_L15
let val#0: uint64 = val#11
goto block@9
block@9: // after_if_else_L13
let _idx#0: uint64 = (+ _idx#0 1u)
goto block@1
block@11: // after_for_L12
return val#0

subroutine test_cases.bug_load_store_load_store.contract.get_bool() -> bool:
block@0: // L4
return 1u

program clear-state:
subroutine test_cases.bug_load_store_load_store.contract.MyContract.clear_state_program() -> bool:
block@0: // L20
return 1u
Loading
Loading