[InstCombine] Negator: cache negation results (PR46362)

It is possible that we can try to negate the same value multiple times.
For example, PHI nodes may happen to have multiple incoming values
(all of which must be the same value) for the same incoming basic block.
It may happen that we try to negate such a PHI node, and succeed,
and that might result in having now-different incoming values..

To avoid that, and in general to reduce the amount of duplicated
work we might be doing, let's introduce a cache where
we'll track results of negating each value.

The added test was previously failing -verify after -instcombine.

Fixes https://bugs.llvm.org/show_bug.cgi?id=46362

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 29fcc3c..2ad9c73 100644 (file)
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -16,6 +16,7 @@
 #define LLVM_LIB_TRANSFORMS_INSTCOMBINE_INSTCOMBINEINTERNAL_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/InstructionSimplify.h"
@@ -1040,6 +1041,8 @@ class Negator final {
 
   const bool IsTrulyNegation;
 
+  SmallDenseMap<Value *, Value *> NegationsCache;
+
   Negator(LLVMContext &C, const DataLayout &DL, AssumptionCache &AC,
           const DominatorTree &DT, bool IsTrulyNegation);
 

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
index f613a6c..deb14f1 100644 (file)
--- a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
@@ -14,6 +14,7 @@
 #include "InstCombineInternal.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
@@ -43,6 +44,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include <functional>
 #include <tuple>
+#include <type_traits>
 #include <utility>
 
 namespace llvm {
@@ -68,6 +70,8 @@ STATISTIC(NegatorTimesDepthLimitReached,
 STATISTIC(
     NegatorNumValuesVisited,
     "Negator: Total number of values visited during attempts to sink negation");
+STATISTIC(NegatorNumNegationsFoundInCache,
+          "Negator: How many negations did we retrieve/reuse from cache");
 STATISTIC(NegatorMaxTotalValuesVisited,
           "Negator: Maximal number of values ever visited while attempting to "
           "sink negation");
@@ -384,8 +388,19 @@ LLVM_NODISCARD Value *Negator::negate(Value *V, unsigned Depth) {
   ++NumValuesVisitedInThisNegator;
 #endif
 
-  // Try negating the value.
-  return visitImpl(V, Depth);
+  // Did we already try to negate this value?
+  auto NegationsCacheIterator = NegationsCache.find(V);
+  if (NegationsCacheIterator != NegationsCache.end()) {
+    ++NegatorNumNegationsFoundInCache;
+    return NegationsCacheIterator->second;
+  }
+
+  // No luck. Try negating it for real.
+  Value *NegatedV = visitImpl(V, Depth);
+  // And cache the result for the future.
+  NegationsCache[V] = NegatedV;
+
+  return NegatedV;
 }
 
 LLVM_NODISCARD Optional<Negator::Result> Negator::run(Value *Root) {

diff --git a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
index 79d1e58..f913de3 100644 (file)
--- a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
+++ b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
@@ -4,6 +4,7 @@
 declare void @use4(i4)
 declare void @use8(i8)
 declare void @use_v2i4(<2 x i4>)
+declare i1 @use32gen1(i32)
 
 ; Constant can be freely negated.
 define i8 @t0(i8 %x) {
@@ -349,6 +350,44 @@ end:
   %n = sub i8 0, %r
   ret i8 %n
 }
+define void @phi_with_duplicate_incoming_basic_blocks(i32 %x, i32 %y, i1 %should_lookup, i32 %z) {
+; CHECK-LABEL: @phi_with_duplicate_incoming_basic_blocks(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[X_INC_NEG:%.*]] = xor i32 [[X:%.*]], -1
+; CHECK-NEXT:    br i1 [[SHOULD_LOOKUP:%.*]], label [[LOOKUP:%.*]], label [[LOOP:%.*]]
+; CHECK:       lookup:
+; CHECK-NEXT:    [[TO_LOOKUP:%.*]] = phi i32 [ [[Y:%.*]], [[ENTRY:%.*]] ], [ [[METAVAL_NEG:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    switch i32 [[TO_LOOKUP]], label [[END:%.*]] [
+; CHECK-NEXT:    i32 0, label [[LOOP]]
+; CHECK-NEXT:    i32 42, label [[LOOP]]
+; CHECK-NEXT:    ]
+; CHECK:       loop:
+; CHECK-NEXT:    [[METAVAL_NEG]] = phi i32 [ [[X_INC_NEG]], [[LOOKUP]] ], [ [[X_INC_NEG]], [[LOOKUP]] ], [ -84, [[ENTRY]] ]
+; CHECK-NEXT:    [[REPEAT:%.*]] = call i1 @use32gen1(i32 [[METAVAL_NEG]])
+; CHECK-NEXT:    br i1 [[REPEAT]], label [[LOOKUP]], label [[END]]
+; CHECK:       end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %x_inc = add i32 %x, 1
+  br i1 %should_lookup, label %lookup, label %loop
+
+lookup:
+  %to_lookup = phi i32 [ %y, %entry ], [ %negated_metaval, %loop ]
+  switch i32 %to_lookup, label %end [
+  i32 0, label %loop
+  i32 42, label %loop
+  ]
+
+loop:
+  %metaval = phi i32 [ %x_inc, %lookup ], [ %x_inc, %lookup ], [ 84, %entry ]
+  %negated_metaval = sub i32 0, %metaval
+  %repeat = call i1 @use32gen1(i32 %negated_metaval)
+  br i1 %repeat, label %lookup, label %end
+
+end:
+  ret void
+}
 
 ; truncation can be negated if it's operand can be negated
 define i8 @t20(i8 %x, i16 %y) {