Skip to content

Commit

Permalink
[DAGCombiner] Fold BRCOND(FREEZE(COND)) to BRCOND(COND)
Browse files Browse the repository at this point in the history
This patch resolves the suboptimal codegen described in http://llvm.org/pr47873 .
When CodeGenPrepare lowers select into a conditional branch, a freeze instruction is inserted.
It is then translated to `BRCOND(FREEZE(SETCC))` in SelDag.
The `FREEZE` in the middle of `SETCC` and `BRCOND` was causing a suboptimal code generation however.
This patch adds `BRCOND(FREEZE(cond))` -> `BRCOND(cond)` fold to DAGCombiner to remove the `FREEZE`.

To make this optimization sound, `BRCOND(UNDEF)` simply should nondeterministically jump to the branch or not, rather than raising UB.
It wasn't clear what happens when the condition was undef according to the comments in ISDOpcodes.h, however.
I updated the comments of `BRCOND` to make it explicit (as well as `BR_CC`, which is also a conditional branch instruction).

Note that it diverges from the semantics of `br` instruction in IR, which is explicitly UB.
Since the UB semantics was necessary to explain optimizations that use branching conditions, and SelDag doesn't seem to have such optimization, I think this divergence is okay.

Reviewed By: spatel

Differential Revision: https://reviews.llvm.org/D92015
  • Loading branch information
aqjune committed Jan 13, 2021
1 parent 76643c4 commit 25eb7b0
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
9 changes: 7 additions & 2 deletions llvm/include/llvm/CodeGen/ISDOpcodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -890,13 +890,18 @@ enum NodeType {
/// BRCOND - Conditional branch. The first operand is the chain, the
/// second is the condition, the third is the block to branch to if the
/// condition is true. If the type of the condition is not i1, then the
/// high bits must conform to getBooleanContents.
/// high bits must conform to getBooleanContents. If the condition is undef,
/// it nondeterministically jumps to the block.
/// TODO: Its semantics w.r.t undef requires further discussion; we need to
/// make it sure that it is consistent with optimizations in MIR & the
/// meaning of IMPLICIT_DEF. See https://reviews.llvm.org/D92015
BRCOND,

/// BR_CC - Conditional branch. The behavior is like that of SELECT_CC, in
/// that the condition is represented as condition code, and two nodes to
/// compare, rather than as a combined SetCC node. The operands in order
/// are chain, cc, lhs, rhs, block to branch to if condition is true.
/// are chain, cc, lhs, rhs, block to branch to if condition is true. If
/// condition is undef, it nondeterministically jumps to the block.
BR_CC,

/// INLINEASM - Represents an inline asm block. This node always has two
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14548,6 +14548,13 @@ SDValue DAGCombiner::visitBRCOND(SDNode *N) {
SDValue N1 = N->getOperand(1);
SDValue N2 = N->getOperand(2);

// BRCOND(FREEZE(cond)) is equivalent to BRCOND(cond) (both are
// nondeterministic jumps).
if (N1->getOpcode() == ISD::FREEZE && N1.hasOneUse()) {
return DAG.getNode(ISD::BRCOND, SDLoc(N), MVT::Other, Chain,
N1->getOperand(0), N2);
}

// If N is a constant we could fold this into a fallthrough or unconditional
// branch. However that doesn't happen very often in normal code, because
// Instcombine/SimplifyCFG should have handled the available opportunities.
Expand Down
6 changes: 2 additions & 4 deletions llvm/test/CodeGen/X86/select-prof-codegen.ll
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s

; TODO: Compiling the select should not create 'seta - testb $1 - jump' sequence.
; Compiling the select should not create 'seta - testb $1 - jump' sequence.
define i32 @f(i32 %x, i32 %y) {
; CHECK-LABEL: f:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: movl %edi, %eax
; CHECK-NEXT: cmpl %esi, %edi
; CHECK-NEXT: seta %cl
; CHECK-NEXT: testb $1, %cl
; CHECK-NEXT: jne .LBB0_2
; CHECK-NEXT: ja .LBB0_2
; CHECK-NEXT: # %bb.1: # %select.false
; CHECK-NEXT: movl %esi, %eax
; CHECK-NEXT: .LBB0_2: # %select.end
Expand Down

0 comments on commit 25eb7b0

Please sign in to comment.