Skip to content
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

Arrays elements failing trivial comparison associated with inlined for loop #18723

Closed
lassade opened this issue Jan 28, 2024 · 1 comment
Closed
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@lassade
Copy link

lassade commented Jan 28, 2024

Zig Version

0.12.0-dev.2341+92211135f

Steps to Reproduce and Observed Behavior

Just run the exe built using zig build-exe from code bellow:

const std = @import("std");

const App = struct {
    scroll_pos: [2]u32 = .{ 0, 0 },
    dirty: bool = false,

    fn scrollGood(self: *App, offset: [2]i32) void {
        const x = self.scroll_pos[0];
        const y = self.scroll_pos[1];

        inline for (0..2) |i| {
            if (offset[i] >= 0) {
                self.scroll_pos[i] +|= @bitCast(offset[i]);
            } else {
                self.scroll_pos[i] -|= @bitCast(-offset[i]);
            }
        }

        std.log.info("{any} -> {any}", .{ .{ x, y }, self.scroll_pos });

        if (self.scroll_pos[0] != x or self.scroll_pos[1] != y) {
            self.dirty = true;
        }
    }

    fn scrollBad(self: *App, offset: [2]i32) void {
        const o = self.scroll_pos;

        inline for (0..2) |i| {
            if (offset[i] >= 0) {
                self.scroll_pos[i] +|= @bitCast(offset[i]);
            } else {
                self.scroll_pos[i] -|= @bitCast(-offset[i]);
            }
        }

        std.log.info("{any} -> {any}", .{ o, self.scroll_pos });

        if (self.scroll_pos[0] != o[0] or self.scroll_pos[1] != o[1]) {
            self.dirty = true;
        }
    }
};

pub fn main() void {
    {
        var app = App{};
        app.scrollGood(.{ 0, 1 });
        std.log.info("{}", .{app.dirty});
    }
    {
        var app = App{};
        app.scrollBad(.{ 0, 1 });
        std.log.info("{}", .{app.dirty});
    }
}

outputs:

info: { 0, 0 } -> { 0, 1 }
info: true
info: { 0, 0 } -> { 0, 1 }
info: false

Expected Behavior

the output of scrollGood and scrollBad should be identical

info: { 0, 0 } -> { 0, 1 }
info: true
info: { 0, 0 } -> { 0, 1 }
info: true
@lassade lassade added the bug Observed behavior contradicts documented or intended behavior label Jan 28, 2024
@Vexu Vexu added the backend-llvm The LLVM backend outputs an LLVM IR Module. label Jan 28, 2024
@Vexu Vexu added this to the 0.13.0 milestone Jan 28, 2024
@Vexu
Copy link
Member

Vexu commented Jan 28, 2024

Fixed by reverting #13972 @shwqf

diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig
index 10323b688c..f01c27042b 100644
--- a/src/codegen/llvm.zig
+++ b/src/codegen/llvm.zig
@@ -6382,34 +6382,6 @@ pub const FuncGen = struct {
                 const elem_alignment = elem_ty.abiAlignment(mod).toLlvm();
                 return self.loadByRef(elem_ptr, elem_ty, elem_alignment, .normal);
             } else {
-                if (bin_op.lhs.toIndex()) |lhs_index| {
-                    if (self.air.instructions.items(.tag)[@intFromEnum(lhs_index)] == .load) {
-                        const load_data = self.air.instructions.items(.data)[@intFromEnum(lhs_index)];
-                        const load_ptr = load_data.ty_op.operand;
-                        if (load_ptr.toIndex()) |load_ptr_index| {
-                            const load_ptr_tag = self.air.instructions.items(.tag)[@intFromEnum(load_ptr_index)];
-                            switch (load_ptr_tag) {
-                                .struct_field_ptr,
-                                .struct_field_ptr_index_0,
-                                .struct_field_ptr_index_1,
-                                .struct_field_ptr_index_2,
-                                .struct_field_ptr_index_3,
-                                => {
-                                    const load_ptr_inst = try self.resolveInst(load_ptr);
-                                    const gep = try self.wip.gep(
-                                        .inbounds,
-                                        array_llvm_ty,
-                                        load_ptr_inst,
-                                        &indices,
-                                        "",
-                                    );
-                                    return self.loadTruncate(.normal, elem_ty, gep, .default);
-                                },
-                                else => {},
-                            }
-                        }
-                    }
-                }
                 const elem_ptr =
                     try self.wip.gep(.inbounds, array_llvm_ty, array_llvm_val, &indices, "");
                 return self.loadTruncate(.normal, elem_ty, elem_ptr, .default);

Vexu added a commit to Vexu/zig that referenced this issue Jan 29, 2024
Vexu added a commit to Vexu/zig that referenced this issue Jan 29, 2024
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

3 participants