-
-
Notifications
You must be signed in to change notification settings - Fork 802
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
feat[venom]: multidimensional fencing #4066
Draft
charles-cooper
wants to merge
71
commits into
vyperlang:master
Choose a base branch
from
charles-cooper:feat/fencing
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
d552898
feat: multidimensional fencing
charles-cooper d4dad09
invalidate liveness
charles-cooper 398a344
Merge branch 'master' into feat/fencing
charles-cooper a50473a
add log, revert to effects
charles-cooper c7da081
sha3 fence
charles-cooper 80c5154
fix fence checker
charles-cooper 6cf7b6b
feat[venom]: extract literals pass
charles-cooper 04b53d0
don't reorder param instructions
charles-cooper 8adf783
remove a comment
charles-cooper 8506bbb
small perf
charles-cooper ef7c369
feat: store expansion pass
charles-cooper ff700b4
lint
charles-cooper ea9b1c5
remove inter-bb restriction
charles-cooper adbf01c
don't replace first use
charles-cooper f3acde1
fix terminator instruction
charles-cooper 988a1a9
wip - fix fence
charles-cooper 058c4db
fix can_reorder
charles-cooper edaf756
Merge branch 'master' into feat/fencing
charles-cooper 3555fcb
force phi instructions first
charles-cooper 3e096de
fix can_reorder(?)
charles-cooper 7dc473d
clean up phi
charles-cooper e087c4f
update fence calculation
charles-cooper 50e13cf
Revert "update fence calculation"
charles-cooper 490c377
sort again
charles-cooper 608bfaa
traverse out_vars
charles-cooper afb49a6
update can_reorder
charles-cooper 3d0c4bb
fix a table
charles-cooper a44c0bd
wip - effects graph
charles-cooper f1bb354
update table
charles-cooper 6e87f1f
minor cleanup
charles-cooper 8b415ff
downstream_of data structure
charles-cooper eb55ab2
remove old fence member
charles-cooper 733b1fb
lint
charles-cooper 7e184e2
fix bad dependency
charles-cooper 83ba491
traverse down effects graph
charles-cooper 9ec2b4e
fix phi+param instructions
charles-cooper 939d671
don't traverse downstream
charles-cooper 6f370ca
cleanup
charles-cooper d50130e
reverse out_vars
charles-cooper 881daf6
fiddle with stack layout, traversal order
charles-cooper aa2234c
fix bugs
charles-cooper b6b7aed
allow inter-bb
charles-cooper a71cad8
lint
charles-cooper ef8a56c
reverse use traversal
charles-cooper 7065892
for debugging
charles-cooper 2dad58b
add balance fence
charles-cooper 20bd8b3
add balance fence
charles-cooper f89711e
Merge branch 'feat/store-expansion' into feat/fencing
charles-cooper 7fa84b4
lift out prev=var
charles-cooper 18192ce
fiddle with store expansion
charles-cooper 372d007
tune order of passes
charles-cooper d83c886
add a degree of freedom
charles-cooper 33f15e5
add a peephole optimization
charles-cooper 6174be0
run dftpass twice!
charles-cooper c7d57f4
remove dead variables
charles-cooper eaa1c67
add returndata fencing
charles-cooper e385ecb
fix lint
charles-cooper 993e875
fix returndata
charles-cooper 1f8001f
Merge branch 'master' into feat/fencing
charles-cooper b84595a
improved sanity check
charles-cooper f7adb8a
Merge branch 'master' into feat/fencing
charles-cooper 2a7fde7
use .first()
charles-cooper 0d19fd5
another usage of first()
charles-cooper 0582db5
wip - store-expand bb in vars
charles-cooper 4f3b0ec
Merge branch 'master' into feat/fencing
charles-cooper 04314e0
move normalization pass
charles-cooper 32a5da5
debug show cost
charles-cooper 35fe8ce
wip
charles-cooper f5b2609
revert cfg traversal order change
charles-cooper 6129813
bring back store expansion
charles-cooper c6dd3ff
Merge branch 'master' into feat/fencing
charles-cooper File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,81 +1,224 @@ | ||
from collections import defaultdict | ||
from dataclasses import asdict, dataclass | ||
|
||
from vyper.utils import OrderedSet | ||
from vyper.venom.analysis.dfg import DFGAnalysis | ||
from vyper.venom.analysis.liveness import LivenessAnalysis | ||
from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable | ||
from vyper.venom.function import IRFunction | ||
from vyper.venom.passes.base_pass import IRPass | ||
|
||
_ALL = ("storage", "transient", "memory", "immutables", "balance", "returndata") | ||
|
||
writes = { | ||
"sstore": "storage", | ||
"tstore": "transient", | ||
"mstore": "memory", | ||
"istore": "immutables", | ||
"call": _ALL, | ||
"delegatecall": _ALL, | ||
"staticcall": "memory", | ||
"create": _ALL, | ||
"create2": _ALL, | ||
"invoke": _ALL, # could be smarter, look up the effects of the invoked function | ||
"dloadbytes": "memory", | ||
"returndatacopy": "memory", | ||
"calldatacopy": "memory", | ||
"codecopy": "memory", | ||
"extcodecopy": "memory", | ||
"mcopy": "memory", | ||
} | ||
reads = { | ||
"sload": "storage", | ||
"tload": "transient", | ||
"iload": "immutables", | ||
"mload": "memory", | ||
"mcopy": "memory", | ||
"call": _ALL, | ||
"delegatecall": _ALL, | ||
"staticcall": _ALL, | ||
"returndatasize": "returndata", | ||
"returndatacopy": "returndata", | ||
"balance": "balance", | ||
"selfbalance": "balance", | ||
"log": "memory", | ||
"revert": "memory", | ||
"return": "memory", | ||
"sha3": "memory", | ||
} | ||
|
||
|
||
@dataclass | ||
class Fence: | ||
storage: int = 0 | ||
memory: int = 0 | ||
transient: int = 0 | ||
immutables: int = 0 | ||
balance: int = 0 | ||
returndata: int = 0 | ||
|
||
|
||
# effects graph | ||
class EffectsG: | ||
def __init__(self): | ||
self._graph = defaultdict(list) | ||
|
||
# not sure if this will be useful | ||
self._outputs = defaultdict(list) | ||
|
||
def analyze(self, bb): | ||
fence = Fence() | ||
|
||
read_groups = {} | ||
terms = {} | ||
|
||
for inst in bb.instructions: | ||
reads = _get_reads(inst.opcode) | ||
writes = _get_writes(inst.opcode) | ||
for eff in reads: | ||
fence_id = getattr(fence, eff) | ||
group = read_groups.setdefault((eff, fence_id), []) | ||
group.append(inst) | ||
|
||
# collect writes in a separate dict | ||
for eff in writes: | ||
fence_id = getattr(fence, eff) | ||
assert (eff, fence_id) not in terms | ||
terms[(eff, fence_id)] = inst | ||
|
||
fence = _compute_fence(inst.opcode, fence) | ||
|
||
for (effect, fence_id), write_inst in terms.items(): | ||
reads = read_groups.get((effect, fence_id), []) | ||
for read in reads: | ||
if read == write_inst: | ||
continue | ||
self._graph[write_inst].append(read) | ||
|
||
next_id = fence_id + 1 | ||
|
||
next_write = terms.get((effect, next_id)) | ||
if next_write is not None: | ||
self._graph[next_write].append(write_inst) | ||
|
||
next_reads = read_groups.get((effect, next_id), []) | ||
for inst in next_reads: | ||
self._graph[inst].append(write_inst) | ||
|
||
# invert the graph, go the other way | ||
for inst, dependencies in self._graph.items(): | ||
# sanity check the graph | ||
assert inst not in dependencies, inst | ||
for target in dependencies: | ||
self._outputs[target].append(inst) | ||
|
||
def required_by(self, inst): | ||
return self._graph.get(inst, []) | ||
|
||
def downstream_of(self, inst): | ||
return self._outputs.get(inst, []) | ||
|
||
|
||
def _get_reads(opcode): | ||
ret = reads.get(opcode, ()) | ||
if not isinstance(ret, tuple): | ||
ret = (ret,) | ||
return ret | ||
|
||
|
||
def _get_writes(opcode): | ||
ret = writes.get(opcode, ()) | ||
if not isinstance(ret, tuple): | ||
ret = (ret,) | ||
return ret | ||
|
||
|
||
def _compute_fence(opcode: str, fence: Fence) -> Fence: | ||
if opcode not in writes: | ||
return fence | ||
|
||
effects = _get_writes(opcode) | ||
|
||
tmp = asdict(fence) | ||
for eff in effects: | ||
tmp[eff] += 1 | ||
|
||
return Fence(**tmp) | ||
|
||
|
||
class DFTPass(IRPass): | ||
function: IRFunction | ||
inst_order: dict[IRInstruction, int] | ||
inst_order_num: int | ||
|
||
def _process_instruction_r(self, bb: IRBasicBlock, inst: IRInstruction, offset: int = 0): | ||
def _process_instruction_r(self, bb: IRBasicBlock, inst: IRInstruction): | ||
if inst.parent != bb: | ||
return | ||
if inst in self.done: | ||
return | ||
|
||
for op in inst.get_outputs(): | ||
assert isinstance(op, IRVariable), f"expected variable, got {op}" | ||
uses = self.dfg.get_uses(op) | ||
|
||
for uses_this in uses: | ||
if uses_this.parent != inst.parent or uses_this.fence_id != inst.fence_id: | ||
# don't reorder across basic block or fence boundaries | ||
continue | ||
|
||
# if the instruction is a terminator, we need to place | ||
# it at the end of the basic block | ||
# along with all the instructions that "lead" to it | ||
self._process_instruction_r(bb, uses_this, offset) | ||
for use in reversed(uses): | ||
self._process_instruction_r(bb, use) | ||
|
||
if inst in self.visited_instructions: | ||
if inst in self.started: | ||
return | ||
self.visited_instructions.add(inst) | ||
self.inst_order_num += 1 | ||
|
||
if inst.is_bb_terminator: | ||
offset = len(bb.instructions) | ||
self.started.add(inst) | ||
|
||
if inst.opcode == "phi": | ||
# phi instructions stay at the beginning of the basic block | ||
# and no input processing is needed | ||
# bb.instructions.append(inst) | ||
self.inst_order[inst] = 0 | ||
if inst.opcode in ("phi", "param"): | ||
return | ||
|
||
for op in inst.get_input_variables(): | ||
target = self.dfg.get_producing_instruction(op) | ||
assert target is not None, f"no producing instruction for {op}" | ||
if target.parent != inst.parent or target.fence_id != inst.fence_id: | ||
# don't reorder across basic block or fence boundaries | ||
continue | ||
self._process_instruction_r(bb, target, offset) | ||
self._process_instruction_r(bb, target) | ||
|
||
self.inst_order[inst] = self.inst_order_num + offset | ||
for target in self._effects_g.required_by(inst): | ||
self._process_instruction_r(bb, target) | ||
|
||
def _process_basic_block(self, bb: IRBasicBlock) -> None: | ||
self.function.append_basic_block(bb) | ||
bb.instructions.append(inst) | ||
self.done.add(inst) | ||
|
||
for inst in bb.instructions: | ||
inst.fence_id = self.fence_id | ||
if inst.is_volatile: | ||
self.fence_id += 1 | ||
|
||
# We go throught the instructions and calculate the order in which they should be executed | ||
# based on the data flow graph. This order is stored in the inst_order dictionary. | ||
# We then sort the instructions based on this order. | ||
self.inst_order = {} | ||
self.inst_order_num = 0 | ||
for inst in bb.instructions: | ||
def _process_basic_block(self, bb: IRBasicBlock) -> None: | ||
self._effects_g = EffectsG() | ||
self._effects_g.analyze(bb) | ||
|
||
instructions = bb.instructions.copy() | ||
bb.instructions = [inst for inst in bb.instructions if inst.opcode in ("phi", "param")] | ||
|
||
# start with out liveness | ||
#if len(bb.cfg_out) > 0: | ||
if False: | ||
next_bb = bb.cfg_out.first() | ||
target_stack = self.liveness.input_vars_from(bb, next_bb) | ||
|
||
for var in reversed(list(target_stack)): | ||
inst = self.dfg.get_producing_instruction(var) | ||
self._process_instruction_r(bb, inst) | ||
|
||
for inst in instructions: | ||
self._process_instruction_r(bb, inst) | ||
|
||
bb.instructions.sort(key=lambda x: self.inst_order[x]) | ||
def key(inst): | ||
if inst.is_bb_terminator: | ||
return 2 | ||
return 1 | ||
|
||
bb.instructions.sort(key=key) | ||
|
||
# sanity check: the instructions we started with are the same | ||
# as we have now | ||
assert set(bb.instructions) == set(instructions), (instructions, bb) | ||
|
||
def run_pass(self) -> None: | ||
self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) | ||
self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) # use out_vars | ||
|
||
self.fence_id = 0 | ||
self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() | ||
self.started: OrderedSet[IRInstruction] = OrderedSet() | ||
self.done: OrderedSet[IRInstruction] = OrderedSet() | ||
|
||
basic_blocks = list(self.function.get_basic_blocks()) | ||
|
||
self.function.clear_basic_blocks() | ||
for bb in basic_blocks: | ||
for bb in self.function.get_basic_blocks(): | ||
self._process_basic_block(bb) | ||
|
||
# for repr | ||
self.analyses_cache.force_analysis(LivenessAnalysis) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
from vyper.venom.analysis.cfg import CFGAnalysis | ||
from vyper.venom.analysis.dfg import DFGAnalysis | ||
from vyper.venom.analysis.liveness import LivenessAnalysis | ||
from vyper.venom.basicblock import IRInstruction | ||
from vyper.venom.passes.base_pass import IRPass | ||
|
||
|
||
class StoreExpansionPass(IRPass): | ||
""" | ||
This pass expands variables to their uses though `store` instructions, | ||
reducing pressure on the stack scheduler | ||
""" | ||
|
||
def run_pass(self): | ||
dfg = self.analyses_cache.request_analysis(DFGAnalysis) | ||
|
||
self.analyses_cache.request_analysis(CFGAnalysis) | ||
liveness = self.analyses_cache.force_analysis(LivenessAnalysis) | ||
Check notice Code scanning / CodeQL Unused local variable Note
Variable liveness is not used.
|
||
|
||
for bb in self.function.get_basic_blocks(): | ||
if len(bb.instructions) == 0: | ||
continue | ||
|
||
for var in bb.instructions[0].liveness: | ||
self._process_var(dfg, bb, var, 0) | ||
|
||
for idx, inst in enumerate(bb.instructions): | ||
if inst.output is None: | ||
continue | ||
|
||
self._process_var(dfg, bb, inst.output, idx + 1) | ||
|
||
bb.instructions.sort(key=lambda inst: inst.opcode not in ("phi", "param")) | ||
|
||
self.analyses_cache.invalidate_analysis(LivenessAnalysis) | ||
self.analyses_cache.invalidate_analysis(DFGAnalysis) | ||
|
||
def _process_var(self, dfg, bb, var, idx): | ||
""" | ||
Process a variable, allocating a new variable for each use | ||
and copying it to the new instruction | ||
""" | ||
uses = dfg.get_uses(var) | ||
|
||
_cache = {} | ||
Check notice Code scanning / CodeQL Unused local variable Note
Variable _cache is not used.
|
||
|
||
for use_inst in uses: | ||
if use_inst.opcode == "phi": | ||
continue | ||
if use_inst.parent != bb: | ||
continue | ||
|
||
for i, operand in enumerate(use_inst.operands): | ||
if operand == var: | ||
new_var = self.function.get_next_variable() | ||
new_inst = IRInstruction("store", [var], new_var) | ||
bb.insert_instruction(new_inst, idx) | ||
use_inst.operands[i] = new_var |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check warning
Code scanning / CodeQL
Unreachable code Warning