From 703c6684d103f14193411b589eaa0e0b1e1189f0 Mon Sep 17 00:00:00 2001 From: Sahnvour Date: Mon, 19 Nov 2018 22:22:21 +0100 Subject: [PATCH] Crash fixes and small improvements to inline asm. (#1756) * codegen: LLVMConstInlineAsm is deprecated. * codegen: replace commas in asm constraint strings by pipes as required by LLVM. * ir: enforce usage of '=' constraint modifier for inline assembly outputs. Others are not currently supported and this was just asserted alter in `ir_render_asm`. * asm: forbid comptime_int/floats as inputs in favor of explicitely sized constants. Fixes a crash due to comptime_int/floats having no type_ref. * asm: handle inputs with integers of <8 or non power of 2 bitsize. We widen them to the next highest power of two. --- src/buffer.hpp | 10 ++++++++++ src/codegen.cpp | 31 +++++++++++++++++++++++++++---- src/ir.cpp | 23 +++++++++++++++++++++-- src/util.hpp | 11 +++++++++++ test/cases/asm.zig | 24 ++++++++++++++++++++++++ test/compile_errors.zig | 28 ++++++++++++++++++++++++++++ 6 files changed, 121 insertions(+), 6 deletions(-) diff --git a/src/buffer.hpp b/src/buffer.hpp index 8155df87a1..afe9fd8a91 100644 --- a/src/buffer.hpp +++ b/src/buffer.hpp @@ -181,5 +181,15 @@ static inline Slice buf_to_slice(Buf *buf) { return Slice{reinterpret_cast(buf_ptr(buf)), buf_len(buf)}; } +static inline void buf_replace(Buf* buf, char from, char to) { + const size_t count = buf_len(buf); + char* ptr = buf_ptr(buf); + for (size_t i = 0; i < count; ++i) { + char& l = ptr[i]; + if (l == from) + l = to; + } +} + #endif diff --git a/src/codegen.cpp b/src/codegen.cpp index bd9063d020..40f71e38fe 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3660,6 +3660,13 @@ static LLVMValueRef ir_render_asm(CodeGen *g, IrExecutable *executable, IrInstru AsmOutput *asm_output = asm_expr->output_list.at(i); bool is_return = (asm_output->return_type != nullptr); assert(*buf_ptr(asm_output->constraint) == '='); + // LLVM uses commas internally to separate different constraints, + // alternative constraints are achieved with pipes. + // We still allow the user to use commas in a way that is similar + // to GCC's inline assembly. + // http://llvm.org/docs/LangRef.html#constraint-codes + buf_replace(asm_output->constraint, ',', '|'); + if (is_return) { buf_appendf(&constraint_buf, "=%s", buf_ptr(asm_output->constraint) + 1); } else { @@ -3679,14 +3686,30 @@ static LLVMValueRef ir_render_asm(CodeGen *g, IrExecutable *executable, IrInstru } for (size_t i = 0; i < asm_expr->input_list.length; i += 1, total_index += 1, param_index += 1) { AsmInput *asm_input = asm_expr->input_list.at(i); + buf_replace(asm_input->constraint, ',', '|'); IrInstruction *ir_input = instruction->input_list[i]; buf_append_buf(&constraint_buf, asm_input->constraint); if (total_index + 1 < total_constraint_count) { buf_append_char(&constraint_buf, ','); } - param_types[param_index] = ir_input->value.type->type_ref; - param_values[param_index] = ir_llvm_value(g, ir_input); + ZigType *const type = ir_input->value.type; + LLVMTypeRef type_ref = type->type_ref; + LLVMValueRef value_ref = ir_llvm_value(g, ir_input); + // Handle integers of non pot bitsize by widening them. + if (type->id == ZigTypeIdInt) { + const size_t bitsize = type->data.integral.bit_count; + if (bitsize < 8 || !is_power_of_2(bitsize)) { + const bool is_signed = type->data.integral.is_signed; + const size_t wider_bitsize = bitsize < 8 ? 8 : round_to_next_power_of_2(bitsize); + ZigType *const wider_type = get_int_type(g, is_signed, wider_bitsize); + type_ref = wider_type->type_ref; + value_ref = gen_widen_or_shorten(g, false, type, wider_type, value_ref); + } + } + + param_types[param_index] = type_ref; + param_values[param_index] = value_ref; } for (size_t i = 0; i < asm_expr->clobber_list.length; i += 1, total_index += 1) { Buf *clobber_buf = asm_expr->clobber_list.at(i); @@ -3705,8 +3728,8 @@ static LLVMValueRef ir_render_asm(CodeGen *g, IrExecutable *executable, IrInstru LLVMTypeRef function_type = LLVMFunctionType(ret_type, param_types, (unsigned)input_and_output_count, false); bool is_volatile = asm_expr->is_volatile || (asm_expr->output_list.length == 0); - LLVMValueRef asm_fn = LLVMConstInlineAsm(function_type, buf_ptr(&llvm_template), - buf_ptr(&constraint_buf), is_volatile, false); + LLVMValueRef asm_fn = LLVMGetInlineAsm(function_type, buf_ptr(&llvm_template), buf_len(&llvm_template), + buf_ptr(&constraint_buf), buf_len(&constraint_buf), is_volatile, false, LLVMInlineAsmDialectATT); return LLVMBuildCall(g->builder, asm_fn, param_values, (unsigned)input_and_output_count, ""); } diff --git a/src/ir.cpp b/src/ir.cpp index 03389a2320..a62da827bc 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -5537,6 +5537,15 @@ static IrInstruction *ir_gen_asm_expr(IrBuilder *irb, Scope *scope, AstNode *nod return irb->codegen->invalid_instruction; } } + + const char modifier = *buf_ptr(asm_output->constraint); + if (modifier != '=') { + add_node_error(irb->codegen, node, + buf_sprintf("invalid modifier starting output constraint for '%s': '%c', only '=' is supported." + " Compiler TODO: see https://github.com/ziglang/zig/issues/215", + buf_ptr(asm_output->asm_symbolic_name), modifier)); + return irb->codegen->invalid_instruction; + } } for (size_t i = 0; i < node->data.asm_expr.input_list.length; i += 1) { AsmInput *asm_input = node->data.asm_expr.input_list.at(i); @@ -15386,9 +15395,19 @@ static IrInstruction *ir_analyze_instruction_asm(IrAnalyze *ira, IrInstructionAs } for (size_t i = 0; i < asm_expr->input_list.length; i += 1) { - input_list[i] = asm_instruction->input_list[i]->child; - if (type_is_invalid(input_list[i]->value.type)) + IrInstruction *const input_value = asm_instruction->input_list[i]->child; + if (type_is_invalid(input_value->value.type)) return ira->codegen->invalid_instruction; + + if (instr_is_comptime(input_value) && + (input_value->value.type->id == ZigTypeIdComptimeInt || + input_value->value.type->id == ZigTypeIdComptimeFloat)) { + ir_add_error_node(ira, input_value->source_node, + buf_sprintf("expected sized integer or sized float, found %s", buf_ptr(&input_value->value.type->name))); + return ira->codegen->invalid_instruction; + } + + input_list[i] = input_value; } IrInstruction *result = ir_build_asm(&ira->new_irb, diff --git a/src/util.hpp b/src/util.hpp index a78231324f..cc83022328 100644 --- a/src/util.hpp +++ b/src/util.hpp @@ -158,6 +158,17 @@ static inline bool is_power_of_2(uint64_t x) { return x != 0 && ((x & (~x + 1)) == x); } +static inline uint64_t round_to_next_power_of_2(uint64_t x) { + --x; + x |= x >> 1; + x |= x >> 2; + x |= x >> 4; + x |= x >> 8; + x |= x >> 16; + x |= x >> 32; + return x + 1; +} + uint32_t int_hash(int i); bool int_eq(int a, int b); uint32_t uint64_hash(uint64_t i); diff --git a/test/cases/asm.zig b/test/cases/asm.zig index 1c4208f403..63e37c857c 100644 --- a/test/cases/asm.zig +++ b/test/cases/asm.zig @@ -17,6 +17,30 @@ test "module level assembly" { } } +test "output constraint modifiers" { + // This is only testing compilation. + var a: u32 = 3; + asm volatile ("" : [_]"=m,r"(a) : : ""); + asm volatile ("" : [_]"=r,m"(a) : : ""); +} + +test "alternative constraints" { + // Make sure we allow commas as a separator for alternative constraints. + var a: u32 = 3; + asm volatile ("" : [_]"=r,m"(a) : [_]"r,m"(a) : ""); +} + +test "sized integer/float in asm input" { + asm volatile ("" : : [_]"m"(usize(3)) : ""); + asm volatile ("" : : [_]"m"(i15(-3)) : ""); + asm volatile ("" : : [_]"m"(u3(3)) : ""); + asm volatile ("" : : [_]"m"(i3(3)) : ""); + asm volatile ("" : : [_]"m"(u121(3)) : ""); + asm volatile ("" : : [_]"m"(i121(3)) : ""); + asm volatile ("" : : [_]"m"(f32(3.17)) : ""); + asm volatile ("" : : [_]"m"(f64(3.17)) : ""); +} + extern fn aoeu() i32; export fn derp() i32 { diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 16690daf29..c5575a0c0b 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -5232,4 +5232,32 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { , ".tmp_source.zig:3:36: error: @ArgType could not resolve the type of arg 0 because 'fn(var)var' is generic", ); + + cases.add( + "unsupported modifier at start of asm output constraint", + \\export fn foo() void { + \\ var bar: u32 = 3; + \\ asm volatile ("" : [baz]"+r"(bar) : : ""); + \\} + , + ".tmp_source.zig:3:5: error: invalid modifier starting output constraint for 'baz': '+', only '=' is supported. Compiler TODO: see https://github.com/ziglang/zig/issues/215", + ); + + cases.add( + "comptime_int in asm input", + \\export fn foo() void { + \\ asm volatile ("" : : [bar]"r"(3) : ""); + \\} + , + ".tmp_source.zig:2:35: error: expected sized integer or sized float, found comptime_int", + ); + + cases.add( + "comptime_float in asm input", + \\export fn foo() void { + \\ asm volatile ("" : : [bar]"r"(3.17) : ""); + \\} + , + ".tmp_source.zig:2:35: error: expected sized integer or sized float, found comptime_float", + ); }