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

Refine liveness of phi and select #24035

Merged
merged 1 commit into from
Oct 10, 2017
Merged

Refine liveness of phi and select #24035

merged 1 commit into from
Oct 10, 2017

Conversation

yuyichao
Copy link
Contributor

@yuyichao yuyichao commented Oct 7, 2017

If both inputs are rooted then we don't need to root the result.
Rename LoadRefinements to Refinements since it's not load specific anymore.

Fix #22421

This is based on #23993 to reduce conflict.

llvmpasses test will come later.

The handling of LiftPhi and LiftSelect isn't very pretty but hopefully good enough ;-p...

@yuyichao yuyichao requested a review from Keno October 7, 2017 02:59
@yuyichao yuyichao added the compiler:codegen Generation of LLVM IR and native code label Oct 7, 2017
If both inputs are rooted then we don't need to root the result.
Rename `LoadRefinements` to `Refinements` since it's not load specific anymore.

Fix #22421
@yuyichao yuyichao force-pushed the yyc/codegen/select_phi branch from 9362dd0 to 5769cb0 Compare October 9, 2017 17:20
@yuyichao
Copy link
Contributor Author

yuyichao commented Oct 9, 2017

Tests added.

Copy link
Member

@Keno Keno left a comment

Choose a reason for hiding this comment

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

Implementation looks fine to me.

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

Seems good to me. @Keno?

@yuyichao yuyichao merged commit 8ea00cc into master Oct 10, 2017
@yuyichao yuyichao deleted the yyc/codegen/select_phi branch October 10, 2017 02:50
@andreasnoack
Copy link
Member

Bisection suggests that this is causing #24098

@yuyichao
Copy link
Contributor Author

OK, I think the issue is that since the incomming value of phi node doesn't dominate the phi node, we can't simply refine phi to the incomming value since they might not be the "same value" anymore.

I'll try to fix it later (hopefully 1-2 days) and if anyone need it urgently feel free to partially revert by commenting out

for (unsigned i = 0; i < nIncoming; ++i)
RefinedPtr[i] = Number(S, lift->getIncomingValue(i));
and
for (unsigned i = 0; i < nIncoming; ++i)
RefinedPtr[i] = Number(S, Phi->getIncomingValue(i));
(4 lines)

vtjnash added a commit that referenced this pull request Oct 11, 2017
vtjnash added a commit that referenced this pull request Oct 11, 2017
yuyichao pushed a commit that referenced this pull request Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants