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

RegScavenger (as used by BranchFolding) pessimises liveness #28516

Open
ahmedbougacha opened this issue Jun 15, 2016 · 5 comments
Open

RegScavenger (as used by BranchFolding) pessimises liveness #28516

ahmedbougacha opened this issue Jun 15, 2016 · 5 comments
Labels
bugzilla Issues migrated from bugzilla llvm:codegen

Comments

@ahmedbougacha
Copy link
Member

Bugzilla Link 28142
Version trunk
OS All
CC @ahmedbougacha,@MatzeB

Extended Description

This came up in http://reviews.llvm.org/D21085; on the original testcase:
Looks like BranchFolding, when merging tails, indiscriminately marks every register defined but not killed (it might really be live, but not necessarily) before the tail as live-in the new tail block. It queries the RegisterScavenger, which walks the original block forward, using kill flags; maybe it should walk the tail block backward instead?

That patch adds forward liveness computation to X86FixupBWInsts (using flags, via LivePhysRegs) as a stopgap.

Instead, seems like we should:

  • teach RegScavenger to compute liveness backwards (or replace it with something else)
  • fix the various users; in this case, BranchFolding
  • remove the X86FixupBWInsts workaround, and probably more
@MatzeB
Copy link
Contributor

MatzeB commented Jul 7, 2016

This looks like a duplicate of llvm/llvm-bugzilla-archive#28295 to me.

@MatzeB
Copy link
Contributor

MatzeB commented Jul 7, 2016

Unfortunately the testcase of r272797 just codifies the bad live-in list in a .mir test so I cannot use it to whether the revision is not necessary any longer after http://reviews.llvm.org/D22027 any tips in reproducing the problem?

(I am on a question to kill the kill flags which means we cannot use forward liveness simulation any longer).

@MatzeB
Copy link
Contributor

MatzeB commented Jul 9, 2016

I proposed http://reviews.llvm.org/D22083 which in my tests produces identical code for the llvm test-suite including all externals.

@ahmedbougacha
Copy link
Member Author

I think Kevin said this was seen on SPEC, so no difference in the test-suite is perfect.

@MatzeB
Copy link
Contributor

MatzeB commented Jul 11, 2016

the test-suite optionally allows to plug in Spec95, spec2000 and spec2006 in the Externals directory which I did.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla llvm:codegen
Projects
None yet
Development

No branches or pull requests

2 participants