stage2: vectorized overflow arithmetic, integer overflow safety, left-shift overflow safety #11316

sengir · 2022-03-26T23:30:48Z

Sorry for yet another probably too big PR, but the changes were dependent on each other, and I didn't want to wait for the first to get merged before PRing the rest.

This breaks basically everything that's not LLVM.

matu3ba

add a few more test cases

matu3ba · 2022-03-27T08:44:04Z

test/behavior/vector.zig

+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
+


missing tests: u8 underflow, i8 overflow and underflow.
edge cases: u1, u0

andrewrk · 2022-03-27T19:42:16Z

cc @kubkon @joachimschmidt557 - could we prioritize getting the overflow arithmetic AIR instructions working in the backends so that we can move forward with this without regressing too many x86 and ARM test cases?

Perhaps even an initial, inefficient implementation that stores the result tuple into a stack allocation, and then a follow-up enhancement that tries to avoid this?

kubkon · 2022-03-27T19:44:05Z

SGTM! Will do just that for x64 tomorrow!

joachimschmidt557 · 2022-03-27T19:48:33Z

I'm currently working on implementing the relevant instructions for ARM. My progress in on the branch I usually use (https://github.com/joachimschmidt557/zig/tree/stage2-arm). I can't promise anything, but I'll probably be done with all instructions for all integers <= 32 bits in a couple of days.

matu3ba · 2022-03-29T10:08:38Z

related issue #6017

sengir · 2022-04-23T11:44:44Z

Rebased! I fixed LLVM's vector codegen, but the CBE and the Arms are missing a couple things still.

kubkon · 2022-05-09T06:29:02Z

@wsengir rebased on top of master - only CBE should be missing now! 🎉

andrewrk · 2022-05-11T21:43:59Z

I'll take a look at the CBE today.

andrewrk · 2022-05-12T20:35:13Z

only CBE should be missing now!

Looks like the behavior tests are failing for the native wasm backend in this branch too. I'm working on the C backend right now.

Luukdegram · 2022-05-12T20:47:32Z

only CBE should be missing now!

Looks like the behavior tests are failing for the native wasm backend in this branch too. I'm working on the C backend right now.

Trying my best to have them pass by tomorrow. Jakub brought the incorrect shifting behavior to my attention already, but haven’t had the time to fix it yet.

andrewrk · 2022-05-13T02:28:15Z

✔️ C backend

andrewrk · 2022-05-13T02:57:20Z

@kubkon mind having a look?

thread 17862 panic: reached unreachable code
Analyzing /workspace/test/behavior/try.zig: behavior/try.zig:test.try on error union
      %43 = decl_val("tryOnErrorUnionImpl") 
      %44 = dbg_stmt(5, 28)
      %45 = call(.auto, %43, []) 
    > %46 = is_non_err(%45) 
      %47 = condbr(%46, {
        %49 = err_union_payload_unsafe(%45) 
        %52 = break(%48, %49)
      }, {
        %50 = err_union_code(%45) 
        %51 = ret_node(%50) 
      }) 
    For full context, use the command
      zig ast-check -t /workspace/test/behavior/try.zig

  in /workspace/test/behavior/try.zig: behavior/try.zig:test.try on error union
    > %48 = block({%43..%47}) 

/workspace/_debug/staging/lib/zig/std/debug.zig:260:14: 0x6d1155b in std.debug.assert (zig)
    if (!ok) unreachable; // assertion failure
             ^
/workspace/_debug/staging/lib/zig/std/array_list.zig:365:19: 0x6daa276 in std.array_list.ArrayListAligned(u8,null).addOneAssumeCapacity (zig)
            assert(self.items.len < self.capacity);
                  ^
/workspace/_debug/staging/lib/zig/std/array_list.zig:176:59: 0x6da010a in std.array_list.ArrayListAligned(u8,null).appendAssumeCapacity (zig)
            const new_item_ptr = self.addOneAssumeCapacity();
                                                          ^
