[DSE] Remove alloc function check in canSkipDef()

[nikic]

Summary

canSkipDef() currently skips inaccessiblememonly calls, but not if they are allocation functions. This check was added in D103009, but I don't understand why. There are no test failures if it is removed, and I don't really see what relation it has to the implemented transform (as canSkipDef() is not used on the storeIsNoop() path, as far as I can tell).

Either this is dead code, or we're missing a test.

[reames]

FWIW, I'm also quite confused by this code. My best guess was that maybe this was handling the case where we introduced a new memory region (e.g. the return noalias) as opposed to merely writing to an existing region, but that's somewhat a guess. I don't see that distinction made in the DSE code, and I'm not sure how memory-ssa models that.

[yurai007]

Hi, it appears that https://reviews.llvm.org/D103009 emerged from https://reviews.llvm.org/D101440 and canSkipDef change was included by mistake.
It should be safe to get rid of it. Also together with isAllocLikeFn we can remove TLI parameter I think.

[fhahn]

LGTM, thanks!

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

703

as @yurai007 suggested, it looks like the TLI arg can be dropped

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

default:
       return false;
     }
   }
   return false;
 }
 
 // Check if we can ignore \p D for DSE.
 bool canSkipDef(MemoryDef *D, bool DefVisibleToCaller,
                 const TargetLibraryInfo &TLI) {
   Instruction *DI = D->getMemoryInst();
   // Calls that only access inaccessible memory cannot read or write any memory
   // locations we consider for elimination.
   if (auto *CB = dyn_cast<CallBase>(DI))
-    if (CB->onlyAccessesInaccessibleMemory()) {
-      if (isAllocLikeFn(DI, &TLI))
-        return false;
+    if (CB->onlyAccessesInaccessibleMemory())
       return true;
-    }
+
   // We can eliminate stores to locations not visible to the caller across
   // throwing instructions.
   if (DI->mayThrow() && !DefVisibleToCaller)
     return true;
 
   // We can remove the dead stores, irrespective of the fence and its ordering
   // (release/acquire/seq_cst). Fences only constraints the ordering of
   // already visible stores, it does not make a store visible to other