[InstCombine] Negator: while there, add detection for cycles during negation
authorRoman Lebedev <lebedev.ri@gmail.com>
Wed, 17 Jun 2020 19:33:44 +0000 (22:33 +0300)
committerRoman Lebedev <lebedev.ri@gmail.com>
Wed, 17 Jun 2020 19:47:20 +0000 (22:47 +0300)
I don't have any testcases showing it happening,
and i haven't succeeded in creating one,
but i'm also not positive it can't ever happen,
and i recall having something that looked like
that in the very beginning of Negator creation.

But since we now already have a negation cache,
we can now detect such cases practically for free.

Let's do so instead of "relying" on stack overflow :D

llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp

index deb14f1..3fe615a 100644 (file)
@@ -388,16 +388,30 @@ LLVM_NODISCARD Value *Negator::negate(Value *V, unsigned Depth) {
   ++NumValuesVisitedInThisNegator;
 #endif
 
+#ifndef NDEBUG
+  // We can't ever have a Value with such an address.
+  Value *Placeholder = reinterpret_cast<Value *>(static_cast<uintptr_t>(-1));
+#endif
+
   // Did we already try to negate this value?
   auto NegationsCacheIterator = NegationsCache.find(V);
   if (NegationsCacheIterator != NegationsCache.end()) {
     ++NegatorNumNegationsFoundInCache;
-    return NegationsCacheIterator->second;
+    Value *NegatedV = NegationsCacheIterator->second;
+    assert(NegatedV != Placeholder && "Encountered a cycle during negation.");
+    return NegatedV;
   }
 
+#ifndef NDEBUG
+  // We did not find a cached result for negation of V. While there,
+  // let's temporairly cache a placeholder value, with the idea that if later
+  // during negation we fetch it from cache, we'll know we're in a cycle.
+  NegationsCache[V] = Placeholder;
+#endif
+
   // No luck. Try negating it for real.
   Value *NegatedV = visitImpl(V, Depth);
-  // And cache the result for the future.
+  // And cache the (real) result for the future.
   NegationsCache[V] = NegatedV;
 
   return NegatedV;