[PATCH] D114260: [SCEV] Remove validity check on lookup (NFC)

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: reames, mkazantsev.
Herald added subscribers: javed.absar, hiraditya.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

When the value held by SCEVUnknown is removed, we will call `forgetMemoizedValues()` on it in https://github.com/llvm/llvm-project/blob/d5de568cc7375281b14bd2632576bff7f4afabc3/llvm/lib/Analysis/ScalarEvolution.cpp#L504.

After recent changes, this will also forget memoized values on recursive users. After D113349 <https://reviews.llvm.org/D113349> this additionally clears the values from the ValueExprMap. This means that the nullptr SCEVUnknown will no longer be directly or indirectly part of ValueExprMap, and we can drop the validity check in `getSCEV()`.

This simplifies our invariants (we essentially remove the notion of an "invalid" SCEV) and saves a bit of compile-time: https://llvm-compile-time-tracker.com/compare.php?from=38ff05f329cb3116c6c4f46a34318b421fda4331&to=20d2f513a21cfd9678df091499f6a6226d9fca89&stat=instructions


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114260

Files:
  llvm/include/llvm/Analysis/ScalarEvolution.h
  llvm/lib/Analysis/ScalarEvolution.cpp


Index: llvm/lib/Analysis/ScalarEvolution.cpp
===================================================================
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -4016,15 +4016,6 @@
   return CouldNotCompute.get();
 }
 
-bool ScalarEvolution::checkValidity(const SCEV *S) const {
-  bool ContainsNulls = SCEVExprContains(S, [](const SCEV *S) {
-    auto *SU = dyn_cast<SCEVUnknown>(S);
-    return SU && SU->getValue() == nullptr;
-  });
-
-  return !ContainsNulls;
-}
-
 bool ScalarEvolution::containsAddRecurrence(const SCEV *S) {
   HasRecMapType::iterator I = HasRecMap.find(S);
   if (I != HasRecMap.end())
@@ -4139,14 +4130,7 @@
   assert(isSCEVable(V->getType()) && "Value is not SCEVable!");
 
   ValueExprMapType::iterator I = ValueExprMap.find_as(V);
-  if (I != ValueExprMap.end()) {
-    const SCEV *S = I->second;
-    if (checkValidity(S))
-      return S;
-    eraseValueFromMap(V);
-    forgetMemoizedResults(S);
-  }
-  return nullptr;
+  return I != ValueExprMap.end() ? I->second : nullptr;
 }
 
 /// Return a SCEV corresponding to -V = -1*V
Index: llvm/include/llvm/Analysis/ScalarEvolution.h
===================================================================
--- llvm/include/llvm/Analysis/ScalarEvolution.h
+++ llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -1926,10 +1926,6 @@
   /// Insert V to S mapping into ValueExprMap and ExprValueMap.
   void insertValueToMap(Value *V, const SCEV *S);
 
-  /// Return false iff given SCEV contains a SCEVUnknown with NULL value-
-  /// pointer.
-  bool checkValidity(const SCEV *S) const;
-
   /// Return true if `ExtendOpTy`({`Start`,+,`Step`}) can be proved to be
   /// equal to {`ExtendOpTy`(`Start`),+,`ExtendOpTy`(`Step`)}.  This is
   /// equivalent to proving no signed (resp. unsigned) wrap in


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114260.388543.patch
Type: text/x-patch
Size: 1807 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211119/f9db56e2/attachment.bin>