New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to inline function when imported directly #913

Closed
AndreaOrru opened this Issue Apr 12, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@AndreaOrru
Member

AndreaOrru commented Apr 12, 2018

The following function fails to compile:

fn moveCursor(self: &VGA) void {
    const outb = if (builtin.os == builtin.Os.zen)
        @import("std").os.zen.outb
    else
        @import("../kernel/x86.zig").outb;

    outb(0x3D4, 0x0F);
    outb(0x3D5, u8(self.cursor & 0xFF));
    outb(0x3D4, 0x0E);
    outb(0x3D5, u8((self.cursor >> 8) & 0xFF));
}
/home/andrea/src/zen/kernel/x86.zig:157:12: error: unable to inline function
pub inline fn outb(port: u16, value: u8) void {

However, the following (totally equivalent) code works:

fn moveCursor(self: &VGA) void {
    const zen = if (builtin.os == builtin.Os.zen)
        @import("std").os.zen
    else
        @import("../kernel/x86.zig");

    zen.outb(0x3D4, 0x0F);
    zen.outb(0x3D5, u8(self.cursor & 0xFF));
    zen.outb(0x3D4, 0x0E);
    zen.outb(0x3D5, u8((self.cursor >> 8) & 0xFF));
}

@AndreaOrru AndreaOrru changed the title from Unable to inline function (which should be inlinable) to Unable to inline function when imported directly Apr 12, 2018

@andrewrk andrewrk added this to the 0.3.0 milestone Apr 12, 2018

@andrewrk andrewrk added the bug label Apr 12, 2018

@andrewrk

This comment has been minimized.

Member

andrewrk commented Apr 12, 2018

I have a suspicion that we're using the wrong function name when asking LLVM if the inlining succeeded. Let me try to put together a test case.

@andrewrk

This comment has been minimized.

Member

andrewrk commented Apr 12, 2018

Oh, this is interesting. We actually are failing to resolve outb at comptime in this case. It's generating an indirect function call, which is certainly a bug.

@andrewrk

This comment has been minimized.

Member

andrewrk commented Apr 12, 2018

Oh, oh, oh. Never mind. Ok here's what's happening (with outb as an empty fn):

; Function Attrs: nobuiltin nounwind
define void @entry() #2 !dbg !41 {
Entry:
  %outb = alloca void ()*, align 8
  store void ()* @outb, void ()** %outb, align 8, !dbg !47
  call void @llvm.dbg.declare(metadata void ()** %outb, metadata !45, metadata !DIExpression()), !dbg !47
  call fastcc void @outb(), !dbg !48
  ret void, !dbg !50
}

and then it gets inlined:

; Function Attrs: nobuiltin nounwind
define void @entry() #2 !dbg !41 {
Entry:
  %outb = alloca void ()*, align 8
  store void ()* @outb, void ()** %outb, align 8, !dbg !47
  call void @llvm.dbg.declare(metadata void ()** %outb, metadata !45, metadata !DIExpression()), !dbg !47
  ret void, !dbg !48
}

So zig is generating correct llvm code here. We inline the call, but we also store the function in a local variable so that it is available in debug info, as expected. If you compile with --release-fast you do not get the compile error.

But the problem is that storing the function in a local variable causes llvm to not delete the function from the module and then we think that we failed to inline the function. So we need to either:

  • Detect this false positive and don't emit a compile error, or
  • Don't store a function pointer in a local variable of a function marked inline (and require references to inline functions to always be comptime known)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment