This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Use CFG reasoning to filter potentially interfering writes
ClosedPublic

Authored by jdoerfert on Jul 20 2021, 2:17 PM.

Details

Summary

Since D104432 we can look through memory by analyzing all writes that
might interfere with a load. This patch provides some logic to exclude
writes that cannot interfere with a location, due to CFG reasoning.
We make sure to avoid multi-thread write-read situations properly while
we ignore writes that cannot reach a load or writes that will be
overwritten before the load is reached.

Diff Detail

Event Timeline

jdoerfert created this revision.Jul 20 2021, 2:17 PM
jdoerfert requested review of this revision.Jul 20 2021, 2:17 PM
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: bbn. · View Herald Transcript
kuter added inline comments.Jul 25 2021, 11:40 PM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
1158

I don't understand why we can ignore a store here.

Even if Acc dominates a store here doesn't mean that there can't be path from Acc that
doesn't go through DomAcc right ? it only means that the execution must pass through
Acc before reaching DomAcc or am I confused ?

Wouldn't it make more sense to use the post-dominator tree here ?

kuter added inline comments.Jul 25 2021, 11:54 PM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
1158

Oh I get it now. There is no path from Acc to load that doesn't go through the DomAcc because DomAcc dominates the load.

Very interesting. Some comments could be added here : - )

jdoerfert added inline comments.Jul 26 2021, 7:50 AM
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
1158

Yes, I will add some comment. You got it though,

 Acc
  |        // dominance
DomAcc
  |        // dominance
 load

means we have to go through DomAcc before we reach load after we executed Acc. So load won't see Acc.

kuter accepted this revision.Aug 11 2021, 8:06 AM

LGTM, maybe a test could be great.

This revision is now accepted and ready to land.Aug 11 2021, 8:06 AM
This revision was landed with ongoing or failed builds.Jan 31 2022, 11:26 PM
This revision was automatically updated to reflect the committed changes.