Order load CSE by memory chain.

src/backend/pass/opt/memory_opt.cpp

@@ -7,8 +7,9 @@
 #include "target/target.h"
 
 namespace rat {
-	Node* MemoryOptPass::effectiveDef(const AliasAnalysis& aa, Node* mem, Node* addr, U32 size) {
-		for(U32 steps = 0; steps < kMaxStoreWalk; ++steps) {
+	Node*
+	MemoryOptPass::effectiveDef(const AliasAnalysis& aa, Node* mem, Node* addr, U32 size, U32& hops) {
+		for(hops = 0; hops < kMaxStoreWalk; ++hops) {
 			StoreNode* s = dyn_cast<StoreNode>(mem);
 			if(!s)
 				break;
@@ -19,6 +20,15 @@ namespace rat {
 		return mem;
 	}
 
+	// two loads of one bucket share an effective def, so the distance back to it
+	// is their order inside a block
+	B32 MemoryOptPass::precedes(const LoadNode* a, const LoadNode* b) const {
+		U32 ha = chainHops[a->getId()], hb = chainHops[b->getId()];
+		if(ha != hb)
+			return ha < hb;
+		return a->getId() < b->getId(); // same memory state, either may represent
+	}
+
 	U32 MemoryOptPass::forwardStores(const AliasAnalysis& aa) {
 		U32 removed = 0;
 		for(LoadNode* l : loads) {
@@ -46,7 +56,7 @@ namespace rat {
 			if(ba < 0 || bb < 0)
 				return false;
 			if(ba == bb)
-				return a->getId() < b->getId(); // same block, no aliasing store between
+				return precedes(a, b); // same block, no aliasing store between
 			return sched.dominates(ba, bb);
 		};
 
@@ -65,13 +75,13 @@ namespace rat {
 			List<LoadNode*>& group = kv.second;
 			if(group.size() < 2)
 				continue;
-			std::sort(group.begin(), group.end(), [&](LoadNode* a, LoadNode* b) {
+			std::sort(group.begin(), group.end(), [&](LoadNode* a, LoadNode* b) -> B32 {
 				I32 ba = sched.blockOf(a), bb = sched.blockOf(b);
 				I32 da = ba < 0 ? -1 : sched.block(ba).domDepth;
 				I32 db = bb < 0 ? -1 : sched.block(bb).domDepth;
 				if(da != db)
 					return da < db;
-				return a->getId() < b->getId();
+				return precedes(a, b);
 			});
 			for(U32 i = 0, e = (U32)group.size(); i < e; ++i) {
 				LoadNode* b = group[i];
@@ -170,9 +180,11 @@ namespace rat {
 			if(LoadNode* l = dyn_cast<LoadNode>(n))
 				loads.push_back(l);
 		defs.assign(fn.idBound(), nullptr);
+		chainHops.assign(fn.idBound(), 0);
 		for(LoadNode* l : loads)
 			if(l->hasUsers())
-				defs[l->getId()] = effectiveDef(aa, l->getMemory(), l->getPointer(), aa.getAccessSize(l));
+				defs[l->getId()] = effectiveDef(
+						aa, l->getMemory(), l->getPointer(), aa.getAccessSize(l), chainHops[l->getId()]);
 
 		U32 removed = 0;
 		removed += forwardStores(aa);

src/backend/pass/opt/memory_opt.h

@@ -38,7 +38,8 @@ namespace rat {
 		static constexpr U32 kMaxStoreWalk = 512;
 
 		// skip back over stores that provably do not alias [addr, addr+size)
-		static Node* effectiveDef(const AliasAnalysis& aa, Node* mem, Node* addr, U32 size);
+		static Node* effectiveDef(const AliasAnalysis& aa, Node* mem, Node* addr, U32 size, U32& hops);
+		B32 precedes(const LoadNode* a, const LoadNode* b) const;
 
 		struct ChainScan {
 			StoreNode* store; // last must-alias store of matching size and type
@@ -67,6 +68,7 @@ namespace rat {
 
 		List<LoadNode*> loads;
 		List<Node*> defs;
+		List<U32> chainHops;
 		std::unordered_map<BucketKey, List<LoadNode*>, BucketKeyHash> buckets;
 	};
 } // namespace rat

src/compiler/test/correctness/custom/memopt_cse_after_inline.c

@@ -0,0 +1,20 @@
+// expect: 0
+// passes: gvn memoryopt inline
+
+// inlining clones the body of get() into main with fresh, higher node ids, so
+// the clone's load of G sits earlier in the memory chain than main's own load
+// of G but carries the larger id
+struct R {
+	long a, b;
+};
+
+static struct R G = {1, 2};
+
+static struct R get(void) { return G; }
+
+int main(void) {
+	struct R r = get();
+	if(r.a != G.a || r.b != G.b)
+		return 1;
+	return 0;
+}