[arc-opts] Teach IsAddressWrittenToDefUseAnalysis how to track "well behaved" writes but don't use it. #30741
Conversation
|
@swift-ci test |
| private: | ||
| bool isWrittenToHelper(SILValue value); | ||
| struct IsAddressWrittenToDefUseAnalysis | ||
| : public SmallMultiMapCache<IsAddressWrittenToDefUseAnalysis, SILValue, |
There was a problem hiding this comment.
Please explain how the Analysis is supposed to be viewed as a cache. What does the SmallMultiMapCache interface mean when it's inherited by this analysis. Or just don't publicly inherit a generic ADT and give the analysis its own interface.
| @@ -1659,7 +1677,8 @@ class StorageGuaranteesLoadVisitor | |||
|
|
|||
| // If we have a modify, check if our value is /ever/ written to. If it is | |||
| // never actually written to, then we convert to a load_borrow. | |||
| return answer(ARCOpt.isAddressWrittenToDefUseAnalysis(access)); | |||
| auto result = ARCOpt.isAddressWrittenToDefUseAnalysis.get(access); | |||
| return answer(!result.hasValue() || result.getValue().size()); | |||
There was a problem hiding this comment.
I can't make sense of this code since it's written in terms of the multi-map interface. What does it mean for the analysis to have or not have a value, and how is that different from it being empty or having multiple elements?
|
Backing up, my main issue here is "multi-map cache", which was in a different PR. A cache cannot be a multi-map by definition. Either the analysis has a valid result or it does not. If the result is comprised of several values, that is distinct from whether the result is valid or complete. |
… of CRTP. This makes it so one uses a passed in std::function, instead of an impl class to map a key to a list of values to be cached.
…behaved" writes but don't do use it. In a future commit, I am going to build on this to promote load [copy] -> load_borrow from inout arguments where none of the writes overlap the load [copy]'s result's lifetime. By committing this separately, I am using the current pass's logic to validate the change.
gottesmm
force-pushed
the
pr-961dc8ea62390b2c78cdfafd88f3d04ebfcb7ade
branch
from
c665b21 to
3eda1ea
Compare
|
@atrick I fixed the cache as per our discussion. |
|
@swift-ci test |
|
Windows failed in a c++ interior test. Not this commit. |
In a future commit, I am going to build on this to promote load [copy] ->
load_borrow from inout arguments where none of the writes overlap the load
[copy]'s result's lifetime. By committing this separately, I am using the
current pass's logic to validate the change.