gematria icon indicating copy to clipboard operation
gematria copied to clipboard

Add support adding context instructions to basic block graphs.

Open virajbshah opened this issue 11 months ago • 2 comments

  • Update the graph builder and its Python bindings to add context instructions to basic block graphs and store context node mask to later be used by models.
  • Add tests for the new graph builder functionality.

virajbshah avatar Jan 19 '25 22:01 virajbshah

I have made some newer changes to the way context_node_mask_ is populated. Previously, only nodes having NodeType::kInstruction were ensured to have their corresponding entries in context_node_mask_ set correctly, as only these values mattered for usage in downstream models (and not checking the other values in BasicBlockGraphBuilderTest::MultipleBasicBlocksWithContext was to reflect this).

After this change, this behavior is better defined with context_node_mask_ set to true for non-instruction nodes connected exclusively to context instruction nodes.

Please have a look.

virajbshah avatar Feb 08 '25 19:02 virajbshah

I have made some newer changes to the way context_node_mask_ is populated. Previously, only nodes having NodeType::kInstruction were ensured to have their corresponding entries in context_node_mask_ set correctly, as only these values mattered for usage in downstream models (and not checking the other values in BasicBlockGraphBuilderTest::MultipleBasicBlocksWithContext was to reflect this).

After this change, this behavior is better defined with context_node_mask_ set to true for non-instruction nodes connected exclusively to context instruction nodes.

Please have a look.

I've added one technical comment, but otherwise this makes sense.

ondrasej avatar Feb 17 '25 14:02 ondrasej