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

[ARM] Always lower direct calls as direct when the outliner is enabled #66434

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

jroelofs
Copy link
Contributor

The indirect lowering hinders the outliner's ability to see that sequences are in fact common, since the sequence similarity is rendered opaque by the register callee. The size savings from making them indirect seems to be dwarfed by the outliner's savings from de-duplication.

rdar://115178034
rdar://115459865

The indirect lowering hinders the outliner's ability to see that sequences are
in fact common, since the sequence similarity is rendered opaque by the
register callee.  The size savings from making them indirect seems to be
dwarfed by the outliner's savings from de-duplication.

rdar://115178034
rdar://115459865
@aemerson
Copy link
Contributor

This change looks reasonable to me, but since ARM have a wide corpus of armv7 size sensitive code, could your team evaluate the impact of this change @davemgreen?

Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

Sounds OK to me

@jroelofs jroelofs merged commit 003bcad into llvm:main Sep 15, 2023
1 of 2 checks passed
@jroelofs jroelofs deleted the jroelofs/rdar115459865 branch September 15, 2023 17:05
@davemgreen
Copy link
Collaborator

Sorry - I managed to run the benchmarks wrong (I was surprised it didn't make any difference).

It turns out this does cause some regressions and appears to be worse than before.

@jroelofs
Copy link
Contributor Author

Ok, thanks for checking! I'll revert tomorrow.

@jroelofs jroelofs restored the jroelofs/rdar115459865 branch September 18, 2023 16:41
jroelofs added a commit to jroelofs/llvm-project that referenced this pull request Sep 18, 2023
…s enabled (llvm#66434)"

This reverts commit 003bcad.

ARM folks say it regresses some of their benchmarks:
llvm#66434 (comment)
@jroelofs
Copy link
Contributor Author

reverted in: 83e6d2e

ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
llvm#66434)

The indirect lowering hinders the outliner's ability to see that
sequences are in fact common, since the sequence similarity is rendered
opaque by the register callee. The size savings from making them
indirect seems to be dwarfed by the outliner's savings from
de-duplication.

rdar://115178034
rdar://115459865
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
…s enabled (llvm#66434)"

This reverts commit 003bcad.

ARM folks say it regresses some of their benchmarks:
llvm#66434 (comment)
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
…s enabled (llvm#66434)"

This reverts commit 003bcad.

ARM folks say it regresses some of their benchmarks:
llvm#66434 (comment)
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
…s enabled (llvm#66434)"

This reverts commit 003bcad.

ARM folks say it regresses some of their benchmarks:
llvm#66434 (comment)
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.

None yet

4 participants