byuccl / bfasst

Tools for FPGA Assurance Flows
Apache License 2.0
12 stars 4 forks source link

Refactor create_rule_snippets #464

Closed KeenanRileyFaulkner closed 2 months ago

KeenanRileyFaulkner commented 2 months ago

Currently, each tool that inherits from the Tool class in tool.py must have its own create_rule_snippets() method. However, they are the same for each one:

def create_rule_snippets(self):
        self._append_rule_snippets_default(__file__)

The reason this must be done in each tool individually is because __file__ is used to piece together the name of the tool's rule snippet mustache file:

def _append_rule_snippets_default(self, py_tool_path, render_dict=None, rules_path=None):
        """Create the rule snippets for a python tool,
        assuming default filenames are used
        """

        py_tool_path = pathlib.Path(py_tool_path)

        if rules_path is None:
            if render_dict:
                rules_path = py_tool_path.parent / (py_tool_path.stem + "_rules.ninja.mustache")
            else:
                rules_path = py_tool_path.parent / (py_tool_path.stem + "_rules.ninja")

        if rules_path in self.flow.rule_paths:
            return

        self.flow.rule_paths.append(rules_path)

        with open(rules_path, "r") as f:
            if render_dict:
                rules = chevron.render(f, render_dict)
            else:
                rules = f.read()

        with open(NINJA_BUILD_PATH, "a") as f:
            f.write(rules)

A better approach would be for each tool that inherits from Tool to explicitly specify the names of its associated rule snippet file, and the Tool class create_rule_snippets() method to specifically reference them. For example

# in vivado_synth.py
class VivadoSynth(SynthTool):
    """Tool to create vivado synthesis ninja snippets."""

    def __init__(self, flow, design_path, ooc=False, synth_options=""):
        super().__init__(flow, design_path, ooc=ooc)
        # ...
        self.rule_snippet_file = "tools/common/vivado_rules.ninja.mustache"

In some cases, like the one above for the vivado_synth tool, we would also need to save a render_dict as part of self. The redundant declaration of the create_rule_snippets() method in each tool could then be removed. If storing this render_dict as part of self is undesirable, then the tools that are required to template their rules files would remain unchanged, but the ones that do not require chevron to template the files could have the create_rule_snippets() removed.