[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()) {
----------------
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.


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

https://reviews.llvm.org/D148514