From 3d88c3845fe0baede222e5aa2a8a336b30232f24 Mon Sep 17 00:00:00 2001 From: Bill Currie Date: Fri, 13 Mar 2020 18:20:38 +0900 Subject: [PATCH] [qfcc] Move struct copy/set into statement emission Doing it in the expression trees was a big mistake for a several reasons. For one, expression trees are meant to be target-agnostic, so they're the wrong place for selecting instruction types. Also, the move and memset expressions broke "a = b = c;" type expression chains. This fixes most things (including the assignchain test) with -Werror turned off (some issues in flow analysis uncovered by the nil migration: memset target not extracted). --- tools/qfcc/source/expr_assign.c | 106 ++------------------------------ tools/qfcc/source/statements.c | 106 ++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 102 deletions(-) diff --git a/tools/qfcc/source/expr_assign.c b/tools/qfcc/source/expr_assign.c index aa759b88d..465746983 100644 --- a/tools/qfcc/source/expr_assign.c +++ b/tools/qfcc/source/expr_assign.c @@ -263,29 +263,6 @@ assign_vector_expr (expr_t *dst, expr_t *src) return 0; } -static __attribute__((pure)) int -is_const_ptr (expr_t *e) -{ - if ((e->type != ex_value || e->e.value->lltype != ev_pointer) - || !(POINTER_VAL (e->e.value->v.pointer) >= 0 - && POINTER_VAL (e->e.value->v.pointer) < 65536)) { - return 1; - } - return 0; -} - -static __attribute__((pure)) int -is_indirect (expr_t *e) -{ - if (e->type == ex_block && e->e.block.result) - return is_indirect (e->e.block.result); - if (e->type == ex_expr && e->e.expr.op == '.') - return 1; - if (!(e->type == ex_uexpr && e->e.expr.op == '.')) - return 0; - return is_const_ptr (e->e.expr.e1); -} - static __attribute__((pure)) int is_memset (expr_t *e) { @@ -345,17 +322,14 @@ assign_expr (expr_t *dst, expr_t *src) src_type = get_type (src); } if (src->type == ex_bool) { + // boolean expressions are chains of tests, so extract the result + // of the tests src = convert_from_bool (src, dst_type); if (src->type == ex_error) { return src; } src_type = get_type (src); } - if (!is_struct (dst_type) && is_nil (src)) { - // nil is a type-agnostic 0 - src_type = dst_type; - convert_nil (src, src_type); - } if (!is_nil (src)) { if ((expr = check_types_compatible (dst, src))) { @@ -366,80 +340,8 @@ assign_expr (expr_t *dst, expr_t *src) if ((expr = assign_vector_expr (dst, src))) { return expr; } - } - - if (is_indirect (dst) && is_indirect (src)) { - debug (dst, "here"); - if (is_struct (src_type)) { - dst = address_expr (dst, 0, 0); - src = address_expr (src, 0, 0); - expr = new_move_expr (dst, src, src_type, 1); - } else { - expr_t *temp = new_temp_def_expr (dst_type); - - expr = new_block_expr (); - append_expr (expr, assign_expr (temp, src)); - append_expr (expr, assign_expr (dst, temp)); - expr->e.block.result = temp; - } - return expr; - } else if (is_indirect (dst)) { - debug (dst, "here"); - if (is_struct (dst_type)) { - dst = address_expr (dst, 0, 0); - if (is_nil (src)) { - return new_memset_expr (dst, new_integer_expr (0), dst_type); - } else { - src = address_expr (src, 0, 0); - return new_move_expr (dst, src, dst_type, 1); - } - } - if (is_nil (src)) { - src_type = dst_type; - convert_nil (src, src_type); - } - if (dst->type == ex_expr) { - if (get_type (dst->e.expr.e1) == &type_entity) { - dst_type = dst->e.expr.type; - dst->e.expr.type = pointer_type (dst_type); - dst->e.expr.op = '&'; - } - op = PAS; - } else { - if (is_const_ptr (dst->e.expr.e1)) { - dst = dst->e.expr.e1; - op = PAS; - } - } - } else if (is_indirect (src)) { - debug (dst, "here"); - if (is_struct (dst_type)) { - dst = address_expr (dst, 0, 0); - src = address_expr (src, 0, 0); - src->rvalue = 1; - return new_move_expr (dst, src, src_type, 1); - } - if (src->type == ex_uexpr) { - expr = src->e.expr.e1; - if (is_const_ptr (expr)) { - if (expr->type == ex_expr && expr->e.expr.op == '&' - && expr->e.expr.type->type == ev_pointer - && !is_constant (expr)) { - src = expr; - src->e.expr.op = '.'; - src->e.expr.type = src_type; - src->rvalue = 1; - } - } - } - } - - if (is_nil (src)) { - src_type = dst_type; - convert_nil (src, src_type); - } - if (is_struct (dst_type)) { - return new_move_expr (dst, src, dst_type, 0); + } else { + convert_nil (src, dst_type); } expr = new_binary_expr (op, dst, src); diff --git a/tools/qfcc/source/statements.c b/tools/qfcc/source/statements.c index e38248bed..4a3aff674 100644 --- a/tools/qfcc/source/statements.c +++ b/tools/qfcc/source/statements.c @@ -42,6 +42,7 @@ #include "qfalloca.h" #include "QF/alloc.h" +#include "QF/mathlib.h" #include "QF/va.h" #include "dags.h" @@ -615,18 +616,123 @@ statement_branch (sblock_t *sblock, expr_t *e) return sblock->next; } +static __attribute__((pure)) int +is_const_ptr (expr_t *e) +{ + if ((e->type != ex_value || e->e.value->lltype != ev_pointer) + || !(POINTER_VAL (e->e.value->v.pointer) >= 0 + && POINTER_VAL (e->e.value->v.pointer) < 65536)) { + return 1; + } + return 0; +} + +static __attribute__((pure)) int +is_indirect (expr_t *e) +{ + if (e->type == ex_block && e->e.block.result) + return is_indirect (e->e.block.result); + if (e->type == ex_expr && e->e.expr.op == '.') + return 1; + if (!(e->type == ex_uexpr && e->e.expr.op == '.')) + return 0; + return is_const_ptr (e->e.expr.e1); +} + +static sblock_t * +expr_assign_copy (sblock_t *sblock, expr_t *e, operand_t **op) +{ + statement_t *s; + expr_t *dst_expr = e->e.expr.e1; + expr_t *src_expr = e->e.expr.e2; + type_t *dst_type = get_type (dst_expr); + type_t *src_type = get_type (src_expr); + unsigned count; + expr_t *count_expr; + operand_t *dst = 0; + operand_t *src = 0; + operand_t *size = 0; + static const char *opcode_sets[][2] = { + {"", ""}, + {"", ""}, + }; + const unsigned max_count = 1 << 16; + const char **opcode_set = opcode_sets[0]; + const char *opcode; + int need_ptr = 0; + operand_t *dummy; + + if (src_expr->type == ex_expr + && (src_expr->e.expr.op == '=' || src_expr->e.expr.op == PAS)) { + sblock = statement_subexpr (sblock, src_expr, &dummy); + } + // follow the assignment chain to find the actual source expression + // (can't take the address of an assignment) + while (src_expr->type == ex_expr + && (src_expr->e.expr.op == '=' || src_expr->e.expr.op == PAS)) { + src_expr = src_expr->e.expr.e2; + } + if (src_expr->type == ex_nil) { + // switch to memset because nil is type agnostic 0 + src_expr = new_integer_expr (0); + opcode_set = opcode_sets[1]; + if (is_indirect (dst_expr)) { + need_ptr = 1; + dst_expr = address_expr (dst_expr, 0, 0); + } + } else { + if (is_indirect (dst_expr) || is_indirect (src_expr)) { + need_ptr = 1; + dst_expr = address_expr (dst_expr, 0, 0); + src_expr = address_expr (src_expr, 0, 0); + } + } + + if (type_size (dst_type) != type_size (src_type)) { + bug (e, "dst and src sizes differ in expr_assign_copy: %d %d", + type_size (dst_type), type_size (src_type)); + } + count = min (type_size (dst_type), type_size (src_type)); + if (count < (1 << 16)) { + count_expr = expr_file_line (new_short_expr (count), e); + } else { + count_expr = expr_file_line (new_integer_expr (count), e); + } + + if (count < max_count && !need_ptr) { + opcode = opcode_set[0]; + } else { + opcode = opcode_set[1]; + } + + s = new_statement (st_move, opcode, e); + sblock = statement_subexpr (sblock, dst_expr, &dst); + sblock = statement_subexpr (sblock, src_expr, &src); + sblock = statement_subexpr (sblock, count_expr, &size); + s->opa = src; + s->opb = size; + s->opc = dst; + sblock_add_statement (sblock, s); + return sblock; +} + static sblock_t * expr_assign (sblock_t *sblock, expr_t *e, operand_t **op) { statement_t *s; expr_t *src_expr = e->e.expr.e2; expr_t *dst_expr = e->e.expr.e1; + type_t *dst_type = get_type (dst_expr); operand_t *src = 0; operand_t *dst = 0; operand_t *ofs = 0; const char *opcode = convert_op (e->e.expr.op); st_type_t type; + if (is_structural (dst_type)) { + return expr_assign_copy (sblock, e, op); + } + if (e->e.expr.op == '=') { sblock = statement_subexpr (sblock, dst_expr, &dst); src = dst;