[PATCH] D54827: [simplifycfg] Handle 3 sequential branches while the first two can infer the third one.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:5953-5955
+          Value *CombinedAndCmp = GetCombinedCompareInstruction(
+              Builder, PPBI->getCondition(), PBI->getCondition(), PCondIsTrue,
+              CondIsTrue);
----------------
lebedev.ri wrote:
> yinyuefengyi wrote:
> > lebedev.ri wrote:
> > > And this too might already exist..
> > > Have you seen [[ https://github.com/llvm-mirror/llvm/blob/5e3c9a56cb080c7a5131d6063d04d1d4a22044d0/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp | `InstCombiner::foldAndOfICmps()` ]] ?
> > Thanks for the comments.
> > I've read the InstCombiner::foldAndOfIcmps(),  it is a local member functions with a lot of other member functions called inside, seems not easy to call it directly from class SimplifyCFGOpt, do you have any suggestions about this?
> Perhaps it can be promoted to not be `private`? (cc @spatel)
> https://github.com/llvm-mirror/llvm/blob/5e3c9a56cb080c7a5131d6063d04d1d4a22044d0/lib/Transforms/InstCombine/InstCombineInternal.h#L437-L581
> Duplicating the exact same logic is probably not the best answer here..
I think the way to share the code is to move it from InstCombine to CmpInstAnalysis.cpp. That's how CmpInstAnalysis was created in the first place.

I'm not sure if would avoid that work, but have you looked at trying to simplify the icmp in CorrelatedValuePropagation instead of here in SimplifyCFG?


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

https://reviews.llvm.org/D54827