/workspace/src/arch/x86_64/bits.zig:324:39: 0x7cc48fd in arch.x86_64.bits.Encoder.modRm (zig)
        self.code.appendAssumeCapacity(
                                      ^
/workspace/src/arch/x86_64/bits.zig:335:19: 0x7b981ed in arch.x86_64.bits.Encoder.modRm_direct (zig)
        self.modRm(0b11, reg_or_opx, rm);
                  ^
/workspace/src/arch/x86_64/Emit.zig:1903:33: 0x79fb6ff in arch.x86_64.Emit.lowerToMrEnc (zig)
            encoder.modRm_direct(reg.lowId(), dst_reg.lowId());
                                ^
/workspace/src/arch/x86_64/Emit.zig:428:32: 0x782433a in arch.x86_64.Emit.mirTest (zig)
            return lowerToMrEnc(.@"test", RegisterOrMemory.reg(ops.reg1), ops.reg2, emit.code);
                               ^
/workspace/src/arch/x86_64/Emit.zig:179:41: 0x76306c3 in arch.x86_64.Emit.lowerMir (zig)
            .@"test" => try emit.mirTest(inst),
                                        ^
/workspace/src/arch/x86_64/CodeGen.zig:346:18: 0x744a9fb in arch.x86_64.CodeGen.generate (zig)
    emit.lowerMir() catch |err| switch (err) {
                 ^
/workspace/src/codegen.zig:114:70: 0x743d7ec in codegen.generateFunction (zig)
        .x86_64 => return @import("arch/x86_64/CodeGen.zig").generate(bin_file, src_loc, func, air, liveness, code, debug_output),
                                                                     ^
/workspace/src/link/MachO.zig:3706:37: 0x7250124 in link.MachO.updateFunc (zig)
        try codegen.generateFunction(&self.base, decl.srcLoc(), func, air, liveness, &code_buffer, .{
                                    ^
/workspace/src/link.zig:468:77: 0x70917df in link.File.updateFunc (zig)
            .macho => return @fieldParentPtr(MachO, "base", base).updateFunc(module, func, air, liveness),
                                                                            ^
/workspace/src/Module.zig:3707:41: 0x7070cbb in Module.ensureFuncBodyAnalyzed (zig)
            mod.comp.bin_file.updateFunc(mod, func, air, liveness) catch |err| switch (err) {
                                        ^
/workspace/src/Sema.zig:20444:36: 0x74ba8b3 in Sema.ensureFuncBodyAnalyzed (zig)
    sema.mod.ensureFuncBodyAnalyzed(func) catch |err| {
                                   ^
/workspace/src/Sema.zig:22229:40: 0x74ba0ba in Sema.resolveInferredErrorSet (zig)
        try sema.ensureFuncBodyAnalyzed(ies.func);
                                       ^
/workspace/src/Sema.zig:20649:49: 0x75d3a5d in Sema.analyzeIsNonErr (zig)
                try sema.resolveInferredErrorSet(block, src, ies);
                                                ^
/workspace/src/Sema.zig:12345:32: 0x73ce666 in Sema.zirIsNonErr (zig)
    return sema.analyzeIsNonErr(block, inst_data.src(), operand);
                               ^
/workspace/src/Sema.zig:756:66: 0x725baeb in Sema.analyzeBodyInner (zig)
            .is_non_err                   => try sema.zirIsNonErr(block, inst),
                                                                 ^
/workspace/src/Sema.zig:4139:34: 0x75e3bd2 in Sema.resolveBlockBody (zig)
        if (sema.analyzeBodyInner(child_block, body)) |_| {
                                 ^
/workspace/src/Sema.zig:4122:33: 0x742f8fd in Sema.zirBlock (zig)
    return sema.resolveBlockBody(parent_block, src, &child_block, body, inst, &label.merges);
                                ^
/workspace/src/Sema.zig:1200:69: 0x7265ed9 in Sema.analyzeBodyInner (zig)
                if (!block.is_comptime) break :blk try sema.zirBlock(block, inst);
                                                                    ^
/workspace/src/Sema.zig:595:30: 0x724d73a in Sema.analyzeBody (zig)
    _ = sema.analyzeBodyInner(block, body) catch |err| switch (err) {
                             ^
/workspace/src/Module.zig:4939:21: 0x708ef04 in Module.analyzeFnBody (zig)
    sema.analyzeBody(&inner_block, fn_info.body) catch |err| switch (err) {
                    ^
/workspace/src/Module.zig:3681:40: 0x70709a6 in Module.ensureFuncBodyAnalyzed (zig)
            var air = mod.analyzeFnBody(func, sema_arena) catch |err| switch (err) {
                                       ^
/workspace/src/Compilation.zig:2777:42: 0x6e1d954 in Compilation.processOneJob (zig)
            module.ensureFuncBodyAnalyzed(func) catch |err| switch (err) {
                                         ^
/workspace/src/Compilation.zig:2719:30: 0x6e09c49 in Compilation.performAllTheWork (zig)
            try processOneJob(comp, work_item);
                             ^
/workspace/src/Compilation.zig:2107:31: 0x6e02897 in Compilation.update (zig)
    try comp.performAllTheWork(main_progress_node);
                              ^
/workspace/src/main.zig:3104:20: 0x6d6c49f in updateModule (zig)
    try comp.update();
                   ^
/workspace/src/main.zig:2793:17: 0x6d3424f in buildOutputType (zig)
    updateModule(gpa, comp, hook) catch |err| switch (err) {
                ^
/workspace/src/main.zig:225:31: 0x6d19b89 in mainArgs (zig)
        return buildOutputType(gpa, arena, args, .zig_test);
                              ^
/workspace/src/main.zig:174:20: 0x6d18b86 in main (zig)
    return mainArgs(gpa, arena, args);
                   ^
/workspace/_debug/staging/lib/zig/std/start.zig:581:37: 0x7203cc7 in std.start.callMain (zig)
            const result = root.main() catch |err| {
                                    ^
/workspace/_debug/staging/lib/zig/std/start.zig:515:12: 0x6d1b72e in std.start.callMainWithArgs (zig)
    return @call(.{ .modifier = .always_inline }, callMain, .{});
           ^
/workspace/_debug/staging/lib/zig/std/start.zig:480:12: 0x6d1b4d4 in std.start.main (zig)
    return @call(.{ .modifier = .always_inline }, callMainWithArgs, .{ @intCast(usize, c_argc), c_argv, envp });
           ^
???:?:?: 0x7e84c54 in ??? (???)
test...The following command terminated unexpectedly:
/workspace/stage2/bin/zig test -fno-stage1 -fno-LLVM /workspace/test/behavior.zig --test-name-prefix behavior-x86_64-macos-gnu-Debug--multi-stage2_x86_64  --cache-dir /workspace/zig-cache --global-cache-dir /root/.cache/zig --name test -fno-single-threaded -target x86_64-macos-gnu -mcpu x86_64 --test-no-exec -I /workspace/test --zig-lib-dir /workspace/lib --enable-cache

kubkon · 2022-05-13T06:06:04Z

@andrewrk on it!

kubkon · 2022-05-13T07:22:14Z

@andrewrk x64-macos fixed. We still get an implicit decl error with CBE though:

error(compilation): clang failed with stderr: ./test_case.c:637:33: error: incompatible pointer types passing 'bool *' to parameter of type 'int *' [-Werror,-Wincompatible-pointer-types]
./test_case.c:174:70: note: passing argument to parameter 'overflow' here
./test_case.c:1142:12: error: implicit declaration of function '__shlodi4' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
./test_case.c:1157:12: error: implicit declaration of function '__shloti4' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
./test_case.c:1202:12: error: implicit declaration of function '__ushlodi4' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
./test_case.c:1212:12: error: implicit declaration of function '__ushloti4' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

andrewrk · 2022-05-13T07:29:24Z

Ah yes I'll take care of that tomorrow. I'll need to add some new compiler-rt routines for these functions. Unless... someone else happens to do it before I wake up 🙈

Luukdegram · 2022-05-13T21:44:34Z

It's now passing for the native Wasm backend also.

kubkon · 2022-05-14T20:26:56Z

@joachimschmidt557 FYI that I had to disable recursive fibonacci test for now.

kubkon · 2022-05-14T20:27:35Z

@andrewrk should be good to go!

andrewrk · 2022-05-14T23:38:08Z

Looks like the next problem is more C backend stuff

Most of the work here was additions to zig.h. The lowering code is mainly responsible for calling the correct function name depending on the operand type. Some of the compiler-rt calls here are not implemented yet and are non-standard symbols due to the C programming language not needing them. After this commit, the behavior tests with -ofmt=c are passing again.

The implementation for add_with_overflow and sub_with_overflow is now a lot more robust and takes account for signed integers and arbitrary integer bitsizes. The final output is equal to that of the LLVM backend.

This re-implements the shl_with_overflow operation from scratch, making it a lot more robust and outputs the equal code to the LLVM backend.

* zig_addo_u128: fix type-o * redo the shift-left overflow inline functions. no need to depend on compiler-rt.

This avoids the following error: ``` error: incompatible pointer types passing 'int64_t *' (aka 'long long *') to parameter of type 'long *' overflow = __builtin_saddl_overflow(lhs, rhs, res); ^~~ ``` My previous understanding was that this error would not occur because prior to this line we check that int64_t is equivalent to long, like this: ```c ``` However, it appears that this is still a warning in C if int64_t is primarily aliased to `long long`, even though `long` and `long long` are the same thing.

sengir force-pushed the stage2-overflow-safety branch from 6d8d0cb to dd6a581 Compare March 26, 2022 23:39

sengir mentioned this pull request Mar 26, 2022

Sema: panic on int add/sub/mul overflow #11174

Closed

sengir force-pushed the stage2-overflow-safety branch from dd6a581 to fc47c9f Compare March 27, 2022 00:03

matu3ba suggested changes Mar 27, 2022

View reviewed changes

kubkon mentioned this pull request Mar 28, 2022

x64: implement add, sub and mul with overflow #11334

Merged

joachimschmidt557 mentioned this pull request Apr 1, 2022

stage2 ARM: implement overflow operations for ints <= 32 bits #11373

Merged

sengir force-pushed the stage2-overflow-safety branch from 4cd7cbe to 3f8f3c5 Compare April 23, 2022 10:26

kubkon mentioned this pull request May 4, 2022

stage2,aarch64: basic overflow arithmetic support #11574

Merged

4 tasks

Luukdegram mentioned this pull request May 7, 2022

stage2: wasm - Improve @mulWithOverflow implementation #11605

Merged

kubkon mentioned this pull request May 7, 2022

stage2: introduce new register locking mechanism, and fix x64, arm and aarch64 native backends for compliance with integer overflow safety PR #11608

Merged

kubkon force-pushed the stage2-overflow-safety branch from 3f8f3c5 to d7cfc23 Compare May 9, 2022 06:28

andrewrk self-assigned this May 11, 2022

andrewrk approved these changes May 13, 2022

View reviewed changes

andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels May 14, 2022

kubkon force-pushed the stage2-overflow-safety branch from ad8ab4e to 70dbe2f Compare May 14, 2022 19:26

kubkon removed their assignment May 14, 2022

andrewrk self-assigned this May 14, 2022

sengir and others added 22 commits May 16, 2022 13:55

stage2: fix {add,sub,mul}_with_overflow vectorization in LLVM backend

c641fb8

stage2: vectorize shl_with_overflow in LLVM backend

c2cb9b7

stage2: add global Type constant for u1

ca1ab38

stage2: handle vectors in Value.intFitsInType

6b5c879

stage2: clean up creation of boolean Values

e8117ba

stage2: perform comptime vectorization of *_with_overflow in Value

86a928c

Sema: vectorize overflow arithmetic

eb06c78

Sema: implement integer overflow safety for add, sub, mul

c2980f3

stage2: implement runtime safety checks for shl_exact

afc714d

stage2: add vectorized overflow arithmetic behavior tests

21be3d9

stage2: add more vector overflow tests

bb3532e

LLVM: correctly pad result tuple of airOverflow

a5ea22d

disable 5 failing stage2_wasm tests

316bf4f

x64: fix capacity prealloc limit in lowerToMrEnc helper

b94d165

wasm: Improve overflow add/sub for ints <= 64bits

0a2d3d4

The implementation for add_with_overflow and sub_with_overflow is now a lot more robust and takes account for signed integers and arbitrary integer bitsizes. The final output is equal to that of the LLVM backend.

wasm: Improve shl_with_overflow

160aa4c

This re-implements the shl_with_overflow operation from scratch, making it a lot more robust and outputs the equal code to the LLVM backend.

zig.h: improve overflow shl

a84be7e

* zig_addo_u128: fix type-o * redo the shift-left overflow inline functions. no need to depend on compiler-rt.

aarch64: sub_with_overflow should always track V flag

852c820

arm: sub_with_overflow should always track V flag

7f96ca1

arm: disable recursive fibonacci

a0de0ad

andrewrk force-pushed the stage2-overflow-safety branch from b526572 to f33b3fc Compare May 16, 2022 21:38

andrewrk merged commit 5888446 into ziglang:master May 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stage2: vectorized overflow arithmetic, integer overflow safety, left-shift overflow safety #11316

stage2: vectorized overflow arithmetic, integer overflow safety, left-shift overflow safety #11316

sengir commented Mar 26, 2022 •

edited

Loading

matu3ba left a comment

matu3ba Mar 27, 2022

andrewrk commented Mar 27, 2022 •

edited

Loading

kubkon commented Mar 27, 2022

joachimschmidt557 commented Mar 27, 2022

matu3ba commented Mar 29, 2022

sengir commented Apr 23, 2022

kubkon commented May 9, 2022

andrewrk commented May 11, 2022

andrewrk commented May 12, 2022

Luukdegram commented May 12, 2022

andrewrk commented May 13, 2022

andrewrk commented May 13, 2022

kubkon commented May 13, 2022

kubkon commented May 13, 2022

andrewrk commented May 13, 2022

Luukdegram commented May 13, 2022

kubkon commented May 14, 2022

kubkon commented May 14, 2022

andrewrk commented May 14, 2022

stage2: vectorized overflow arithmetic, integer overflow safety, left-shift overflow safety #11316

stage2: vectorized overflow arithmetic, integer overflow safety, left-shift overflow safety #11316

Conversation

sengir commented Mar 26, 2022 • edited Loading

matu3ba left a comment

Choose a reason for hiding this comment

matu3ba Mar 27, 2022

Choose a reason for hiding this comment

andrewrk commented Mar 27, 2022 • edited Loading

kubkon commented Mar 27, 2022

joachimschmidt557 commented Mar 27, 2022

matu3ba commented Mar 29, 2022

sengir commented Apr 23, 2022

kubkon commented May 9, 2022

andrewrk commented May 11, 2022

andrewrk commented May 12, 2022

Luukdegram commented May 12, 2022

andrewrk commented May 13, 2022

andrewrk commented May 13, 2022

kubkon commented May 13, 2022

kubkon commented May 13, 2022

andrewrk commented May 13, 2022

Luukdegram commented May 13, 2022

kubkon commented May 14, 2022

kubkon commented May 14, 2022

andrewrk commented May 14, 2022

sengir commented Mar 26, 2022 •

edited

Loading

andrewrk commented Mar 27, 2022 •

edited

Loading