Fix bug in memdep's load clobber analysis

[• dberlin]

Summary

This patch fixes a currently untriggerable bug in GVN.
In particular, GVN happily calls analyzeLoadFromLoadClobber on any loads given to it by MemDep.
It is supposed to be doing legality checking as to whether it can widen the load.
In particular, it can only widen overaligned integer loads.
The first function it calls in analyzeLoadFromClobberingLoad has no idea this is the case,
and is just being asked to give an offset inside of the load.
It will happily say "sure, we can coerce this load" even when it requires widening.

It has code to check the legality (below the current code), that does in fact, check that it will
later be able to widen it, but this does not get called in a lot of cases

The reason this is untriggerable is because, due to the incestuous relationship GVN has with memdep, memdep
does some GVN legality checking before deciding something is a clobber, because it knows what GVN can widen.
(IE it checks whether it is an overaligned, integer load.)
Whether memdep should do this, etc, not going to fix in this fix.

This bug was found by reusing this code in something that doesn't use memdep :)

[reames]

LGTM

lib/Transforms/Scalar/GVN.cpp

 static int AnalyzeLoadFromClobberingLoad(Type *LoadTy, Value *LoadPtr,
                                          LoadInst *DepLI, const DataLayout &DL){
   // Cannot handle reading from store of first-class aggregate yet.
   if (DepLI->getType()->isStructTy() || DepLI->getType()->isArrayTy())
     return -1;
 
   Value *DepPtr = DepLI->getPointerOperand();
   uint64_t DepSize = DL.getTypeSizeInBits(DepLI->getType());
-  int R = AnalyzeLoadFromClobberingWrite(LoadTy, LoadPtr, DepPtr, DepSize, DL);
-  if (R != -1) return R;
+  int Offset = AnalyzeLoadFromClobberingWrite(LoadTy, LoadPtr, DepPtr, DepSize, DL);
+  if (Offset != -1) {
+    // If the size is too large and we will have to widen, ensure we pass the
+    // widening rules below
+    unsigned SrcValSize = DL.getTypeStoreSize(DepLI->getType());
+    unsigned LoadSize = DL.getTypeStoreSize(LoadTy);
+    if (Offset + LoadSize <= SrcValSize)
+      return Offset;
+  }
 
   // If we have a load/load clobber an DepLI can be widened to cover this load,
   // then we should widen it!
   int64_t LoadOffs = 0;
   const Value *LoadBase =
     GetPointerBaseWithConstantOffset(LoadPtr, LoadOffs, &DL);
   unsigned LoadSize = DL.getTypeStoreSize(LoadTy);