Skip to content

Conversation

@asl
Copy link
Contributor

@asl asl commented Mar 8, 2023

Extend activity analysis to handle try_apply normal result properly.

asl added 2 commits March 7, 2023 22:04
…turn value.

Extend activity analysis to handle `try_apply` normal result properly.
@asl asl requested review from dan-zheng and rxwei March 8, 2023 06:05
@asl
Copy link
Contributor Author

asl commented Mar 8, 2023

Tagging @BradLarson

@asl
Copy link
Contributor Author

asl commented Mar 8, 2023

@swift-ci please test

TRohit20

This comment was marked as off-topic.

// an `Optional`-typed value. `switch_enum` instructions require
// special-case adjoint value propagation for the operand.
auto isSwitchEnumInstOnOptional =
if (bbArg->getSingleTerminatorOperands(incomingValues)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do an early return here to reduce nesting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems we'd need to eventually handle else case since it at least covers try_apply instruction among the others. The fix for activity calculation was pretty straightforward, but I will need to think how to proceed with adjoint propagation. I do not have a testcase for this right now, but hopefully @BradLarson will be able to distill some from their codebase.

For now we just do not silently generate incorrect code :)

@dan-zheng
Copy link
Contributor

(A bit too busy to review this week – cheers!)

@asl asl merged commit 869fc17 into swiftlang:main Mar 8, 2023
@asl asl deleted the diff-try-apply branch March 8, 2023 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants