Skip to content

[LICM] Support hoisting of non-argmemonly readonly calls #144497

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jun 17, 2025

The code checking whether a readonly call is safe to hoist is
currently limited to only argmemonly calls. However, the actual
implementation does not depend on this in any way. It either
does an MSSA clobber walk on the memory access (which will take
all locations accessed by the call into account), or it will
look at all MemoryDefs in an entirely location-independent manner.

The current restriction dates back to the time when LICM still
supported AST, in which case this code did reason about the
individual pointer arguments.

nikic added 2 commits June 17, 2025 11:59
The code checking whether a readonly call is safe to hoist is
currently limited to only argmemonly calls. However, the actual
implementation of does not depend on this in anyway. It either
does an MSSA clobber walk on the memory access (which will take
all locations accessed by the call into account), or it will
look at all MemoryDefs in an entirely location-independent manner.

The current restriction dates back to the time when LICM still
supported AST, in which case this code *did* reason about the
individual pointer arguments.
@@ -153,6 +153,6 @@ else: ; preds = %postinvoke

declare void @may_throw()

declare i32 @pure_computation() nounwind argmemonly readonly willreturn
declare i32 @pure_computation() nounwind willreturn memory(none)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argmemonly for a function without arguments is memory(none). Not LICM's job to figure that out...

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants