ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.45k stars 2.52k forks source link

Weird and inefficient codegen for initialization of packed struct with undefined values #11534

Open michalsieron opened 2 years ago

michalsieron commented 2 years ago

Zig Version

0.10.0-dev.1933+5f2d0d414

Steps to Reproduce

Compile following code with

$ zig build-obj example.zig --strip --name safe -OReleaseSafe
$ zig build-obj example.zig --strip --name small -OReleaseSmall
$ zig build-obj example.zig --strip --name fast -OReleaseFast

and then run objdump -d safe.o small.o fast.o


const MyPackedStruct = packed struct {
    field1: u17,
    field2: u15,
};

export fn packed_normal_assign(x: *MyPackedStruct) void {
    x.* = .{ .field1 = 2, .field2 = 3 };
}

export fn packed_assign_with_undefined(x: *MyPackedStruct) void {
    x.* = .{ .field1 = 2, .field2 = undefined };
}

Expected Behavior

All of compiled examples have the same or similar code. Or at least code that makes sense.

Actual Behavior

  1. Fast and Small modes for some reason set undefined bits to 1. They also do it inefficiently
  2. Safe mode doesn't set undefined bytes to 0xaa.

safe.o:     file format elf64-x86-64

Disassembly of section .text:

0000000000000000 <packed_normal_assign>:
   0:   c7 07 02 00 06 00       movl   $0x60002,(%rdi)
   6:   c3                      ret    
   7:   66 0f 1f 84 00 00 00    nopw   0x0(%rax,%rax,1)
   e:   00 00 

0000000000000010 <packed_assign_with_undefined>:
  10:   c7 07 02 00 00 00       movl   $0x2,(%rdi)
  16:   c3                      ret    

small.o:     file format elf64-x86-64

Disassembly of section .text:

0000000000000000 <packed_normal_assign>:
   0:   c7 07 02 00 06 00       movl   $0x60002,(%rdi)
   6:   c3                      ret    

0000000000000007 <packed_assign_with_undefined>:
   7:   b8 00 00 fe ff          mov    $0xfffe0000,%eax
   c:   23 07                   and    (%rdi),%eax
   e:   83 c8 02                or     $0x2,%eax
  11:   89 07                   mov    %eax,(%rdi)
  13:   c3                      ret    

fast.o:     file format elf64-x86-64

Disassembly of section .text:

0000000000000000 <packed_normal_assign>:
   0:   c7 07 02 00 06 00       movl   $0x60002,(%rdi)
   6:   c3                      ret    
   7:   66 0f 1f 84 00 00 00    nopw   0x0(%rax,%rax,1)
   e:   00 00 

0000000000000010 <packed_assign_with_undefined>:
  10:   b8 00 00 fe ff          mov    $0xfffe0000,%eax
  15:   23 07                   and    (%rdi),%eax
  17:   83 c8 02                or     $0x2,%eax
  1a:   89 07                   mov    %eax,(%rdi)
  1c:   c3                      ret    
michalsieron commented 2 years ago

If I change MyPackedStruct to have two u16 fields instead I get: for Fast and Small

0000000000000010 <packed_assign_with_undefined>:
  10:   66 c7 07 02 00          movw   $0x2,(%rdi)
  15:   c3                      ret

and for Safe

0000000000000010 <packed_assign_with_undefined>:
  10:   c7 07 02 00 aa aa       movl   $0xaaaa0002,(%rdi)
  16:   c3                      ret
michalsieron commented 2 years ago

When I change packed_assign_with_undefined to assign undefined to both fields I get:

for Fast and Small for both original and two u16 fields versions of MyPackedStruct

0000000000000010 <packed_assign_with_undefined>:
  10:   c3                      ret

for Safe for original MyPackedStruct definition

0000000000000010 <packed_assign_with_undefined>:
  10:   c7 07 00 00 00 00       movl   $0x0,(%rdi)
  16:   c3                      ret

and for Safe for MyPackedStruct definition with two u16 fields

0000000000000010 <packed_assign_with_undefined>:
  10:   c7 07 aa aa aa aa       movl   $0xaaaaaaaa,(%rdi)
  16:   c3                      ret

It's kinda weird, because in both cases we 'assign' 32 bits of undefined data and yet Safe mode behaves differently.

michalsieron commented 2 years ago

If I force packed_assign_with_undefined to compute value to assign in comptime then all modes generate correct code

export fn packed_assign_with_undefined(x: *MyPackedStruct) void {
    x.* = comptime blk: {
      break :blk MyPackedStruct{ .field1 = 2, .field2 = undefined };
    };
}

both u17/u15 and u16/u16 MyPackedStruct definitions, all modes:

0000000000000010 <packed_assign_with_undefined>:
  10:   c7 07 02 00 00 00       movl   $0x2,(%rdi)
  16:   c3                      ret

changing packed_assign_with_undefined to assign undefined to both fields (still computed in comptime): both u17/u15 and u16/u16 MyPackedStruct definitions, Fast and Small modes:

0000000000000010 <packed_assign_with_undefined>:
  10:   c3                      ret

both u17/u15 and u16/u16 MyPackedStruct definitions, Safe mode:

0000000000000010 <packed_assign_with_undefined>:
  10:   c7 07 aa aa aa aa       movl   $0xaaaaaaaa,(%rdi)
  16:   c3                      ret
Vexu commented 1 year ago

Packed structs work differently now and these produce the expected result except that safe modes don't set the bytes to 0xAA.