Skip to content

Commit

Permalink
fix[venom]: alloca for default arguments (vyperlang#4155)
Browse files Browse the repository at this point in the history
this commit fixes an `ir_node_to_venom` translation bug. when there is
a default argument to an external function, it can generate multiple
allocas, because the entry points allocate separate symbol tables, but
actually they should all correspond to the same alloca. for instance,
`external 1 foo(uint256)12345` and `external 1 foo()67890` both feed
into the same `external 1 foo()__common`, but the current translator
mistakenly creates different symbol tables for the two "feeder" entry
points, resulting in separate allocas for the same logical variable.

this commit fixes the bug by fusing the symbol tables for multiple
entry points to the same external function.
  • Loading branch information
charles-cooper authored Jun 17, 2024
1 parent 2d82a74 commit c79c0b6
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions vyper/venom/ir_node_to_venom.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,16 @@
NOOP_INSTRUCTIONS = frozenset(["pass", "cleanup_repeat", "var_list", "unique_symbol"])

SymbolTable = dict[str, Optional[IROperand]]
_global_symbols: SymbolTable = {}
_global_symbols: SymbolTable = None # type: ignore
MAIN_ENTRY_LABEL_NAME = "__main_entry"
_external_functions: dict[int, SymbolTable] = None # type: ignore


# convert IRnode directly to venom
def ir_node_to_venom(ir: IRnode) -> IRContext:
global _global_symbols
global _global_symbols, _external_functions
_global_symbols = {}
_external_functions = {}

ctx = IRContext()
fn = ctx.create_function(MAIN_ENTRY_LABEL_NAME)
Expand Down Expand Up @@ -214,10 +216,6 @@ def _convert_ir_bb_list(fn, ir, symbols):
return ret


current_func = None
var_list: list[str] = []


def pop_source_on_return(func):
@functools.wraps(func)
def pop_source(*args, **kwargs):
Expand All @@ -232,7 +230,10 @@ def pop_source(*args, **kwargs):
@pop_source_on_return
def _convert_ir_bb(fn, ir, symbols):
assert isinstance(ir, IRnode), ir
global _break_target, _continue_target, current_func, var_list, _global_symbols
# TODO: refactor these to not be globals
global _break_target, _continue_target, _global_symbols, _external_functions

# keep a map from external functions to all possible entry points

ctx = fn.ctx
fn.push_source(ir)
Expand Down Expand Up @@ -274,7 +275,6 @@ def _convert_ir_bb(fn, ir, symbols):

return ret
elif is_external:
_global_symbols = {}
ret = _convert_ir_bb(fn, ir.args[0], symbols)
_append_return_args(fn)
else:
Expand Down Expand Up @@ -382,6 +382,13 @@ def _convert_ir_bb(fn, ir, symbols):
data = _convert_ir_bb(fn, c, symbols)
ctx.append_data("db", [data]) # type: ignore
elif ir.value == "label":
function_id_pattern = r"external (\d+)"
function_name = ir.args[0].value
m = re.match(function_id_pattern, function_name)
if m is not None:
function_id = m.group(1)
_global_symbols = _external_functions.setdefault(function_id, {})

label = IRLabel(ir.args[0].value, True)
bb = fn.get_basic_block()
if not bb.is_terminated:
Expand Down

0 comments on commit c79c0b6

Please sign in to comment.