`mergeIf` arguments are swapped in the no-else case in Dataflow IR

[sumleo]

Summary

In doVisitIf in the Dataflow graph builder, when an if has no else branch, the two state arguments passed to mergeIf are in the wrong order. This causes the phi node to pair the "condition true" value with the initial state (before the if body ran) and the "condition false" value with the after-if-true state (after the if body ran). The two values are swapped.

This affects the --flatten --dfo (DataFlowOpts) and --flatten --souperify passes.

Location

src/dataflow/graph.h:277

Node* doVisitIf(If* curr) {
    // ...
    auto initialState = locals;
    visit(curr->ifTrue);
    auto afterIfTrueState = locals;
    if (curr->ifFalse) {
      locals = initialState;
      visit(curr->ifFalse);
      auto afterIfFalseState = locals;
      mergeIf(afterIfTrueState, afterIfFalseState, condition, curr, locals);
    } else {
      mergeIf(initialState, afterIfTrueState, condition, curr, locals); // BUG
    }
    // ...
}

Why this is wrong

mergeIf maps its first argument (aState) to the ifTrue condition and its second argument (bState) to the ifFalse condition:

void mergeIf(Locals& aState, Locals& bState, ...) {
    // ...
    states.emplace_back(aState, ifTrue);   // aState → selected when condition is true
    states.emplace_back(bState, ifFalse);  // bState → selected when condition is false
    // ...
}

In the if-with-else case (line 275), the call is correct:

• aState = afterIfTrueState → ifTrue condition (correct: this is the state after the true branch ran)
• bState = afterIfFalseState → ifFalse condition (correct: this is the state after the false branch ran)

In the no-else case (line 277), the call is wrong:

• aState = initialState → ifTrue condition (wrong: this is the state when the body was skipped)
• bState = afterIfTrueState → ifFalse condition (wrong: this is the state when the body ran)

Test case

Given this flat wasm function:

(func $test (param $cond i32)
  (local $x i32)
  (local.set $x (i32.const 10))
  (if (local.get $cond)
    (then
      (local.set $x (i32.const 42))
    )
  )
  (drop (local.get $x))
)

After building the Dataflow graph, the phi node for $x at the merge point should be:

• When condition is true (body ran): value = 42
• When condition is false (body skipped): value = 10

Actual result: The phi has the values swapped:

• When condition is true: value = 10
• When condition is false: value = 42

GTest output:

Expected equality of these values:
  ifTrueValue
    Which is: 10
  42
When condition is TRUE, the phi should select 42 (the value set in the
ifTrue body). Got 10 — mergeIf arguments are swapped in the no-else case.

Expected equality of these values:
  ifFalseValue
    Which is: 42
  10
When condition is FALSE, the phi should select 10 (the initial value,
since the body was skipped). Got 42 — mergeIf arguments are swapped in
the no-else case.

Fix

Swap the two arguments on line 277:

-      mergeIf(initialState, afterIfTrueState, condition, curr, locals);
+      mergeIf(afterIfTrueState, initialState, condition, curr, locals);

This makes the no-else case consistent with the if-else case: the first argument is the state after the true branch, and the second is the state for the false branch (which is just the initial state when there is no else body).