switches with a single case reuse temporary names

Bug #682536 reported by Matt Giuca
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mars
Fix Released
Critical
Matt Giuca

Bug Description

Normal switches end the current block by generating the result of the switch expression, then executing a 'switch' instruction to branch to another block where the temporaries are bound for the matched variables, finally branching to another block to bind the variables. A switch with a single case will be optimised by ast_cfg into a single block where a) the switch expression result is generated, and b) temporaries are bound for the matched variables.

However, the temporaries in (b) re-use the same names as the temporaries from (a). This is very bad -- we should not be re-using names in SSA code, especially since the second assignments usually have a different type (so the localtable is incorrect).

For example:

type Pair(a, b):
    Pair(a, b)

def switchpair() :: Int:
    switch Pair(1, 2):
        case Pair(x, y):
            pass
    return x

This generates the assembly code:
def switchpair() :: Int:
    var $T:0 :: Int # Also used as Pair(Int, Int)
    var $T:0:1 :: Int # Also used as (Int, Int) -> Pair(Int, Int)
    var $T:0:2 :: Int
    var $T:0:3 :: Int
    Block 0:
        # Preds: []
        $T:0:1 = Pair
        $T:0:2 = 1
        $T:0:3 = 2
        $T:0 = $T:0:1($T:0:2, $T:0:3)
        $T:0 = $T:0.[Pair: 0]
        $T:0:1 = $T:0.[Pair: 1]
        ....

This produces a run-time error: "Interpreter Error: Field reference to something not an ADT."

1. The ast_cfg should terminate with an internal error if it attempts to set a variable in the localtable which already exists.
2. It needs to generate fresh temporary names for the second part of that block.

Related branches

Revision history for this message
Matt Giuca (mgiuca) wrote :

Added test case in trunk r1146 (switch:switch_onecase). Oddly, this case actually doesn't run at all, due to this bug (it is simply omitted from the test output). Fix #1 (making an internal error) should turn this into a firm failing case.

description: updated
Revision history for this message
Matt Giuca (mgiuca) wrote :

Possible duplicate of bug #578068.

Revision history for this message
Matt Giuca (mgiuca) wrote :

In ast_cfg.type_map_union is some commented-out error checking code for bug #578068. If uncommented, it will catch this bug.

Revision history for this message
Matt Giuca (mgiuca) wrote :

OK this is actually not a duplicate of bug #578068. That relates to the wrong subscript being added to user-specified case-bound variables. This bug relates to temporaries within a block being repeated.

Revision history for this message
Matt Giuca (mgiuca) wrote :

Note that if you generate some more temporaries before the switch, the unpack temporaries do not start from 0; they start from the index which the switch started at:

    i = add(1, 2)
    switch Pair(1, 2):
        case Pair(x, y):
            pass

becomes:

    Block 0:
        # Preds: []
        $T:0 = 1
        $T:0:1 = 2
        i:0 = @add($T:0, $T:0:1)
        $T:0:3 = Pair
        $T:0:4 = 1
        $T:0:5 = 2
        $T:0:2 = $T:0:3($T:0:4, $T:0:5)
        $T:0:2 = $T:0:2.[Pair: 0]
        $T:0:3 = $T:0:2.[Pair: 1]

Note that temporaries 0 and 1 are used to compute i. Temporaries 2, 3, 4 and 5 are used to compute the switch value, and then temporaries 2 and 3 hold the unpack values. So it is using the subscript map from the beginning of the switch rather than the one after the switch value is generated.

Revision history for this message
Matt Giuca (mgiuca) wrote :

Fixed in trunk r1152.

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.