ast_cfg: Case-bound variables have wrong subscript

Bug #578068 reported by Matt Giuca
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mars
Fix Released
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

Revision history for this message
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
Revision history for this message
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)
Changed in mars:
milestone: none → 0.3.1
Matt Giuca (mgiuca)
Changed in mars:
importance: High → Critical
Matt Giuca (mgiuca)
summary: - ast_cfg: Case variables are not single-assignment
+ ast_cfg: Case-bound variables have wrong subscript
Revision history for this message
Matt Giuca (mgiuca) wrote :

Fixed in trunk r1150.

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

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.