-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Make late_gc_lowering more robust #57380
Conversation
… usage a bit more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me, but having a test would be great.
Maybe you can just manually write one? E.g. one with a phi node and one with a select?
Co-authored-by: Jameson Nash <[email protected]>
for (AllocaInst *Alloca : gc_allocas) { | ||
llvm_dump(Alloca); | ||
} | ||
assert(false && "Expected single alloca"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason that this is an assert, somewhat annoying the message doesn't print on non assert builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a fix to your branch :)
There are cases where we optimize the SRet more than the pass expected so try and handle those. I'm tryin to get a test for this, this is separated from #52850 to make merging both easier