[GVN] Remove duplicated canCoerce tests

[vtjnash]

Summary

Now that canCoerceMustAliasedValueToLoad should contain the superset of all requirements for coercion to be legal to consider, we can now remove what should be duplicate tests scattered elsewhere.

[vtjnash]

rebase

[vtjnash]

Removed NFCI, since as I look back, I think this also enabled optimization of constant structures, and I was misusing the NFCI term

llvm/lib/Transforms/Utils/VNCoercion.cpp

 #include "llvm/Transforms/Utils/VNCoercion.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "vncoerce"
 
 namespace llvm {
 namespace VNCoercion {
 
-static bool isFirstClassAggregateOrScalableType(Type *Ty) {
-  return Ty->isStructTy() || Ty->isArrayTy() || isa<ScalableVectorType>(Ty);
+static bool isFirstClassAggregate(Type *Ty) {
+  return Ty->isStructTy() || Ty->isArrayTy();
 }
 
 bool canCoerceMustAliasedTypeToLoad(Type *StoredTy, Type *LoadTy,
                                     const DataLayout &DL) {
   // TODO: Caller expects to be able to check the results for aliasing, which
   // is implemented by checking the sizes. Thus even if we know the types are
   // identical, the code may later crash.
   if (isa<ScalableVectorType>(StoredTy) || isa<ScalableVectorType>(LoadTy))
     return false;
 
   if (StoredTy == LoadTy)
     return true;
 
   // If the loaded/stored value is a first class array/struct, don't try to
   // transform them. We need to be able to bitcast to integer.
-  if (isFirstClassAggregateOrScalableType(LoadTy) ||
-      isFirstClassAggregateOrScalableType(StoredTy))
+  if (isFirstClassAggregate(LoadTy) || isFirstClassAggregate(StoredTy))
     return false;
 
   TypeSize StoreSize = DL.getTypeSizeInBits(StoredTy);
 
   // The store size must be byte-aligned to support future type casts.
   if (!StoreSize.isByteSized())
     return false;
 

 ///
 /// Check this case to see if there is anything more we can do before we give
 /// up.  This returns -1 if we have to give up, or a byte number in the stored
 /// value of the piece that feeds the load.
 static int analyzeLoadFromClobberingWrite(Type *LoadTy, Value *LoadPtr,
                                           Value *WritePtr,
                                           uint64_t WriteSizeInBits,
                                           const DataLayout &DL) {
-  // If the loaded/stored value is a first class array/struct, or scalable type,
-  // don't try to transform them. We need to be able to bitcast to integer.
-  if (isFirstClassAggregateOrScalableType(LoadTy))
-    return -1;
-
   int64_t StoreOffset = 0, LoadOffset = 0;
   Value *StoreBase =
       GetPointerBaseWithConstantOffset(WritePtr, StoreOffset, DL);
   Value *LoadBase = GetPointerBaseWithConstantOffset(LoadPtr, LoadOffset, DL);
   if (StoreBase != LoadBase)
     return -1;
 
   // If the load and store are to the exact same address, they should have been
static int analyzeLoadFromClobberingWrite(Type *LoadTy, Value *LoadPtr,
   return LoadOffset - StoreOffset;
 }
 
 /// This function is called when we have a
 /// memdep query of a load that ends up being a clobbering store.
 int analyzeLoadFromClobberingStore(Type *LoadTy, Value *LoadPtr,
                                    StoreInst *DepSI, const DataLayout &DL) {
   auto *StoredVal = DepSI->getValueOperand();
-
-  // Cannot handle reading from store of first-class aggregate or scalable type.
-  if (isFirstClassAggregateOrScalableType(StoredVal->getType()))
-    return -1;
-
-  if (!canCoerceMustAliasedValueToLoad(StoredVal, LoadTy, DL))
+  if (!canCoerceMustAliasedValueToLoad(StoredVal, LoadTy, DL) ||
+      isa<ScalableVectorType>(StoredVal->getType()))
     return -1;
 
   Value *StorePtr = DepSI->getPointerOperand();
   uint64_t StoreSize =
-      DL.getTypeSizeInBits(DepSI->getValueOperand()->getType()).getFixedSize();
+      DL.getTypeSizeInBits(StoredVal->getType()).getFixedSize();
   return analyzeLoadFromClobberingWrite(LoadTy, LoadPtr, StorePtr, StoreSize,
                                         DL);
 }
 
 /// Looks at a memory location for a load (specified by MemLocBase, Offs, and
 /// Size) and compares it against a load.
 ///
 /// If the specified load could be safely widened to a larger integer load
static unsigned getLoadLoadClobberFullWidthSize(const Value *MemLocBase,
   }
 }
 
 /// This function is called when we have a
 /// memdep query of a load that ends up being clobbered by another load.  See if
 /// the other load can feed into the second load.
 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;
-
-  if (!canCoerceMustAliasedValueToLoad(DepLI, LoadTy, DL))
-    return -1;
-
   Value *DepPtr = DepLI->getPointerOperand();
+  if (canCoerceMustAliasedTypeToLoad(DepLI->getType(), LoadTy, DL)) {
     uint64_t DepSize = DL.getTypeSizeInBits(DepLI->getType()).getFixedSize();
-  int R = analyzeLoadFromClobberingWrite(LoadTy, LoadPtr, DepPtr, DepSize, DL);
+    int R =
+        analyzeLoadFromClobberingWrite(LoadTy, LoadPtr, DepPtr, DepSize, DL);
     if (R != -1)
       return R;
+  }
 
-  // If we have a load/load clobber an DepLI can be widened to cover this load,
+  // If we have a load/load clobber and DepLI can be widened to cover this load,
   // then we should widen it!
+  if (!canCoerceMustAliasedTypeToLoad(LoadTy, DepLI->getType(), DL))
+    return -1;
+
   int64_t LoadOffs = 0;
   const Value *LoadBase =
       GetPointerBaseWithConstantOffset(LoadPtr, LoadOffs, DL);
   unsigned LoadSize = DL.getTypeStoreSize(LoadTy).getFixedSize();
 
   unsigned Size =
       getLoadLoadClobberFullWidthSize(LoadBase, LoadOffs, LoadSize, DepLI);
   if (Size == 0)
int analyzeLoadFromClobberingMemInst(Type *LoadTy, Value *LoadPtr,
   ConstantInt *SizeCst = dyn_cast<ConstantInt>(MI->getLength());
   if (!SizeCst)
     return -1;
   uint64_t MemSizeInBits = SizeCst->getZExtValue() * 8;
 
   // If this is memset, we just need to see if the offset is valid in the size
   // of the memset..
   if (MI->getIntrinsicID() == Intrinsic::memset) {
-    if (DL.isNonIntegralPointerType(LoadTy->getScalarType())) {
-      auto *CI = dyn_cast<ConstantInt>(cast<MemSetInst>(MI)->getValue());
-      if (!CI || !CI->isZero())
-        return -1;
-    }
+    Value *StoredVal = cast<MemSetInst>(MI)->getValue();
+    if (auto *CI = dyn_cast<Constant>(StoredVal))
+      if (CI->isNullValue())
+        return analyzeLoadFromClobberingWrite(LoadTy, LoadPtr, MI->getDest(),
+                                              MemSizeInBits, DL);
+    Type *StoreTy = IntegerType::get(LoadTy->getContext(), MemSizeInBits);
+    if (canCoerceMustAliasedTypeToLoad(StoreTy, LoadTy, DL))
       return analyzeLoadFromClobberingWrite(LoadTy, LoadPtr, MI->getDest(),
                                             MemSizeInBits, DL);
+    return -1;
   }
 
   // If we have a memcpy/memmove, the only case we can handle is if this is a
   // copy from constant memory.  In that case, we can read directly from the
   // constant memory.
   MemTransferInst *MTI = cast<MemTransferInst>(MI);
 
   Constant *Src = dyn_cast<Constant>(MTI->getSource());