[PATCH] D67281: [AArch64][SimplifyCFG] Add additional cost for instructions in mergeConditionalStoreToAddress

[Pavel Kosov via Phabricator via llvm-commits]

kpdev42 added a comment.

Thank you for detailed answer.

> But then we decide "if.then" is small enough to "predicate" it at the IR level, and we never reverse that decision when it turns out that doesn't simplify anything.

Yes, I totally agree with you.

> I agree, five instructions is probably too many to speculate to eliminate a store. But the patch doesn't really reflect the actual costs here, which largely have to do with the PHI->select transform rather than the actual arithmetic instructions.

Two phi nodes are replaced with select instructions in `static bool FoldTwoEntryPHINode` (which is invoked after `mergeConditionalStoreToAddress`)

  while (PHINode *PN = dyn_cast<PHINode>(BB->begin())) {
    // Change the PHI node into a select instruction.
    Value *TrueVal = PN->getIncomingValue(PN->getIncomingBlock(0) == IfFalse);
    Value *FalseVal = PN->getIncomingValue(PN->getIncomingBlock(0) == IfTrue);
  
    Value *Sel = Builder.CreateSelect(IfCond, TrueVal, FalseVal, "", InsertPt);
    PN->replaceAllUsesWith(Sel);
    Sel->takeName(PN);
    PN->eraseFromParent();
  }

So it is needed to add some additional analysis here? I mean - `FoldTwoEntryPHINode` should make some checks about expediency of this folding?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67281