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

[InstCombine] Taking an order of magnitude longer in 12.0.0 than in 10.0.0 for some examples. #49499

Closed
mcandress mannequin opened this issue Apr 28, 2021 · 10 comments
Closed
Labels
bugzilla Issues migrated from bugzilla

Comments

@mcandress
Copy link
Mannequin

mcandress mannequin commented Apr 28, 2021

Bugzilla Link 50155
Resolution FIXED
Resolved on Jun 14, 2021 16:23
Version unspecified
OS Linux
Blocks #48661
Attachments gzipped ir module exhibiting much slower InstCombine optimization pass.
CC @aqjune,@RKSimon,@nikic,@rotateright,@tstellar
Fixed by commit(s) 2cd7868 ce77909

Extended Description

For some functions, the --instcombine optimization takes excessively longer with llvm 12.0.0 than it does with llvm 10.0.0.

In the attached example, a run of "opt -o=out.o --instcombine test_instcomb.ir" takes 53 seconds, where it used to take around 2 seconds when version 10.0.0 was used (albeit version 12.0.0 combines more instructions).

Is there a way to mitigate this speed performance degradation when using the library interface (i.e. from C++ library API, not from the opt tool)?

@nikic
Copy link
Contributor

nikic commented Apr 29, 2021

Looks like most of the time is spent in isGuaranteedNotToBeUndefOrPoison() -> programUndefinedIfUndefOrPoison().

@nikic
Copy link
Contributor

nikic commented Apr 29, 2021

for (auto &I : make_range(Begin, End)) {
SmallPtrSet<const Value *, 4> WellDefinedOps;
getGuaranteedWellDefinedOps(&I, WellDefinedOps);
for (auto *Op : WellDefinedOps) {
if (Op == V)
return true;
}
if (!isGuaranteedToTransferExecutionToSuccessor(&I))
break;
}
performs an instruction scan from the instruction to the end of the block. The test case contains one very large basic block, so this scan ends up looking at approximately 100000 instructions. This obviously needs a limit.

@nikic
Copy link
Contributor

nikic commented Apr 30, 2021

Fixed on main by 2cd7868.

This should probably be backported to LLVM 12.0.1. I don't believe there is any way you can work around this.

@tstellar
Copy link
Collaborator

tstellar commented May 3, 2021

Fixed on main by
https://github.com/llvm/llvm-project/commit/
2cd7868.

This should probably be backported to LLVM 12.0.1. I don't believe there is
any way you can work around this.

Was there a pre-commit review for this patch?

@tstellar
Copy link
Collaborator

tstellar commented Jun 5, 2021

I couldn't find a phabricator review for this, so I would like to have someone else review it before it is backported.

@aqjune
Copy link
Contributor

aqjune commented Jun 5, 2021

I'm the main author of the function programUndefinedIfUndefOrPoison and the change looks good to me.

@tstellar
Copy link
Collaborator

tstellar commented Jun 8, 2021

I'm the main author of the function programUndefinedIfUndefOrPoison and the
change looks good to me.

Thank you.

@tstellar
Copy link
Collaborator

tstellar commented Jun 9, 2021

The fix does not apply cleanly, could someone backport this and push a branch to their local github fork?

@aqjune
Copy link
Contributor

aqjune commented Jun 10, 2021

Would aqjune@89d5009 be okay?

@tstellar
Copy link
Collaborator

Merged: ce77909

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

No branches or pull requests

3 participants