Skip to content

Transform inst(X, ...) to op(X, ...) plus macro(X) = X in code generator #109287

Closed

Description

@gvanrossum

This should make it easier for all further passes in the code generator -- they can just look at macros, ignoring the singleton instructions. @markshannon tried this in #108997 but got stuck, I hope I can get it unstuck.

It's possible that the synthetic uop will be named __X instead of _X, since the latter would create some name conflicts. (I don't think that this causes issues with __X being treated as "private" in Python, since we won't have Python-level variables named after uops.) (UPDATE: The synthetic uop and macro will both be called X.)

Linked PRs

Activity

  1. gvanrossum commented on Sep 11, 2023

    @gvanrossum
    MemberAuthor

    This is even subtler than I thought.

    • This adds a lot of new (uop) opcodes, I had to change the number for the lowest uop from 300 to 275.
      It might be better to reuse the Tier 1 opcodes for the Tier 2 opcodes; I haven't figured out how yet.
    • All existing (hand-written) macros have to reference other opcodes as __X, which makes things ugly.
    • There's some special-casing for super-instructions; you can't just say macro(LOAD_FAST_LOAD_FAST) = LOAD_FAST + LOAD_FAST (because these need special-casing for oparg).

    And the savings in code are not that spectacular.

  2. gvanrossum commented on Sep 12, 2023

    @gvanrossum
    MemberAuthor

    I am considering a different approach, where inst(X) translates to op(X) and macro(X) = X, so the macro name is the same as the op name. The Tier 1 pass generator can just walk over self.macro_instrs. The Tier 2 pass generator can just walk over self.instrs. The metadata pass will need to consider both (and others, self.families and self.pseudos).

    PRIORITY: There's still an open issue for super-instructions (I think we will have to add something to the syntax).

  3. gvanrossum commented on Sep 13, 2023

    @gvanrossum
    MemberAuthor

    @markshannon @brandtbucher This would be easier if we dropped the whole concept of super-instructions. There are only three left (LOAD_FAST_LOAD_FAST, STORE_FAST_LOAD_FAST, STORE_FAST_STORE_FAST). But when these were introduced, they gave a whopping 1.7% speedup (gh-105230).

  4. gvanrossum commented on Sep 13, 2023

    @gvanrossum
    MemberAuthor

    Well, I managed to add the super-instruction special casing back. [UPDATE:] I also managed to reuse the opcode names and numbers for both the op and the macro, reducing the strain on the opcode namespace.

  5. changed the title [-]Transform inst(X, ...) to op(_X, ...) plus macro(X) = _X in code generator[/-] [+]Transform inst(X, ...) to op(X, ...) plus macro(X) = X in code generator[/+] on Sep 14, 2023
  6. added a commit that references this issue on Sep 15, 2023
  7. gvanrossum commented on Sep 15, 2023

    @gvanrossum
    MemberAuthor

    @carljm I just discovered I introduced a bug on the handling of overrides -- an overridden inst() will be emitted twice (because there are two macro instances in everything). I'm about to head off on vacation but maybe you or @jbower-fb can come up with a fix? FWIW I am getting tired of the everything list, and would rather have various code bits iterate over self.instrs, self.macros, etc. Maybe we can tolerate not having the placeholder comment in the generated output? That would simplify a few things.

  8. gvanrossum commented on Sep 15, 2023

    @gvanrossum
    MemberAuthor

    (One idea before I really go: the Macro could have a copy of the override flag, and this could be used to just add // override to the emitted cases. Or the code could just look in the first component for an override flag.)

  9. gvanrossum commented on Oct 2, 2023

    @gvanrossum
    MemberAuthor

    @carljm @jbower-fb Did you manage to confirm and fix the bug with overrides I referred to above?

  10. carljm commented on Oct 4, 2023

    @carljm
    Member

    @gvanrossum No, sorry; I've had it noted on my list of things to look into, but haven't gotten to it yet. I should be able to work on it this week.

  11. added a commit that references this issue on Oct 5, 2023
  12. linked a pull request that will close this issuegh-109287: fix overrides in cases generator #110419on Oct 5, 2023
  13. added a commit that references this issue on Oct 5, 2023
  14. jbower-fb commented on Nov 3, 2023

    @jbower-fb
    Contributor

    (Sorry I didn't comment on this before, I still care about this feature but was taking some extended leave from work/Python to look after my new-born son.)

  15. added a commit that references this issue on Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions