Selective cachereads #21
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wsmoses
requested changes
Nov 1, 2019
…gle function when checking for modrefs
… we can avoid caching loads is correct though
… logic for more conservative detection of uncacheable loads
timkaler
force-pushed
the
selective-cachereads-rebase
branch
from
fe6fc67
to
532cf59
Compare
|
There are two flags to force "always caching" and "never caching" of loads. Let's try not to use them though, especially in any CI tests. I don't think the logic is so conservative as to make it common for loads to be cached unnecessarily. This is ready to be merged. We need to move onto other things that depend on correct load caching, and ought to be separate pull requests. |
wsmoses
reviewed
Nov 5, 2019
enzyme/Enzyme/ActiveVariable.cpp
Outdated
| @@ -152,7 +152,8 @@ bool isIntASecretFloat(Value* val) { | |||
| if (!pointerUse && floatingUse) return true; | |||
| llvm::errs() << *inst->getParent()->getParent() << "\n"; | |||
| llvm::errs() << " val:" << *val << " pointer:" << pointerUse << " floating:" << floatingUse << "\n"; | |||
| assert(0 && "ambiguous unsure if constant or not"); | |||
| //assert(0 && "ambiguous unsure if constant or not"); | |||
| return false; // NOTE(TFK): Return false instead of asset. | |||
There was a problem hiding this comment.
This should error otherwise there is likely incorrect behavior
There was a problem hiding this comment.
Yea didn't intend to include that.
Fixed.
There was a problem hiding this comment.
There's another change I didn't intend to include in EnzymeLogic.cpp --- the change with placeholder values being erased. Can you double check that change. Can just leave it in if its correct.
wsmoses
reviewed
Nov 7, 2019
wsmoses
reviewed
Nov 7, 2019
wsmoses
reviewed
Nov 7, 2019
wsmoses
reviewed
Nov 7, 2019
wsmoses
reviewed
Nov 7, 2019
wsmoses
reviewed
Nov 7, 2019
wsmoses
reviewed
Nov 7, 2019
wsmoses
reviewed
Nov 7, 2019
wsmoses
approved these changes
Nov 7, 2019
ludgerpaehler
pushed a commit
that referenced
this pull request
May 16, 2021
* Enable cache reads by default, which is needed for correctness. * Selectively omit caching for reads whose value does not change after the load instruction. Loads that are modified after the load instruction are called "uncacheable" in the code. * Propagate the uncacheable status of pointer arguments to calls. * readwriteread C test illustrates behavior of code.
ludgerpaehler
pushed a commit
that referenced
this pull request
May 16, 2021
* Enable cache reads by default, which is needed for correctness. * Selectively omit caching for reads whose value does not change after the load instruction. Loads that are modified after the load instruction are called "uncacheable" in the code. * Propagate the uncacheable status of pointer arguments to calls. * readwriteread C test illustrates behavior of code.
ludgerpaehler
pushed a commit
that referenced
this pull request
May 16, 2021
* Enable cache reads by default, which is needed for correctness. * Selectively omit caching for reads whose value does not change after the load instruction. Loads that are modified after the load instruction are called "uncacheable" in the code. * Propagate the uncacheable status of pointer arguments to calls. * readwriteread C test illustrates behavior of code.
wsmoses
pushed a commit
that referenced
this pull request
May 17, 2021
* Enable cache reads by default, which is needed for correctness. * Selectively omit caching for reads whose value does not change after the load instruction. Loads that are modified after the load instruction are called "uncacheable" in the code. * Propagate the uncacheable status of pointer arguments to calls. * readwriteread C test illustrates behavior of code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are a few more tweaks needed before this is ready, e.g. I need to add ordering constraints to the modref analysis within a function (like is done for finding "volatile" arguments), and also handle pointers of unknown origin. I want to write the tests before doing these fixes though.