Hey All,
I've come across what I believe to be a bug in MemorySSA. George, I wasn't sure if this was a known issue that you'll be addressing in your upcoming walker caching changes or not, so I haven't investigated it very much. The test case is attached. The bug is that the defining access for the second load is set to the loop MemoryPhi node instead of being liveOnEntry as it should be as I understand things.
memssa-bug-test.patch (1.48 KB)
This is definitely a caching bug related to this code:
1038 // Don’t try to optimize this phi again if we’ve already tried to do so.
1039 if (!Q.Visited.insert(PHIPair).second) {
1040 ModifyingAccess = CurrAccess;
1041 break;
1042 }
We don’t differentiate elsewhere between having stopped at a phi because we were path walking and discovered that path has no conflicts, and having stopped at a phi because that was the clobbering access.
Because of this, we cache the result as if it was a clobbering access, when it really means “you can ignore this path”.
The logic in the code that handles the former case only exists after the cache lookups, and so the first use properly detects that it can ignore this phi, and the second use , which just hits the cache, cannot.
Danny’s right; the “we need to cache everything immediately” behavior doesn’t play very nicely with loops.
The new walker (which should be out for review later today) correctly optimizes %loadG1_1 and %loadG1_2 to liveOnEntry. 
George