Skip to content

LICM: add an optimization to move multiple loads and stores from/to the same memory location out of a loop. #27990

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

Merged
merged 2 commits into from
Nov 1, 2019

Conversation

eeckstein
Copy link
Contributor

This is a combination of load hoisting and store sinking, e.g.

  preheader:
    br header_block
  header_block:
    %x = load %not_aliased_addr
    // use %x and define %y
    store %y to %not_aliased_addr
    ...
  exit_block:

is transformed to:

  preheader:
    %x = load %not_aliased_addr
    br header_block
  header_block:
    // use %x and define %y
    ...
  exit_block:
    store %y to %not_aliased_addr

This optimization is important to optimize inout arguments, especially with COW support in SIL: it relies on values being in SSA values rather than in memory.

This is the second try to land #27849. It includes the fix which broke the ASAN bot.

Because the set includes all side-effect instructions, also may-reads.
NFC
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

1 similar comment
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

2 similar comments
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

…he same memory location out of a loop.

This is a combination of load hoisting and store sinking, e.g.

  preheader:
    br header_block
  header_block:
    %x = load %not_aliased_addr
    // use %x and define %y
    store %y to %not_aliased_addr
    ...
  exit_block:

is transformed to:

  preheader:
    %x = load %not_aliased_addr
    br header_block
  header_block:
    // use %x and define %y
    ...
  exit_block:
    store %y to %not_aliased_addr
@eeckstein
Copy link
Contributor Author

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 2cf8487b63c60848bc54c78d6cb468cbc39233d9

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 2cf8487b63c60848bc54c78d6cb468cbc39233d9

@eeckstein eeckstein merged commit 74328cd into swiftlang:master Nov 1, 2019
@eeckstein eeckstein deleted the licm branch November 1, 2019 18:00
// If a store just stores the loaded value, bail. The operand (= the load)
// will be removed later, so it cannot be used as available value.
// This corner case is suprisingly hard to handle, so we just give up.
if (isLoadFromAddr(dyn_cast<LoadInst>(SI->getSrc()), addr))
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this isLoadFromAddr was a bug that you found after the first implementation, but I really don't get it. Just remove the store if it's not changing the available value! The SSA updater should get it right if it doesn't see this store as a def.

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.

3 participants