Skip to content

Conversation

MAJKFL
Copy link
Contributor

@MAJKFL MAJKFL commented Sep 23, 2025

Before attempting load splitting, licm needs to check the projection path is actually materializable. By bailing earlier, we ensure wider stores are not sank without the narrower load projection.

@MAJKFL MAJKFL requested a review from eeckstein as a code owner September 23, 2025 14:12
@MAJKFL
Copy link
Contributor Author

MAJKFL commented Sep 23, 2025

@swift-ci Please smoke test

return accessPath.isEqualOrContains(self.operand.value.accessPath) || self.operand.value.accessPath.isEqualOrContains(accessPath)
// If store is wider than load, it needs to be materializable.
// Otherwise we won't be able to project it.
return (accessPath.getProjection(to: self.operand.value.accessPath)?.isMaterializable ?? false) ||
Copy link
Contributor

@eeckstein eeckstein Sep 23, 2025

Choose a reason for hiding this comment

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

nitpick: IMO it's more readable to split this complex condition into multiple statements, e.g:

  if let path = accessPath.getProjection(to: self.operand.value.accessPath),
    // If the accessPath is wider than load, it needs to be materializable.
    // Otherwise we won't be able to project it.
    path. isMaterializable
  {
    // The load is narrower than the access path.
    return true
  }
  if self.operand.value.accessPath.isEqualOrContains(accessPath) {
    // The load is wider than the access path.
    return true
  }
  return false

@MAJKFL MAJKFL force-pushed the fix-licm-missing-earlier-materializable-projection-check branch from a83bb70 to 6788017 Compare September 24, 2025 10:50
@MAJKFL
Copy link
Contributor Author

MAJKFL commented Sep 24, 2025

@swift-ci Please smoke test

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm!

@MAJKFL MAJKFL enabled auto-merge September 24, 2025 14:13
@MAJKFL MAJKFL merged commit aebebd9 into swiftlang:main Sep 24, 2025
3 checks passed
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.

2 participants