[PATCH] D44626: [InstCombine] Fold (A OR B) AND B code sequence over Phi node

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs created this revision.
inouehrs added reviewers: efriedma, dberlin, echristo, hfinkel, kbarton, nemanjai.

This patch intends to enable jump threading with a method whose return type is std::pair<int, bool> or std::pair<bool, int>
For example, jump threading does not work for the if statement in func.

  std::pair<int, bool> callee(int v) {
    int a = dummy(v);
    if (a) return std::make_pair(dummy(v), true);
    else return std::make_pair(v, v < 0);
  }
  
  int func(int v) {
    std::pair<int, bool> rc = callee(v);
    if (rc.second) {
      // do something
    }

SROA executed before the method inlining replaces std::pair by i64 without splitting in both `callee` and `func` since at this point no access to the individual fields is seen to SROA.
After inlining, jump threading fails to identify that the incoming value is a constant due to additional instructions (like or, and, trunc).

This patch finds a phi node, which has OR instruction as an incoming value and AND instruction as its use. If the OR and AND instructions take the same operand, e.g. (%A OR %B) AND %B, then replace the incoming OR by %B. For example,

  BB1:
    %or = or i64 %val, 1
    br %BB2
  BB2:
    %phi = phi i64 [ %or, %BB1 ], ... # -> phi i64 [ 1, %BB1 ], ...
    %and = and i64 %phi, 1

This helps jump threading identify the opportunity listed above.
Later, in the CFG simplification pass, the similar code modification happens. But it is too late to help jump threading.


https://reviews.llvm.org/D44626

Files:
  lib/Transforms/InstCombine/InstCombineInternal.h
  lib/Transforms/InstCombine/InstCombinePHI.cpp


Index: lib/Transforms/InstCombine/InstCombinePHI.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -635,6 +635,50 @@
   return NewLI;
 }
 
+// FoldPHIArgOrIntoPHI finds a phi node, which has OR instruction as an incoming value
+// and AND instruction as its use. If the OR and AND instructions take the same operand
+// as (%A OR %B) AND %B then replace the incoming value as %B.
+// For example,
+//   BB1:
+//     %or = or i64 %val, 1
+//     br %BB2
+//   BB2:
+//     %phi = phi i64 [ %or, %BB1 ], ... # -> phi i64 [ 1, %BB1 ], ...
+//     %and = and i64 %phi, 1
+// This optimization allows jump threading pass to find the opportunity in method call
+// whose return value is std::pair<int, bool>.
+Instruction *InstCombiner::FoldPHIArgOrIntoPHI(PHINode &Phi) {
+  for (User *User : Phi.users()) {
+    // Try to find OR - Phi - AND code sequence.
+    Value *UserVal = nullptr;
+    if (!match(User, m_And(m_Specific(&Phi), m_Value(UserVal))))
+      continue;
+
+    auto IsEligible = [&](Value *V) {
+      return match(V, m_Or(m_Value(), m_Specific(UserVal)));
+    };
+    if (llvm::none_of(Phi.incoming_values(), IsEligible))
+      continue;
+
+    PHINode* NewPhi = Φ
+    // If there is another use of the phi node, we create a new one
+    // for this AND instruction by cloning the original phi node.
+    if (!Phi.hasOneUse()) {
+      NewPhi = cast<PHINode>(Phi.clone());
+      InsertNewInstBefore(NewPhi, Phi);
+      cast<Instruction>(User)->setOperand(0, NewPhi);
+    }
+
+    // We replace the incoming OR with UserVal.
+    for (unsigned Idx = 0; Idx < NewPhi->getNumIncomingValues(); Idx++) {
+      Value *V = NewPhi->getIncomingValue(Idx);
+      if (match(V, m_Or(m_Value(), m_Specific(UserVal))))
+        NewPhi->setIncomingValue(Idx, UserVal);
+    }
+  }
+  return nullptr;
+}
+
 /// TODO: This function could handle other cast types, but then it might
 /// require special-casing a cast from the 'i1' type. See the comment in
 /// FoldPHIArgOpIntoPHI() about pessimizing illegal integer types.
@@ -1130,6 +1174,9 @@
     if (Instruction *Result = FoldPHIArgOpIntoPHI(PN))
       return Result;
 
+  if (Instruction *Result = FoldPHIArgOrIntoPHI(PN))
+    return Result;
+
   // If this is a trivial cycle in the PHI node graph, remove it.  Basically, if
   // this PHI only has a single use (a PHI), and if that PHI only has one use (a
   // PHI)... break the cycle.
Index: lib/Transforms/InstCombine/InstCombineInternal.h
===================================================================
--- lib/Transforms/InstCombine/InstCombineInternal.h
+++ lib/Transforms/InstCombine/InstCombineInternal.h
@@ -703,6 +703,7 @@
   Instruction *FoldPHIArgGEPIntoPHI(PHINode &PN);
   Instruction *FoldPHIArgLoadIntoPHI(PHINode &PN);
   Instruction *FoldPHIArgZextsIntoPHI(PHINode &PN);
+  Instruction *FoldPHIArgOrIntoPHI(PHINode &PN);
 
   /// If an integer typed PHI has only one use which is an IntToPtr operation,
   /// replace the PHI with an existing pointer typed PHI if it exists. Otherwise


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44626.138908.patch
Type: text/x-patch
Size: 3150 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180319/4a46646f/attachment.bin>