Hanging while performing PE.

[madmann91]

The following code hangs on master. Compile with impala -emit-thorin. It works on mem2reg, so maybe backporting fixes will be enough.

extern "C" {
    fn side_effect(&[u8]) -> ();
    fn side_effect_f32(f32) -> ();
}
struct FooBar {
    foo: i32,
    bar: f32
}

fn get_ptr(data: FooBar) -> &[u8] {
    &data.bar as &[u8]
}

extern fn crash_the_compiler(bar: f32) -> () {
    let data = make_foobar(bar);

    let ptr = &data.bar as &[u8];   // causes crash
    // let ptr = get_ptr(data);     // doesn't cause crash?
    // let ptr = @@get_ptr(data);   // doesn't cause crash?!

    side_effect(ptr);

    let sum = iterate(42f, data.foo, @ |x| x + 42f);    // data.foo is constant
                                                        // so this should not
                                                        // diverge
    side_effect_f32(sum);
}


fn @make_foobar(bar: f32) -> FooBar {
    FooBar {
        foo: 4,
        bar: bar
    }
}

fn @iterate(value: f32, n: i32, op: fn (f32) -> f32) -> f32 {
    if n > 0 {
        iterate(op(value), n - 1, op)
    } else {
        value
    }
}

[leissa]

Maybe the new resolve_loads in mem2reg does already the trick?

[madmann91]

It does but the issue is that it shouldn't: The value of data should not be known regardless, since side_effect is potentially writing into the slot (i.e. is_safe_slot should return false).

[madmann91]

OK after inspection it's perfectly correct. In Impala, emission is actually done by value for non mutable lets and therefore a separate slot is created for the bar member. This is nice but does not solve the problem in general, so we may want to make resolve_loads more clever still.

EDIT: making data mutable actually makes impala crash on mem2reg.

[madmann91]

OK, I have found a simple improvement in resolve_loads that partially addresses this problem. After the call to side_effect, information about data is lost. But even before the call, assuming we use the current criterion for safety, the value of data is unknown.

By changing the criterion to allow partially safe data in resolve_loads, one can now partially load data if a subset of an object is safe. So now (after commit d27f3c4 on mem2reg), the following code works:

extern "C" {
    fn side_effect(&[u8]) -> ();
}
struct FooBar {
    foo: i32,
    bar: f32
}

extern fn crash_the_compiler(bar: f32) -> () {
    let data = make_foobar(bar);

    let ptr = &data.bar as &[u8];
    let foo = data.foo; // this now works, was not working before
    // data.bar is unknown here, since the compiler assumes it might be
    // modified by side_effect() -- even if the call happens after...
    side_effect(ptr);
    // data is completely unknown here

    let sum = iterate(42f, foo, @ |x| x + 42f);
}

fn @make_foobar(bar: f32) -> FooBar {
    FooBar {
        foo: 4,
        bar: bar
    }
}

fn @iterate(value: f32, n: i32, op: fn (f32) -> f32) -> f32 {
    if n > 0 {
        iterate(op(value), n - 1, op)
    } else {
        value
    }
}

[madmann91]

To address the issue in the general case, we need a much smarter optimization than resolve_loads, probably implemented using the technique described in the paper: Composing dataflow analyses and transformations.

The current workaround is the following:

1. Use the mem2reg branch, and
2. Either:
   • Make data immutable, or
   • Make data mutable but make a local immutable copy before any side effect for parameters that have to be known at compile-time (e.g. data.foo)

[leissa]

Yes, I want to implement the new optimizer in my tuple branch.

[dexterlb]

I can confirm that the crash has been removed on mem2reg.

From what I gather, this is intended behaviour, since by marking data as immutable, we take on the assumption that the user ensures side_effect does not mutate?

As a side note, before mem2reg, this code also produced a crash:

extern "C" {
    fn side_effect(&[u8]) -> ();
    fn side_effect_f32(f32) -> ();
}

struct FooBar {
    foo: i32,
    bars: fn(i32) -> f32,
}

fn get_ptr(data: FooBar, i: i32) -> &[u8] {
    &data.bars(i) as &[u8]
}

extern fn crash_the_compiler(bars: &[f32], i: i32) -> () {
    let data = make_foobar(bars);

    let ptr = &data.bars(i) as &[u8];   // causes crash
    // let ptr = get_ptr(data, i);      // doesn't cause crash?
    // let ptr = @@get_ptr(data, i);    // doesn't cause crash?!

    side_effect(ptr);

    let sum = iterate(42f, data.foo, @ |x| x + 42f);    // data.foo is constant
                                                        // so this should not
                                                        // diverge
    side_effect_f32(sum);
}


fn @make_foobar(bars: &[f32]) -> FooBar {
    FooBar {
        foo: 4,
        bars: |i| bars(i)
    }
}

fn @iterate(value: f32, n: i32, op: fn (f32) -> f32) -> f32 {
    if n > 0 {
        iterate(op(value), n - 1, op)
    } else {
        value
    }
}

Which is even worse than the first example, since side_effect has absolutely no way of modifying the value of foo.