[PATCH] D84629: [LazyValueInfo] Let getEdgeValueLocal look into freeze instructions

Mon Aug 10 18:18:00 PDT 2020

aqjune marked an inline comment as done.
aqjune added inline comments.

================
Comment at: llvm/lib/Analysis/LazyValueInfo.cpp:1316
+          (isa<FreezeInst>(Val) &&
+           Condition == cast<FreezeInst>(Val)->getOperand(0)))
         return ValueLatticeElement::get(ConstantInt::get(
----------------
nikic wrote:
> aqjune wrote:
> > nikic wrote:
> > > efriedma wrote:
> > > > nikic wrote:
> > > > > Wouldn't we expect such a freeze to get dropped by InstSimplify/InstCombine anyway?
> > > > You mean, we should rewrite uses of a freeze in blocks where we know the operand of the freeze can't be undef/poison?  I guess that's something we should do.  I don't think instsimplify has the context necessary to do that sort of rewrite.
> > > You're right, if there are multiple uses and only some are dominated by the condition we currently don't handle it in InstCombine. The `@simple` case from the test would be handled though, because the freeze gets sunk.
> > Would it be the right direction if I implement this optimization in InstCombine instead?
> Is this code actually needed? I'm thinking that the constantFoldUser code should already take care of it. The Freeze usesOperand Condition, and should then get folded.
The check seemed redundant to me as well, so removed it. The unit tests still work too.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84629