[PATCH] D78133: [PredicateInfo] Optionally set OriginalOp to renamed value it refers to.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 276418.
fhahn retitled this revision from "[PredicateInfo] Optionally set OriginalOp to renamed value it refers to. " to "[PredicateInfo] Optionally set OriginalOp to renamed value it refers to.".
fhahn added a comment.

Add new RenamedOp to PredicateBase instead of having a separate option.

>> In D78133#2131838 <https://reviews.llvm.org/D78133#2131838>, @nikic wrote:
>> 
>>>
> 
> Why don't we include both variants? While NewGVN needs the current OriginalOp for the replacement instruction patching, it also needs what is being implemented here (lets call it CondOp) when originally handling the PredicateInfo. As this comment indicates, NewGVN currently has the same problem as SCCP: https://github.com/llvm/llvm-project/blob/2279380eab08219911910e1ecdcef3eacb0b7f0c/llvm/lib/Transforms/Scalar/NewGVN.cpp#L1565-L1572
> 
> As PredicateInfo structures aren't particularly memory critical, I'd suggest to include both `OriginalOp` (value prior to any renaming) and `CondOp` (value used inside the condition) and use them as appropriate.

Initially I wanted to not increase memory consumption, but it's probably not a big deal. I also had same patches further reducing the memory consumption here, but did not have time to follow up on them. Anyways, I've added a separate RenamedOp field.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78133/new/

https://reviews.llvm.org/D78133

Files:
  llvm/include/llvm/Transforms/Utils/PredicateInfo.h
  llvm/lib/Transforms/Utils/PredicateInfo.cpp


Index: llvm/lib/Transforms/Utils/PredicateInfo.cpp
===================================================================
--- llvm/lib/Transforms/Utils/PredicateInfo.cpp
+++ llvm/lib/Transforms/Utils/PredicateInfo.cpp
@@ -600,6 +600,9 @@
         RenameIter == RenameStack.begin() ? OrigOp : (RenameIter - 1)->Def;
     ValueDFS &Result = *RenameIter;
     auto *ValInfo = Result.PInfo;
+    ValInfo->RenamedOp = (RenameStack.end() - Start) == RenameStack.begin()
+                             ? OrigOp
+                             : (RenameStack.end() - Start - 1)->Def;
     // For edge predicates, we can just place the operand in the block before
     // the terminator.  For assume, we have to place it right before the assume
     // to ensure we dominate all of our uses.  Always insert right before the
Index: llvm/include/llvm/Transforms/Utils/PredicateInfo.h
===================================================================
--- llvm/include/llvm/Transforms/Utils/PredicateInfo.h
+++ llvm/include/llvm/Transforms/Utils/PredicateInfo.h
@@ -79,6 +79,10 @@
   // This can be use by passes, when destroying predicateinfo, to know
   // whether they can just drop the intrinsic, or have to merge metadata.
   Value *OriginalOp;
+  // The renamed operand in the condition used for this predicate. For nested
+  // predicates, this is different to OriginalOp which refers to the initial
+  // operand.
+  Value *RenamedOp;
   PredicateBase(const PredicateBase &) = delete;
   PredicateBase &operator=(const PredicateBase &) = delete;
   PredicateBase() = delete;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78133.276418.patch
Type: text/x-patch
Size: 1566 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200708/0194393a/attachment.bin>