Skip to content
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

Remove allocation prefetch experimental code #9038

Closed
0xdaryl opened this issue Mar 31, 2020 · 4 comments · Fixed by #18808
Closed

Remove allocation prefetch experimental code #9038

0xdaryl opened this issue Mar 31, 2020 · 4 comments · Fixed by #18808
Assignees

Comments

@0xdaryl
Copy link
Contributor

0xdaryl commented Mar 31, 2020

There was an experimental allocation prefetch strategy implemented more than a decade ago (I think it was mine!) that was never enabled and never went past the experimental stage. It has long been deprecated. Remove it.

Look for code guarded with TR_EnableNewX86PrefetchTLH and remove dead code produced from that exercise. A followup PR in OMR to remove the option is in order as well.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Mar 31, 2020

@mw918

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Apr 7, 2020

TR_EnableNewX86PrefetchTLH is the internal Option flag that enables the feature to be removed. You should assume that any code that asks whether this feature is enabled will receive false as the answer. For example,
https://github.com/eclipse/openj9/blob/a235102acaeb0cf7dee1bd9590bd714b0daf66eb/runtime/compiler/x/codegen/J9TreeEvaluator.cpp#L6342
the code guarded here should be considered to be always enabled and the else code should be considered dead and can be removed. You can fold this if/else statement based on that logic.

Similarly, in AllocPrefetchSnippet.cpp any code that would become active if this option were to be active is now dead. This means removing the TR_X86newPrefetchTLH enum and the assembler helper code that goes along with it.

Repeat this process of assuming the option is false and removing/folding code accordingly for each reference to this feature you find in the code. Once it has been removed from OpenJ9, you can remove the TR_X86newPrefetchTLH Option from OMR.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented May 2, 2023

@jmesyou : please complete this

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Jan 23, 2024

I asked @BradleyWood to investigate TLH prefetching performance, so this cleanup work could be done as part of that investigation (unless you have a PR imminent for this @jmesyou).

@BradleyWood BradleyWood self-assigned this Jan 23, 2024
BradleyWood added a commit to BradleyWood/openj9 that referenced this issue Jan 23, 2024
Issue: eclipse-openj9#9038

Signed-off-by: Bradley Wood <bradley.wood@ibm.com>
BradleyWood added a commit to BradleyWood/omr that referenced this issue Jan 23, 2024
Issue: eclipse-openj9/openj9#9038

Signed-off-by: Bradley Wood <bradley.wood@ibm.com>
BradleyWood added a commit to BradleyWood/omr that referenced this issue Apr 2, 2024
Issue: eclipse-openj9/openj9#9038

Signed-off-by: Bradley Wood <bradley.wood@ibm.com>
rmnattas pushed a commit to rmnattas/omr that referenced this issue Jun 14, 2024
Issue: eclipse-openj9/openj9#9038

Signed-off-by: Bradley Wood <bradley.wood@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants