Skip to content

Simplify terminator integer condition#1809

Merged
mshinwell merged 4 commits intooxcaml:mainfrom
xclerc:simplify-terminator-int-condition
Jul 4, 2024
Merged

Simplify terminator integer condition#1809
mshinwell merged 4 commits intooxcaml:mainfrom
xclerc:simplify-terminator-int-condition

Conversation

@xclerc
Copy link
Copy Markdown
Contributor

@xclerc xclerc commented Sep 6, 2023

We can sometimes statically determine the
destination of a conditional terminator, and
thus change it to a mere jump.

This pull request does so if the following case:

  • a first block ends with an Iconst_int
    instruction followed by an Always terminator;
  • the next block has an empty body and a
    terminator which is an integer condition
    over the same register used by the
    Iconst_int instruction;

by changing the destination of the Always terminator.
(ASCII art available at #1782)

Fixes #1782

@xclerc xclerc force-pushed the simplify-terminator-int-condition branch from 908ad14 to 2bec637 Compare September 6, 2023 12:52
@xclerc
Copy link
Copy Markdown
Contributor Author

xclerc commented Sep 6, 2023

This optimization does not fire that
much on the compiler distribution:

  • flambda2: 22 hits for Truth_test;
  • flambda1: 1 hit for Parity_test and 165 hits for Int_test;
  • closure: 105 hits for Int_test.

@xclerc xclerc removed the request for review from mshinwell September 6, 2023 13:25
@mshinwell mshinwell requested review from Ekdohibs and removed request for gretay-js July 3, 2024 16:39
@Ekdohibs
Copy link
Copy Markdown
Contributor

Ekdohibs commented Jul 4, 2024

This looks good, but could we extend it a bit to the case where the block has multiple assignments? Something like a block ending in reg1 = 0, reg2 = 0 followed by a test on reg1 (currently only a test on reg2 would be optimized), or even a block containing reg1 = 0 then no further modification to reg1?

@xclerc
Copy link
Copy Markdown
Contributor Author

xclerc commented Jul 4, 2024

I agree in principle, but would be tempted to
merge as-in and register your suggestion in
an issue. Does that sound OK?

@mshinwell
Copy link
Copy Markdown
Collaborator

That sounds like the right course of action to me - these PRs are overdue for merging as-is.

@Ekdohibs
Copy link
Copy Markdown
Contributor

Ekdohibs commented Jul 4, 2024

Good with me, we can do that later indeed.

@mshinwell mshinwell merged commit 3141c41 into oxcaml:main Jul 4, 2024
bclement-ocp added a commit to bclement-ocp/oxcaml that referenced this pull request Jul 4, 2024
`Poll_and_jump` was removed in oxcaml#1927 but the recent merge of oxcaml#1809 tries
to use it.
Ekdohibs pushed a commit to Ekdohibs/flambda-backend that referenced this pull request Jul 5, 2024
* Statically evaluate terminators for conditions over integer values.

* Eliminate dead blocks later since simplify_terminator can create more opportunities.

* ocamformat

* (debug) Partially revert Eliminate_dead_code move.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify terminator when destination is known

3 participants