[llvm-bugs] [Bug 43800] New: [IfConversion] Diamond assumes unanalyzable dests are equivalent

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=43800

            Bug ID: 43800
           Summary: [IfConversion] Diamond assumes unanalyzable dests are
                    equivalent
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: normal
          Priority: P
         Component: Common Code Generator Code
          Assignee: unassignedbugs at nondot.org
          Reporter: alex.davies at iinet.net.au
                CC: llvm-bugs at lists.llvm.org

ValidDiamond considers if-converting sub-CFGs like:

  // Diamond:
  //   EBB
  //   / \_
  //  |   |
  // TBB FBB
  //   \ /
  //  TailBB

But does not check the actual successors of TBB/FBB when merging, but rather
previously analyzed results.

The problem with that is that if TBB and FBB _both_ fail to be analyzed, it's
comparing nullptr with nullptr, considers TailBB(nullptr) equal, and evaluates
it for a (code-breaking) merge.

***

Relevant code segments:

TrueBB/FalseBB are nullptr on failure to analyze:

  if (!BBI.IsBrAnalyzable) {
    BBI.TrueBB = nullptr;
    BBI.FalseBB = nullptr;
    BBI.BrCond.clear();
  }

ValidDiamond:

  MachineBasicBlock *TT = TrueBBI.TrueBB;
  MachineBasicBlock *FT = FalseBBI.TrueBB;

  if (!TT && blockAlwaysFallThrough(TrueBBI))
    TT = getNextBlock(*TrueBBI.BB);
  if (!FT && blockAlwaysFallThrough(FalseBBI))
    FT = getNextBlock(*FalseBBI.BB);
  if (TT != FT)
    return false;

(blockAlwaysFallThrough returns false if not analyzable, leaving TT and FT as
nullptr)

***

What is odd to me, is that ValidDiamond seems _designed_ to try and operate
with non-analyzable branches, without even checking CFG successors.

I can understand this in the case of 0 successors (ret/tailcall) or _maybe_
where analyzeBranch() fails but each block has the same, single, successor (I
wouldn't go beyond that myself)... but to not have any checks at all is just
asking for trouble. Apologies for lack of demonstration code, it's hard to
reproduce without familiarity with an in-tree architecture and their
analyzeBranch results, but I am 99% sure there is a real problem here (that/or
I've misimplemented analyzeBranch on the arch I'm on...).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20191025/deefdaa2/attachment.html>