[PATCH] D148514: [BranchFolding] Remove redundant conditional branch.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: llvm/lib/CodeGen/BranchFolding.cpp:1684
+      if (CurTBB && IsBranchOnlyBlock(MBB) &&
+          AreOperandsIdentical(CurCond, PriorCond) && !MBB->hasAddressTaken() &&
+          !MBB->isEHPad()) {
----------------
skatkov wrote:
> goldstein.w.n wrote:
> > skatkov wrote:
> > > skatkov wrote:
> > > > goldstein.w.n wrote:
> > > > > Does this take into account comparisons against memory? The pointer (operand) stays the same but value in memory is modified in BB1?
> > > > BB1 is a predecessor or MBB? For MBB we check that it is IsBranchOnlyBlock(MBB).
> > > > 
> > > > you mean that predecessor may contains store AFTER conditional branch and conditional branch itself may reference memory?
> > > > 
> > > and -> or
> > > BB1 is a predecessor or MBB? For MBB we check that it is IsBranchOnlyBlock(MBB).
> > > 
> > > you mean that predecessor may contains store AFTER conditional branch and conditional branch itself may reference memory?
> > > 
> > Yes, exactly.
> > If this is still in SSA then it's fine, but x86 machine instructions (and other) have `rm` modifier where operands match but results can vary
> > 
> Interesting, something new to me. At least on x86_64 conditional instruction is JCC which actually only reads the eflags and has no access to memory. If there is no other modification of eflags - we are ok.
> 
> But at least I need to check two things:
> 1) is it possible that there are some instructions after conditional branch which may affect the result of next equivalent conditional branch.
> 2) Are there any platforms where two conditional branches, one right after another one, may produce the right results.
> 
> Thanks for good question.
may produce the right results. -> may produce different results.


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

https://reviews.llvm.org/D148514