ast_cfg: Case-bound variables have wrong subscript

Bug #578068 reported by Matt Giuca on 2010-05-10
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mars
Critical
Matt Giuca

Bug Description

ast_cfg is tasked with converting the code to SSA, so all variables must be assigned at at most one point in the code.

The unpacking statements for case variables are not SSA, because the same qualified variable name may be used multiple times. This is because the variables are qualified with the index of the block in which the switch statement appears, not the block in which they are actually generated. Consider:

type AOrB:
    A(Int)
    B(Int)
def foo(x :: AOrB) :: Int:
    switch x:
        case A(y):
            return y
        case B(y):
            return y

This generates the code:

    Block 0:
        # Preds: []
        switch x:
            case A: goto 4
            case B: goto 5
    Block 1:
        # Preds: [3, 2]
        PHI $RET:1 [2 - "$RET:2", 3 - "$RET:3"]
        # Null block terminator.
    Block 2:
        # Preds: [4]
        PHI y:0 [4 - "$T:4"] # Note y:0, should be y:2
        $RET:2 = y:0
        goto 1
    Block 3:
        # Preds: [5]
        PHI y:0 [5 - "$T:5"] # Note y:0, should be y:3
        $RET:3 = y:0
        goto 1
    Block 4:
        # Preds: [0]
        $T:4 = x.A(0)
        goto 2
    Block 5:
        # Preds: [0]
        $T:5 = x.B(0)
        goto 3

In the above example, the variable y:0 is assigned at two distinct program points. Fix this by naming the unpacked variables after the block in which they are unpacked.

Related branches

Matt Giuca (mgiuca) wrote :

Note in ast_cfg, type_map_union has a check to test whether all variable names are unique. This is currently disabled due to this bug. Re-enable the check once the bug is fixed.

tags: added: code-generation
Matt Giuca (mgiuca) wrote :

Added explicit test cases for this condition (which won't actually test it until the above check in type_map_union is enabled, but then they will). compiler/switch.twobinds to test a valid one (same type), and semantic/dupvarcase2 to test an invalid one (different types).

Matt Giuca (mgiuca) on 2010-06-28
Changed in mars:
milestone: none → 0.3.1
Matt Giuca (mgiuca) on 2010-11-29
Changed in mars:
importance: High → Critical
Matt Giuca (mgiuca) on 2010-11-29
summary: - ast_cfg: Case variables are not single-assignment
+ ast_cfg: Case-bound variables have wrong subscript
Matt Giuca (mgiuca) wrote :

Fixed in trunk r1150.

Changed in mars:
status: Triaged → Fix Committed
Matt Giuca (mgiuca) on 2011-10-27
Changed in mars:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers