Merge pull request 'compiler: add testing for Air.Legalize pass' (#37410) from mlugg/legalize-tests into master

Reviewed-on: https://codeberg.org/ziglang/zig/pulls/37410

build.zig

@@ -110,6 +110,7 @@ pub fn build(b: *std.Build) !void {
     update_cpu_features_step.dependOn(&run_update_cpu_features.step);
 
     const no_matrix = b.option(bool, "no-matrix", "Limit test matrix to exactly one target configuration") orelse false;
+    const legalize_only = b.option(bool, "legalize-only", "Limit test matrix to target configurations where all legalizations are enabled") orelse false;
     const fuzz_only = b.option(bool, "fuzz-only", "Limit test matrix to one target suitable for fuzzing") orelse false;
     const skip_debug = b.option(bool, "skip-debug", "Main test suite skips debug builds") orelse false;
     const skip_release = b.option(bool, "skip-release", "Main test suite skips release builds") orelse no_matrix;
@@ -468,6 +469,8 @@ pub fn build(b: *std.Build) !void {
 
     const test_only: ?tests.ModuleTestOptions.TestOnly = if (no_matrix)
         .default
+    else if (legalize_only)
+        .legalize
     else if (fuzz_only)
         .{ .fuzz = optimize }
     else

lib/compiler/Maker/Step/Compile.zig

@@ -850,6 +850,8 @@ fn lowerZigArgs(
 
     try addFlag(gpa, zig_args, "incremental", conf_comp.flags4.incremental.toBool() orelse graph.incremental);
 
+    try addBool(gpa, zig_args, "-flegalize-all", conf_comp.flags3.legalize_all);
+
     try zig_args.append(gpa, "--listen=-");
 
     // Windows has an argument length limit of 32,766 characters, macOS 262,144 and Linux

lib/std/Build/Configuration.zig

@@ -1050,7 +1050,8 @@ pub const Step = extern struct {
             entry: Entry,
             lto: Lto,
             subsystem: Subsystem,
-            _: u2 = 0,
+            legalize_all: bool,
+            _: u1 = 0,
         };
 
         pub const Flags4 = packed struct(u32) {

lib/std/Build/Serialize.zig

@@ -187,6 +187,7 @@ pub fn write(b: *std.Build, wc: *Configuration.Wip, writer: *std.Io.Writer) !voi
                             },
                             .lto = .init(c.lto),
                             .subsystem = .init(c.subsystem),
+                            .legalize_all = c.legalize_all,
                         },
                         .flags4 = .{
                             .libc_file = c.libc_file != null,

lib/std/Build/Step/Compile.zig

@@ -241,6 +241,11 @@ sanitize_coverage_trace_pc_guard: ?bool = null,
 /// incremental compilation is slower but preserves previous build artifacts.
 incremental: ?bool = null,
 
+/// Corresponds to the `-flegalize-all` CLI flag.
+///
+/// This is a debug feature intended solely for Zig compiler development.
+legalize_all: bool = false,
+
 emit_directory: Configuration.OptionalGeneratedFileIndex = .none,
 generated_docs: Configuration.OptionalGeneratedFileIndex = .none,
 generated_asm: Configuration.OptionalGeneratedFileIndex = .none,

src/Air.zig

@@ -2541,34 +2541,34 @@ pub const CompilerRtFunc = enum(u32) {
     __divhf3, __divsf3, __divdf3, __divxf3, __divtf3,
 
     // float minmax
-    __fminh, fminf, fmin, __fminx, fminq,
-    __fmaxh, fmaxf, fmax, __fmaxx, fmaxq,
+    __fminh, fminf, fmin, __fminx, fminf128,
+    __fmaxh, fmaxf, fmax, __fmaxx, fmaxf128,
 
     // float round
-    __ceilh,  ceilf,  ceil,  __ceilx,  ceilq,
-    __floorh, floorf, floor, __floorx, floorq,
-    __trunch, truncf, trunc, __truncx, truncq,
-    __roundh, roundf, round, __roundx, roundq,
+    __ceilh,  ceilf,  ceil,  __ceilx,  ceilf128,
+    __floorh, floorf, floor, __floorx, floorf128,
+    __trunch, truncf, trunc, __truncx, truncf128,
+    __roundh, roundf, round, __roundx, roundf128,
 
     // float log
-    __logh,   logf,   log,   __logx,   logq,
-    __log2h,  log2f,  log2,  __log2x,  log2q,
-    __log10h, log10f, log10, __log10x, log10q,
+    __logh,   logf,   log,   __logx,   logf128,
+    __log2h,  log2f,  log2,  __log2x,  log2f128,
+    __log10h, log10f, log10, __log10x, log10f128,
 
     // float exp
-    __exph,  expf,  exp,  __expx,  expq,
-    __exp2h, exp2f, exp2, __exp2x, exp2q,
+    __exph,  expf,  exp,  __expx,  expf128,
+    __exp2h, exp2f, exp2, __exp2x, exp2f128,
 
     // float trigonometry
-    __sinh, sinf, sin, __sinx, sinq,
-    __cosh, cosf, cos, __cosx, cosq,
-    __tanh, tanf, tan, __tanx, tanq,
+    __sinh, sinf, sin, __sinx, sinf128,
+    __cosh, cosf, cos, __cosx, cosf128,
+    __tanh, tanf, tan, __tanx, tanf128,
 
     // float misc ops
-    __fabsh, fabsf, fabs, __fabsx, fabsq,
-    __sqrth, sqrtf, sqrt, __sqrtx, sqrtq,
-    __fmodh, fmodf, fmod, __fmodx, fmodq,
-    __fmah,  fmaf,  fma,  __fmax,  fmaq,
+    __fabsh, fabsf, fabs, __fabsx, fabsf128,
+    __sqrth, sqrtf, sqrt, __sqrtx, sqrtf128,
+    __fmodh, fmodf, fmod, __fmodx, fmodf128,
+    __fmah,  fmaf,  fma,  __fmax,  fmaf128,
 
     // float comparison
     __eqhf2, __eqsf2, __eqdf2, __eqxf2, __eqtf2, // == iff return == 0
@@ -2811,12 +2811,12 @@ pub const CompilerRtFunc = enum(u32) {
             .__sinx, .__cosx, .__tanx,
             .__fabsx, .__sqrtx, .__fmodx, .__fmax,
             => .f80,
-            .fminq, .fmaxq,
-            .ceilq, .floorq, .truncq, .roundq,
-            .logq, .log2q, .log10q,
-            .expq, .exp2q,
-            .sinq, .cosq, .tanq,
-            .fabsq, .sqrtq, .fmodq, .fmaq,
+            .fminf128, .fmaxf128,
+            .ceilf128, .floorf128, .truncf128, .roundf128,
+            .logf128, .log2f128, .log10f128,
+            .expf128, .exp2f128,
+            .sinf128, .cosf128, .tanf128,
+            .fabsf128, .sqrtf128, .fmodf128, .fmaf128,
             => .f128,
             // zig fmt: on
 

src/Air/Legalize.zig

@@ -325,6 +325,23 @@ pub const Feature = enum {
 
 pub const Features = std.enums.EnumSet(Feature);
 
+/// The maximal set of `Features` which may be simultaneously enabled.
+pub const all_features: Features = .complement(.initMany(&.{
+    // There are a few features that conflict with others, so we need to enable the more useful one.
+    .scalarize_add_safe, // conflicts with `.expand_add_safe`
+    .scalarize_sub_safe, // conflicts with `.expand_sub_safe`
+    .scalarize_mul_safe, // conflicts with `.expand_mul_safe`
+    .scalarize_int_cast_safe, // conflicts with `.expand_int_cast_safe`
+    .scalarize_div_ceil, // conflicts with `.expand_div_ceil`
+    .scalarize_div_ceil_optimized, // conflicts with `.expand_div_ceil_optimized`
+    .scalarize_int_from_float_safe, // conflicts with `.expand_int_from_float_safe`
+    .scalarize_int_from_float_optimized_safe, // conflicts with `.expand_int_from_float_optimized_safe`
+
+    // This feature introduces a new AIR form, and acts solely as an optimization (I'm not actually
+    // sure if it should be here at all), so is excluded from `-flegalize-all`.
+    .unsplat_shift_rhs,
+}));
+
 pub const Error = std.mem.Allocator.Error;
 
 pub fn legalize(air: *Air, pt: Zcu.PerThread, features: *const Features) Error!void {

src/Compilation/Config.zig

@@ -58,6 +58,7 @@ import_memory: bool,
 export_memory: bool,
 shared_memory: bool,
 is_test: bool,
+legalize_all: bool,
 debug_format: DebugFormat,
 root_optimize_mode: std.lang.Optimize,
 root_strip: bool,
@@ -111,6 +112,7 @@ pub const Options = struct {
     use_new_linker: ?bool = null,
     pie: ?bool = null,
     lto: ?std.zig.LtoMode = null,
+    legalize_all: bool = false,
     incremental: bool = false,
     /// WASI-only. Type of WASI execution model ("command" or "reactor").
     wasi_exec_model: ?std.lang.WasiExecModel = null,
@@ -161,6 +163,7 @@ pub const ResolveError = error{
     DllExportFnsRequiresWindows,
     NewLinkerIncompatibleWithLld,
     NewLinkerIncompatibleObjectFormat,
+    LegalizeAllRequiresLlvm,
 };
 
 pub fn resolve(options: Options) ResolveError!Config {
@@ -395,6 +398,10 @@ pub fn resolve(options: Options) ResolveError!Config {
             // There is no compiler backend available for this target.
             return error.ZigLacksTargetSupport;
         }
+
+        if (options.legalize_all and !use_llvm) {
+            return error.LegalizeAllRequiresLlvm;
+        }
     }
 
     // Make a decision on whether to use LLD or our own linker.
@@ -613,6 +620,7 @@ pub fn resolve(options: Options) ResolveError!Config {
         .output_mode = options.output_mode,
         .have_zcu = options.have_zcu,
         .is_test = options.is_test,
+        .legalize_all = options.legalize_all,
         .link_mode = link_mode,
         .link_libc = link_libc,
         .link_libcpp = link_libcpp,

src/codegen.zig

@@ -68,6 +68,9 @@ fn importBackend(comptime backend: std.lang.CompilerBackend) type {
 
 pub fn legalizeFeatures(pt: Zcu.PerThread, nav_index: InternPool.Nav.Index) ?*const Air.Legalize.Features {
     const zcu = pt.zcu;
+    if (zcu.comp.config.legalize_all) {
+        return &Air.Legalize.all_features;
+    }
     const target = &zcu.navFileScope(nav_index).mod.?.resolved_target.result;
     switch (target_util.zigBackend(target, zcu.comp.config.use_llvm)) {
         else => unreachable,

src/codegen/llvm/FuncGen.zig

@@ -350,8 +350,9 @@ fn genBody(self: *FuncGen, body: []const Air.Inst.Index, coverage_point: Air.Cov
             .legalize_vec_elem_val   => try self.airLegalizeVecElemVal(inst),
             .legalize_vec_store_elem => try self.airLegalizeVecStoreElem(inst),
 
-            // No soft float legalizations are enabled.
-            .legalize_compiler_rt_call => unreachable,
+            // No soft float legalizations are requested, but for testing we sometimes enable *all*
+            // legalizations in this backend (`-flegalize-all`), so we still need to handle this.
+            .legalize_compiler_rt_call => try self.airLegalizeCompilerRtCall(inst),
 
             .add            => try self.airAdd(inst, .normal),
             .add_optimized  => try self.airAdd(inst, .fast),
@@ -2241,10 +2242,22 @@ fn airArrayElemVal(self: *FuncGen, inst: Air.Inst.Index) Allocator.Error!Builder
 }
 
 fn airLegalizeVecElemVal(fg: *FuncGen, inst: Air.Inst.Index) Allocator.Error!Builder.Value {
+    const zcu = fg.object.zcu;
+
     const bin_op = fg.air.instructions.items(.data)[@backingInt(inst)].bin_op;
+
+    const vec_ty = fg.typeOf(bin_op.lhs);
+
     const vec = try fg.resolveInst(bin_op.lhs);
     const index = try fg.resolveInst(bin_op.rhs);
-    return fg.wip.extractElement(vec, index, "");
+
+    if (isByRef(vec_ty, zcu)) {
+        const elem_ty = vec_ty.childType(zcu);
+        const elem_ptr = try fg.ptraddScaled(vec, index, elem_ty.abiSize(zcu));
+        return fg.load(elem_ptr, elem_ty.abiAlignment(zcu), elem_ty, .normal);
+    } else {
+        return fg.wip.extractElement(vec, index, "");
+    }
 }
 fn airLegalizeVecStoreElem(fg: *FuncGen, inst: Air.Inst.Index) Allocator.Error!Builder.Value {
     const zcu = fg.object.zcu;
@@ -2261,13 +2274,65 @@ fn airLegalizeVecStoreElem(fg: *FuncGen, inst: Air.Inst.Index) Allocator.Error!B
     const index = try fg.resolveInst(extra.lhs);
     const elem = try fg.resolveInst(extra.rhs);
 
-    const old_vec = try fg.load(vec_ptr, ptr_align, vec_ty, .normal);
-    const new_vec = try fg.wip.insertElement(old_vec, elem, index, "");
-    try fg.store(vec_ptr, ptr_align, new_vec, vec_ty, .normal);
+    if (isByRef(vec_ty, zcu)) {
+        const elem_ty = vec_ty.childType(zcu);
+        const elem_ptr = try fg.ptraddScaled(vec_ptr, index, elem_ty.abiSize(zcu));
+        try fg.store(elem_ptr, ptr_align.min(elem_ty.abiAlignment(zcu)), elem, elem_ty, .normal);
+    } else {
+        const old_vec: Builder.Value = if (vec_ty.vectorLen(zcu) == 1) old_vec: {
+            // Workaround for https://github.com/llvm/llvm-project/issues/235439
+            const o = fg.object;
+            break :old_vec try o.builder.poisonValue(try o.lowerType(vec_ty, .as_value));
+        } else try fg.load(vec_ptr, ptr_align, vec_ty, .normal);
+        const new_vec = try fg.wip.insertElement(old_vec, elem, index, "");
+        try fg.store(vec_ptr, ptr_align, new_vec, vec_ty, .normal);
+    }
 
     return .none;
 }
 
+fn airLegalizeCompilerRtCall(fg: *FuncGen, inst: Air.Inst.Index) Allocator.Error!Builder.Value {
+    const zcu = fg.object.zcu;
+    const comp = zcu.comp;
+    const gpa = comp.gpa;
+    const target = comp.getTarget();
+    const builder = &fg.object.builder;
+
+    const compiler_rt_call = fg.air.unwrapCompilerRtCall(inst);
+
+    const param_types = try gpa.alloc(InternPool.Index, compiler_rt_call.args.len);
+    defer gpa.free(param_types);
+
+    const llvm_args = try gpa.alloc(Builder.Value, compiler_rt_call.args.len);
+    defer gpa.free(llvm_args);
+
+    for (param_types, llvm_args, compiler_rt_call.args) |*param_ty, *llvm_arg, arg_ref| {
+        param_ty.* = fg.typeOf(arg_ref).toIntern();
+        llvm_arg.* = try fg.resolveInst(arg_ref);
+    }
+
+    const fn_info: Object.FuncInfo = .{
+        .cc = compiler_rt_call.func.@"callconv"(target),
+        .param_types = param_types,
+        .return_type = compiler_rt_call.func.returnType().toIntern(),
+    };
+
+    const llvm_fn = try fg.object.getLibcFunction(
+        fg.pt,
+        try builder.strtabString(compiler_rt_call.func.name(target)),
+        fn_info,
+    );
+
+    return fg.buildCall(
+        .{},
+        llvm_fn.typeOf(builder),
+        llvm_fn.toValue(builder),
+        fn_info,
+        fn_info.param_types,
+        llvm_args,
+    );
+}
+
 fn airPtrElemVal(self: *FuncGen, inst: Air.Inst.Index) Allocator.Error!Builder.Value {
     const zcu = self.object.zcu;
     const bin_op = self.air.instructions.items(.data)[@backingInt(inst)].bin_op;

src/main.zig

@@ -793,6 +793,7 @@ const compile_usage =
     \\Debug Options (Zig Compiler Development):
     \\  -fopt-bisect-limit=[limit]   Only run [limit] first LLVM optimization passes
     \\  -fstack-report               Print stack size diagnostics
+    \\  -flegalize-all               Force-enable all codegen legalizations for testing
     \\  --debug-log [scope]          Enable printing debug/info log messages for scope
     \\  --debug-compile-errors       Crash with helpful diagnostics at the first compile error
     \\  --debug-link-snapshot        Dump linker state and output file information for troubleshooting
@@ -1498,6 +1499,8 @@ fn buildOutputType(
                         test_execve = true;
                     } else if (mem.eql(u8, arg, "-fstack-report")) {
                         stack_report = true;
+                    } else if (mem.eql(u8, arg, "-flegalize-all")) {
+                        create_module.opts.legalize_all = true;
                     } else if (mem.eql(u8, arg, "-fPIC")) {
                         mod_opts.pic = true;
                     } else if (mem.eql(u8, arg, "-fno-PIC")) {
@@ -4328,6 +4331,7 @@ fn createModule(
             error.DllExportFnsRequiresWindows => fatal("only Windows OS targets support DLLs", .{}),
             error.NewLinkerIncompatibleWithLld => fatal("using the new linker is incompatible with using lld", .{}),
             error.NewLinkerIncompatibleObjectFormat => fatal("no new linker available for '{t}' files", .{target.ofmt}),
+            error.LegalizeAllRequiresLlvm => fatal("'-flegalize-all' requires the LLVM backend", .{}),
         };
     }
 

test/tests.zig

@@ -29,6 +29,10 @@ const ModuleTestTarget = struct {
     function_sections: ?bool = null,
     data_sections: ?bool = null,
     skip_modules: []const []const u8 = &.{},
+    /// When this is set, module tests with `.no_builtin = true` are skipped, because `-fno-builtin`
+    /// is likely to conflict with some IR legalizations (e.g. those which convert operations to
+    /// compiler_rt calls).
+    legalize_all: bool = false,
 
     // This is intended for targets that, for any reason, shouldn't be run as part of a normal test
     // invocation. This could be because of a slow backend, requiring a newer LLVM version, being
@@ -1760,6 +1764,30 @@ const module_test_targets = blk: {
             },
             .link_libc = true,
         },
+
+        // Targets with `.legalize_all = true`, to test the compiler's IR legalization passes
+        //
+        // Running these tests is likely to be significantly slower than the corresponding target
+        // without `-flegalize-all`, so avoid adding entries here without good reason. It's useful
+        // to have coverage for both big-endian and little-endian though.
+        .{
+            .target = .{
+                .cpu_arch = .x86_64,
+                .os_tag = .linux,
+                .abi = .none,
+            },
+            .use_llvm = true,
+            .legalize_all = true,
+        },
+        .{
+            .target = .{
+                .cpu_arch = .s390x,
+                .os_tag = .linux,
+                .abi = .none,
+            },
+            .use_llvm = true,
+            .legalize_all = true,
+        },
     };
 };
 
@@ -2560,7 +2588,8 @@ pub const ModuleTestOptions = struct {
     build_options: ?*Step.Options = null,
 
     pub const TestOnly = union(enum) {
-        default: void,
+        default,
+        legalize,
         fuzz: OptimizeMode,
     };
 };
@@ -2568,11 +2597,12 @@ pub const ModuleTestOptions = struct {
 pub fn addModuleTests(b: *std.Build, options: ModuleTestOptions) *Step {
     const step = b.step(b.fmt("test-{s}", .{options.name}), options.desc);
 
-    if (options.test_only) |test_only| {
+    const only_legalize: bool = if (options.test_only) |test_only| only_legalize: {
         const test_target: ModuleTestTarget = switch (test_only) {
             .default => .{
                 .link_libc = if (std.mem.eql(u8, options.name, "libc")) true else null,
             },
+            .legalize => break :only_legalize true,
             .fuzz => |optimize| .{
                 .optimize_mode = optimize,
                 .use_llvm = true,
@@ -2583,7 +2613,7 @@ pub fn addModuleTests(b: *std.Build, options: ModuleTestOptions) *Step {
         addOneModuleTest(b, step, test_target, &resolved_target, triple_txt, options);
 
         return step;
-    }
+    } else false;
 
     for_targets: for (module_test_targets) |test_target| {
         if (test_target.skip_modules.len > 0) {
@@ -2644,6 +2674,16 @@ pub fn addModuleTests(b: *std.Build, options: ModuleTestOptions) *Step {
         if (options.skip_single_threaded and test_target.single_threaded == true)
             continue;
 
+        if (options.no_builtin and test_target.legalize_all) {
+            // `-flegalize-all` is likely to conflict with `-fno-builtin`, due to e.g. legalization
+            // passes which convert arithmetic operations to compiler_rt calls.
+            continue;
+        }
+
+        if (only_legalize and !test_target.legalize_all) {
+            continue;
+        }
+
         const want_this_mode = for (options.optimize_modes) |m| {
             if (m == test_target.optimize_mode) break true;
         } else false;
@@ -2694,6 +2734,7 @@ fn addOneModuleTest(
     }
     if (test_target.function_sections) |fs| these_tests.function_sections = fs;
     if (test_target.data_sections) |ds| these_tests.data_sections = ds;
+    these_tests.legalize_all = test_target.legalize_all;
     const single_threaded_suffix = if (test_target.single_threaded == true) "-single" else "";
     const backend_suffix = if (test_target.use_llvm == true)
         "-llvm"