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
Refactor the preheader subst map computation in SESE canonicalization. #19324
Conversation
@swift-ci please test tensorflow linux |
@swift-ci please test tensorflow macos |
@@ -316,6 +316,13 @@ class SingleExitLoopTransformer { | |||
|
|||
void initialize(); | |||
|
|||
/// Return a map that captures information about what SILValue should be | |||
/// used at the pre-header of the loop for every SILValue in the given | |||
/// `values` set. If it cannnot find a suitable SILValue for an entry in |
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.
typo: cannot
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.
done
} | ||
// Do not eliminate undefs unless requested for. | ||
if (!TFNoUndefsInSESE) return result; |
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.
Seems to be a double-width space before return
?
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.
done.
@@ -403,14 +423,14 @@ void SingleExitLoopTransformer::initialize() { | |||
|
|||
llvm::DenseMap<SILValue, SILValue> | |||
SingleExitLoopTransformer::computeEscapingValues() const { |
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.
can this be a static method?
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.
Unfortunately no as it relies on some object state.
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.
Thanks for the review.
@@ -403,14 +423,14 @@ void SingleExitLoopTransformer::initialize() { | |||
|
|||
llvm::DenseMap<SILValue, SILValue> | |||
SingleExitLoopTransformer::computeEscapingValues() const { |
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.
Unfortunately no as it relies on some object state.
} | ||
// Do not eliminate undefs unless requested for. | ||
if (!TFNoUndefsInSESE) return result; |
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.
done.
@@ -316,6 +316,13 @@ class SingleExitLoopTransformer { | |||
|
|||
void initialize(); | |||
|
|||
/// Return a map that captures information about what SILValue should be | |||
/// used at the pre-header of the loop for every SILValue in the given | |||
/// `values` set. If it cannnot find a suitable SILValue for an entry in |
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.
done
Refactoring some code that I find useful for subsequent PRs.