LLVM poisons partially undef constant, leading to infinite loops.

[eddyb]

#![feature(asm)]

// Isomorphic to Option<usize>.
#[derive(Copy)]
enum Foo {
    Yes(usize),
    No
}

// All these functions are necessary to trigger the bug.
fn swap(x: &mut Foo, y: &mut Foo) {
    let tmp = *x;
    *x = *y;
    *y = tmp;
}

fn replace(x: &mut Foo, mut y: Foo) -> Foo {
    swap(x, &mut y);
    y
}

fn take(x: &mut Foo) -> Foo {
    static NO_FOO: Foo = Foo::No;
    replace(x, NO_FOO)
}

fn take2(x: &mut Foo) -> Foo {
    take(x)
}

fn main() {
    let mut iter = Foo::Yes(5);
    while let Foo::Yes(_) = take2(&mut iter) {
        unsafe {
            // Just to keep the loop alive.
            asm!(""::::"volatile");
        }
    }
}

This happens only after the recent LLVM upgrade. The culprit seems to be half of NO_FOO:

@NO_FOO = internal constant { i64, [8 x i8] } { i64 1, [8 x i8] undef }
; ...
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %3, i8* bitcast ({ i64, [8 x i8] }* @NO_FOO to i8*), i64 16, i32 8, i1 false)

Changing the memcpy call to copy only 8 bytes instead of 16 (to not touch the undef portion) removes the infinite loop.
cc @dotdash @Aatch

[eddyb]

One workaround would be to replace undef with zeroinitializer but I'm not sure if that will cause a hit on performance.

[dotdash]

I haven't fully analyzed this, but that changing the memcpy size helps is coincidental. That just breaks a memcpy optimization that is required for the problem to show up, because the memcpy sizes now differ. AFAICT for now, this is some problem with aliasing.

[dotdash]

So looking through the relevant changes in LLVM, I noticed that the docs for noalias have been refined. And now they clarify that using noalias on the return value is only meant for malloc like functions, i.e. functions that return "new" pointers, not previously accessible to the caller at all. We use it for functions returning unique pointers and functions that return through sret pointers. Seems bad.

See http://llvm.org/docs/LangRef.html#noalias

Testing a patch now.

[dotdash]

FWIW, removing noalias from the no-opt IR for the given testcase does indeed fix the problem.

[jdm]

Nice find!

[dotdash]

@eddyb Fun fact, our suspicion that our usage of lifetime intrinsics was at fault for a bunch of failures might have been totally off. Different usage of lifetimes might just have killed different memcpy optimizations, hiding the real culprit lol I still need to fix them to allow for better optimization though :-/

[dotdash]

So here's the minimal LLVM IR I have so far:

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-linux-gnu"

%struct.S = type { i64, i64 }

@take.s = internal constant %struct.S { i64 1, i64 0 }

define internal void @swap(%struct.S* noalias %x, %struct.S* noalias %y) {
  %tmp = alloca %struct.S
  %1 = bitcast %struct.S* %tmp to i8*
  %2 = bitcast %struct.S* %x to i8*
  %3 = bitcast %struct.S* %y to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %1, i8* %2, i64 16, i32 8, i1 false)
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %2, i8* %3, i64 16, i32 8, i1 false)
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %3, i8* %1, i64 16, i32 8, i1 false)
  ret void
}

; Function Attrs: nounwind
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1) #0

define internal void @replace(%struct.S* noalias sret %agg.result, %struct.S* noalias %x, %struct.S* noalias %y) {
  call void @swap(%struct.S* %x, %struct.S* %y)
  %1 = bitcast %struct.S* %agg.result to i8*
  %2 = bitcast %struct.S* %y to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %1, i8* %2, i64 16, i32 8, i1 false)
  ret void
}

define internal void @take(%struct.S* noalias sret %agg.result, %struct.S* noalias %x) {
  %arg = alloca %struct.S
  %1 = bitcast %struct.S* %arg to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %1, i8* bitcast (%struct.S* @take.s to i8*), i64 16, i32 8, i1 false)
  call void @replace(%struct.S* sret %agg.result, %struct.S* %x, %struct.S* %arg)
  ret void
}

define void @take2(%struct.S* noalias sret %agg.result, %struct.S* noalias %x) {
  call void @take(%struct.S* sret %agg.result, %struct.S* %x)
  ret void
}

attributes #0 = { nounwind }

Removing thenoalias attribute on %y on replace() makes the problem go away, but having noalias there doesn't seem wrong to me.

[dotdash]

Pretty sure this is a bug in the memcpyopt pass that doesn't update the scoped alias data.

[dotdash]

http://reviews.llvm.org/D7482