[PATCH] D76989: [LVI] Move assume intersection from getEdgeValue() to getValueOnEdge()

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: reames, lebedev.ri.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

We currently perform an assume intersection with the context instruction directly in the internal getEdgeValue() API. This patch moves it into getValueOnEdge(), similar to what we do in getValueInBlock() and getValueAt().

There are three users of getEdgeValue():

1. getValueOnEdge(): As this is where the intersection is moved, no change in behavior there.
2. solveBlockValueNonLocal(): This call is done without the context instruction.
3. solveBlockValuePHINode(): This call is done with the phi node as context, i.e. the start of the basic block. We already intersect with the terminators of the predecessors, so this doesn't add anything.

Moving the intersection in this way avoids unnecessary work, makes the LVI APIs more symmetric, and makes the caching more obviously correct (as the lengthy comments that are being removed here indicate.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76989

Files:
  lib/Analysis/LazyValueInfo.cpp


Index: lib/Analysis/LazyValueInfo.cpp
===================================================================
--- lib/Analysis/LazyValueInfo.cpp
+++ lib/Analysis/LazyValueInfo.cpp
@@ -398,7 +398,7 @@
 
   bool getBlockValue(ValueLatticeElement &Result, Value *Val, BasicBlock *BB);
   bool getEdgeValue(Value *V, BasicBlock *F, BasicBlock *T,
-                    ValueLatticeElement &Result, Instruction *CxtI = nullptr);
+                    ValueLatticeElement &Result);
 
   // These methods process one work item and may add more. A false value
   // returned means that the work item was not completely processed and must
@@ -761,10 +761,7 @@
     BasicBlock *PhiBB = PN->getIncomingBlock(i);
     Value *PhiVal = PN->getIncomingValue(i);
     ValueLatticeElement EdgeResult;
-    // Note that we can provide PN as the context value to getEdgeValue, even
-    // though the results will be cached, because PN is the value being used as
-    // the cache key in the caller.
-    if (!getEdgeValue(PhiVal, PhiBB, BB, EdgeResult, PN))
+    if (!getEdgeValue(PhiVal, PhiBB, BB, EdgeResult))
       // Explore that input, then return here
       return false;
 
@@ -1463,8 +1460,7 @@
 /// the basic block if the edge does not constrain Val.
 bool LazyValueInfoImpl::getEdgeValue(Value *Val, BasicBlock *BBFrom,
                                      BasicBlock *BBTo,
-                                     ValueLatticeElement &Result,
-                                     Instruction *CxtI) {
+                                     ValueLatticeElement &Result) {
   // If already a constant, there is nothing to compute.
   if (Constant *VC = dyn_cast<Constant>(Val)) {
     Result = ValueLatticeElement::get(VC);
@@ -1490,15 +1486,6 @@
   // Try to intersect ranges of the BB and the constraint on the edge.
   intersectAssumeOrGuardBlockValueConstantRange(Val, InBlock,
                                                 BBFrom->getTerminator());
-  // We can use the context instruction (generically the ultimate instruction
-  // the calling pass is trying to simplify) here, even though the result of
-  // this function is generally cached when called from the solve* functions
-  // (and that cached result might be used with queries using a different
-  // context instruction), because when this function is called from the solve*
-  // functions, the context instruction is not provided. When called from
-  // LazyValueInfoImpl::getValueOnEdge, the context instruction is provided,
-  // but then the result is not cached.
-  intersectAssumeOrGuardBlockValueConstantRange(Val, InBlock, CxtI);
 
   Result = intersect(LocalResult, InBlock);
   return true;
@@ -1547,12 +1534,13 @@
                     << "'\n");
 
   ValueLatticeElement Result;
-  if (!getEdgeValue(V, FromBB, ToBB, Result, CxtI)) {
+  if (!getEdgeValue(V, FromBB, ToBB, Result)) {
     solve();
-    bool WasFastQuery = getEdgeValue(V, FromBB, ToBB, Result, CxtI);
+    bool WasFastQuery = getEdgeValue(V, FromBB, ToBB, Result);
     (void)WasFastQuery;
     assert(WasFastQuery && "More work to do after problem solved?");
   }
+  intersectAssumeOrGuardBlockValueConstantRange(V, Result, CxtI);
 
   LLVM_DEBUG(dbgs() << "  Result = " << Result << "\n");
   return Result;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76989.253343.patch
Type: text/x-patch
Size: 3247 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200328/fcf5cf70/attachment-0001.bin>