From 768113d37b2cd076bf34d4cc0d45550407d6f368 Mon Sep 17 00:00:00 2001 From: Bill Currie Date: Sun, 18 Feb 2024 20:56:41 +0900 Subject: [PATCH] [qfcc] Replace expr_file_line with a scoped version Two birds with one stone: eliminates most of the problems with going const-correct with expr_t, and it make dealing with internally generated expressions at random locations much easier as the set source location affects all new expressions created within that scope, to any depth. Debug output is much easier to read now. --- tools/qfcc/include/expr.h | 18 +++---- tools/qfcc/source/constfold.c | 7 +-- tools/qfcc/source/def.c | 2 +- tools/qfcc/source/expr.c | 84 ++++++++++++++++++++++----------- tools/qfcc/source/expr_bool.c | 11 ++--- tools/qfcc/source/expr_obj.c | 5 +- tools/qfcc/source/method.c | 3 +- tools/qfcc/source/statements.c | 28 ++++------- tools/qfcc/test/ptrstructinit.r | 58 ++++++++++++----------- 9 files changed, 122 insertions(+), 94 deletions(-) diff --git a/tools/qfcc/include/expr.h b/tools/qfcc/include/expr.h index d92d38149..b4cd191a2 100644 --- a/tools/qfcc/include/expr.h +++ b/tools/qfcc/include/expr.h @@ -367,6 +367,16 @@ const expr_t *param_mismatch (const expr_t *e, int param, const char *fn, const type_t *t1, const type_t *t2); const expr_t *test_error (const expr_t *e, const type_t *t); +/** Set the current source location for subsequent new expressions. + + Returns error expression with saved source location +*/ +expr_t *set_src_loc (const expr_t *e); +void restore_src_loc (expr_t **e); +#define scoped_src_loc(e) \ + __attribute__((cleanup(restore_src_loc))) \ + expr_t *srclocScope = set_src_loc(e) + extern expr_t *local_expr; /** Get the type descriptor of the expression result. @@ -407,14 +417,6 @@ expr_t *expr_prepend_list (expr_t *list, ex_list_t *prepend); */ expr_t *new_expr (void); -/** Copy source expression's file and line to the destination expression - - \param dst The expression to receive the file and line - \param src The expression from which the file and line will be taken - \return \a dst -*/ -expr_t *expr_file_line (expr_t *dst, const expr_t *src); - /** Create a new label name. The label name is guaranteed to be unique to the compilation. It is made diff --git a/tools/qfcc/source/constfold.c b/tools/qfcc/source/constfold.c index 2bfec9f60..ed1ea6ff0 100644 --- a/tools/qfcc/source/constfold.c +++ b/tools/qfcc/source/constfold.c @@ -148,8 +148,8 @@ convert_to_float (const expr_t *e) if (is_float(get_type (e))) return e; + scoped_src_loc (e); return cast_expr (&type_float, e); - //return expr_file_line (n, e); } #if 0 static const expr_t * @@ -158,8 +158,8 @@ convert_to_double (const expr_t *e) if (is_double(get_type (e))) return e; + scoped_src_loc (e); expr_t *n = cast_expr (&type_double, e); - return expr_file_line (n, e); } #endif static const expr_t * @@ -387,7 +387,8 @@ do_op_vector (int op, const expr_t *e, const expr_t *e1, const expr_t *e2) if (!valid_op (op, sv_valid)) return error (e1, "invalid operator for scalar-vector"); - auto t = expr_file_line (new_binary_expr (op, e2, e1), e); + scoped_src_loc (e); + auto t = new_binary_expr (op, e2, e1); t->expr.type = e->expr.type; return do_op_vector (op, e, e2, e1); } diff --git a/tools/qfcc/source/def.c b/tools/qfcc/source/def.c index 748ea9285..4ca7acefc 100644 --- a/tools/qfcc/source/def.c +++ b/tools/qfcc/source/def.c @@ -531,11 +531,11 @@ init_field_def (def_t *def, const expr_t *init, storage_class_t storage, symbol_t *sym = init->symbol; symbol_t *field = symtab_lookup (pr.entity_fields, sym->name); if (field) { + scoped_src_loc (init); auto new = new_field_expr (0, field->type, field->s.def); if (new->type != ex_value) { internal_error (init, "expected value expression"); } - //FIXME init = expr_file_line (new, init); init = new; } } diff --git a/tools/qfcc/source/expr.c b/tools/qfcc/source/expr.c index 8962db60c..8f73214d2 100644 --- a/tools/qfcc/source/expr.c +++ b/tools/qfcc/source/expr.c @@ -260,6 +260,32 @@ new_expr (void) return e; } +void +restore_src_loc (expr_t **e) +{ + if (*e) { + pr.source_line = (*e)->line; + pr.source_file = (*e)->file; + + FREE (exprs, *e); + } +} + +expr_t * +set_src_loc (const expr_t *e) +{ + if (!e) { + return nullptr; + } + // save the current source location + auto n = new_expr (); + n->type = ex_error; + + pr.source_line = e->line; + pr.source_file = e->file; + return n; +} + ex_listitem_t * new_listitem (const expr_t *e) { @@ -395,14 +421,6 @@ list_gather (ex_list_t *list, const expr_t **exprs, int count) } } -expr_t * -expr_file_line (expr_t *dst, const expr_t *src) -{ - dst->file = src->file; - dst->line = src->line; - return dst; -} - const char * new_label_name (void) { @@ -499,6 +517,7 @@ new_label_ref (const ex_label_t *label) expr_t * new_block_expr (const expr_t *old) { + scoped_src_loc (old); expr_t *b = new_expr (); b->type = ex_block; @@ -513,7 +532,6 @@ new_block_expr (const expr_t *old) } b->block.result = old->block.result; b->block.is_call = old->block.is_call; - expr_file_line (b, old); } b->block.return_addr = __builtin_return_address (0); return b; @@ -891,8 +909,8 @@ constant_expr (const expr_t *e) } else { return e; } - auto new = new_value_expr (value); - return expr_file_line ((expr_t *) new, e);//FIXME cast + scoped_src_loc (e); + return new_value_expr (value); } int @@ -1541,7 +1559,8 @@ field_expr (const expr_t *e1, const expr_t *e2) internal_error (e2, "unexpected field exression"); } auto fv = new_field_val (e2->value->v.pointer.val + field->s.offset, field->type, e2->value->v.pointer.def); - e2 = expr_file_line ((expr_t *) new_value_expr (fv), e2); + scoped_src_loc (e2); + e2 = new_value_expr (fv); // create a new . expression return field_expr (e1, e2); } else { @@ -1594,7 +1613,8 @@ convert_from_bool (const expr_t *e, const type_t *type) const expr_t * convert_nil (const expr_t *e, const type_t *t) { - auto nil = expr_file_line (new_expr (), e); + scoped_src_loc (e); + auto nil = new_expr (); nil->type = ex_nil; nil->nil = t; return nil; @@ -2046,11 +2066,14 @@ bitnot_expr: } break; case '.': - if (extract_type (e) != ev_ptr) - return error (e, "invalid type for unary ."); - auto new = new_unary_expr ('.', e); - new->expr.type = get_type (e)->t.fldptr.type; - return expr_file_line (new, e); + { + if (extract_type (e) != ev_ptr) + return error (e, "invalid type for unary ."); + scoped_src_loc (e); + auto new = new_unary_expr ('.', e); + new->expr.type = get_type (e)->t.fldptr.type; + return new; + } case '+': if (!is_math (get_type (e))) return error (e, "invalid type for unary +"); @@ -2197,7 +2220,8 @@ build_function_call (const expr_t *fexpr, const type_t *ftype, const expr_t *par if (ftype->t.func.num_params < 0) { emit_args = !ftype->t.func.no_va_list; } - call = expr_file_line (new_block_expr (0), fexpr); + scoped_src_loc (fexpr); + call = new_block_expr (0); call->block.is_call = 1; int arg_expr_count = 0; const expr_t *arg_exprs[arg_count][2]; @@ -2211,15 +2235,16 @@ build_function_call (const expr_t *fexpr, const type_t *ftype, const expr_t *par auto e = arguments[i]; if (e->type == ex_compound) { - e = expr_file_line (initialized_temp_expr (arg_types[i], e), e); + scoped_src_loc (e); + e = initialized_temp_expr (arg_types[i], e); } // FIXME this is target-specific info and should not be in the // expression tree // That, or always use a temp, since it should get optimized out if (has_function_call (e)) { + scoped_src_loc (e); auto cast = cast_expr (arg_types[i], e); auto tmp = new_temp_def_expr (arg_types[i]); - tmp = expr_file_line (tmp, e); expr_prepend_expr (args, tmp); arg_exprs[arg_expr_count][0] = cast; @@ -2235,14 +2260,14 @@ build_function_call (const expr_t *fexpr, const type_t *ftype, const expr_t *par expr_prepend_expr (args, new_args_expr ()); } for (int i = 0; i < arg_expr_count - 1; i++) { + scoped_src_loc (arg_exprs[i][0]); auto assign = assign_expr (arg_exprs[i][1], arg_exprs[i][0]); - //append_expr (call, expr_file_line (assign, arg_exprs[i][0])); append_expr (call, assign); } if (arg_expr_count) { + scoped_src_loc (arg_exprs[arg_expr_count - 1][0]); auto e = assign_expr (arg_exprs[arg_expr_count - 1][1], arg_exprs[arg_expr_count - 1][0]); - //e = expr_file_line (e, arg_exprs[arg_expr_count - 1][0]); append_expr (call, e); } auto ret_type = ftype->t.func.type; @@ -2383,7 +2408,8 @@ return_expr (function_t *f, const expr_t *e) } if (e->type == ex_compound) { - e = expr_file_line (initialized_temp_expr (ret_type, e), e); + scoped_src_loc (e); + e = initialized_temp_expr (ret_type, e); } else { e = algebra_optimize (e); } @@ -2479,12 +2505,14 @@ conditional_expr (const expr_t *cond, const expr_t *e1, const expr_t *e2) if (c->type == ex_error) return c; - expr_t *block = expr_file_line (new_block_expr (0), cond); + scoped_src_loc (cond); + + expr_t *block = new_block_expr (0); auto type1 = get_type (e1); auto type2 = get_type (e2); - expr_t *tlabel = expr_file_line (new_label_expr (), cond); - expr_t *flabel = expr_file_line (new_label_expr (), cond); - expr_t *elabel = expr_file_line (new_label_expr (), cond); + expr_t *tlabel = new_label_expr (); + expr_t *flabel = new_label_expr (); + expr_t *elabel = new_label_expr (); backpatch (c->boolean.true_list, tlabel); backpatch (c->boolean.false_list, flabel); diff --git a/tools/qfcc/source/expr_bool.c b/tools/qfcc/source/expr_bool.c index 46928e1dc..6dd3c9d13 100644 --- a/tools/qfcc/source/expr_bool.c +++ b/tools/qfcc/source/expr_bool.c @@ -78,6 +78,7 @@ test_expr (const expr_t *e) auto type = get_type (e); if (e->type == ex_error) return e; + scoped_src_loc (e); switch (type->type) { case ev_type_count: internal_error (e, 0); @@ -127,12 +128,12 @@ test_expr (const expr_t *e) } return e; } - new = expr_file_line ((expr_t *) new_zero_expr (type), e); - new = expr_file_line ((expr_t *) binary_expr (NE, e, new), e); + new = new_zero_expr (type); + new = binary_expr (NE, e, new); return test_expr (new); case ev_double: - new = expr_file_line ((expr_t *) new_zero_expr (type), e); - new = expr_file_line ((expr_t *) binary_expr (NE, e, new), e); + new = new_zero_expr (type); + new = binary_expr (NE, e, new); return test_expr (new); case ev_vector: new = new_zero_expr (&type_vector); @@ -155,9 +156,7 @@ test_expr (const expr_t *e) } return test_error (e, get_type (e)); } - new = expr_file_line ((expr_t *) new, e); new = binary_expr (NE, e, new); - new = expr_file_line ((expr_t *) new, e); return new; } diff --git a/tools/qfcc/source/expr_obj.c b/tools/qfcc/source/expr_obj.c index a48a79ac2..7ca3c7ee2 100644 --- a/tools/qfcc/source/expr_obj.c +++ b/tools/qfcc/source/expr_obj.c @@ -228,7 +228,8 @@ message_expr (const expr_t *receiver, keywordarg_t *message) if (method) return_type = method->type->t.func.type; - expr_t *args = expr_file_line (new_list_expr (0), receiver); + scoped_src_loc (receiver); + expr_t *args = new_list_expr (0); for (m = message; m; m = m->next) { if (m->expr && m->expr->list.head) { expr_append_list (args, &m->expr->list); @@ -237,7 +238,7 @@ message_expr (const expr_t *receiver, keywordarg_t *message) expr_append_expr (args, selector); expr_append_expr (args, receiver); - send_msg = expr_file_line (send_message (super), receiver); + send_msg = send_message (super); if (method) { const expr_t *err; if ((err = method_check_params (method, args))) diff --git a/tools/qfcc/source/method.c b/tools/qfcc/source/method.c index e60ffd2b9..1be97dc3a 100644 --- a/tools/qfcc/source/method.c +++ b/tools/qfcc/source/method.c @@ -728,7 +728,8 @@ method_check_params (method_t *method, const expr_t *args) const type_t *arg_type = i < param_count ? mtype->t.func.param_types[i] : nullptr; if (e->type == ex_compound) { - e = expr_file_line (initialized_temp_expr (arg_type, e), e); + scoped_src_loc (e); + e = initialized_temp_expr (arg_type, e); } auto t = get_type (e); if (!t) { diff --git a/tools/qfcc/source/statements.c b/tools/qfcc/source/statements.c index 972400616..1333e2201 100644 --- a/tools/qfcc/source/statements.c +++ b/tools/qfcc/source/statements.c @@ -746,8 +746,8 @@ expr_address (sblock_t *sblock, const expr_t *e, operand_t **op) } else if (offset && is_constant (offset)) { int o = expr_int (offset); if (o < 32768 && o >= -32768) { + scoped_src_loc (offset); offset = new_short_expr (o); - //offset = expr_file_line (new_short_expr (o), offset); } } @@ -851,6 +851,8 @@ expr_assign_copy (sblock_t *sblock, const expr_t *e, operand_t **op, operand_t * operand_t *def = nullptr; operand_t *kill = nullptr; + scoped_src_loc (e); + if ((src && src->op_type == op_nil) || src_expr->type == ex_nil) { // switch to memset because nil is type agnostic 0 and structures // can be any size @@ -866,7 +868,6 @@ expr_assign_copy (sblock_t *sblock, const expr_t *e, operand_t **op, operand_t * } } else { if (is_indirect (src_expr)) { - //src_expr = expr_file_line (address_expr (src_expr, 0), e); src_expr = address_expr (src_expr, 0); need_ptr = true; } @@ -893,9 +894,8 @@ expr_assign_copy (sblock_t *sblock, const expr_t *e, operand_t **op, operand_t * // type from the statement expression in dags. Also, can't // use address_expr() because src_expr may be a function call // and unary_expr doesn't like that - src_expr = expr_file_line ( - new_address_expr (get_type (src_expr), src_expr, 0), - src_expr); + scoped_src_loc (src_expr); + src_expr = new_address_expr (get_type (src_expr), src_expr, 0); s = lea_statement (src, 0, src_expr); sblock_add_statement (sblock, s); src = s->opc; @@ -916,7 +916,6 @@ expr_assign_copy (sblock_t *sblock, const expr_t *e, operand_t **op, operand_t * //FIXME is this even necessary? if it is, should use copy_operand sblock = statement_subexpr (sblock, dst_expr, &kill); } - //dst_expr = expr_file_line (address_expr (dst_expr, 0), e); dst_expr = address_expr (dst_expr, 0); need_ptr = true; } @@ -928,10 +927,8 @@ expr_assign_copy (sblock_t *sblock, const expr_t *e, operand_t **op, operand_t * } count = min (type_size (dst_type), type_size (src_type)); if (count < (1 << 16)) { - //count_expr = expr_file_line (new_short_expr (count), e); count_expr = new_short_expr (count); } else { - //count_expr = expr_file_line (new_int_expr (count), e); count_expr = new_int_expr (count); } sblock = statement_subexpr (sblock, count_expr, &size); @@ -1028,7 +1025,7 @@ dereference_dst: // need to get a pointer type, entity.field expressions do not provide // one directly. FIXME it was probably a mistake extracting the operand // type from the statement expression in dags - //dst_expr = expr_file_line (address_expr (dst_expr, 0), dst_expr); + scoped_src_loc (dst_expr); dst_expr = address_expr (dst_expr, 0); s = new_statement (st_address, "lea", dst_expr); s->opa = dst; @@ -1174,6 +1171,7 @@ expr_call (sblock_t *sblock, const expr_t *call, operand_t **op) int num_params = 0; defspace_reset (arg_space); + scoped_src_loc (call); int num_args = list_count (&call->branch.args->list); const expr_t *args[num_args]; @@ -1194,7 +1192,7 @@ expr_call (sblock_t *sblock, const expr_t *call, operand_t **op) alignment); def->type = arg_type; def->reg = current_func->temp_reg; - const expr_t *def_expr = expr_file_line (new_def_expr (def), call); + const expr_t *def_expr = new_def_expr (def); if (a->type == ex_args) { args_va_list = def_expr; } else { @@ -1205,7 +1203,6 @@ expr_call (sblock_t *sblock, const expr_t *call, operand_t **op) num_params++; } const expr_t *assign = assign_expr (def_expr, a); - //expr_file_line (assign, call); sblock = statement_single (sblock, assign); } @@ -1228,12 +1225,9 @@ expr_call (sblock_t *sblock, const expr_t *call, operand_t **op) new_name_expr ("count")); const expr_t *args_list = field_expr (args_va_list, new_name_expr ("list")); - //expr_file_line (args_count, call); - //expr_file_line (args_list, call); count = new_short_expr (num_params); assign = assign_expr (args_count, count); - //expr_file_line (assign, call); sblock = statement_single (sblock, assign); if (args_params) { @@ -1241,9 +1235,7 @@ expr_call (sblock_t *sblock, const expr_t *call, operand_t **op) } else { list = new_nil_expr (); } - //expr_file_line (list, call); assign = assign_expr (args_list, list); - //expr_file_line (assign, call); sblock = statement_single (sblock, assign); } statement_t *s = new_statement (st_func, "call", call); @@ -1343,6 +1335,7 @@ ptr_addressing_mode (sblock_t *sblock, const expr_t *ref, if (!is_ptr (type)) { internal_error (ref, "expected pointer in ref"); } + scoped_src_loc (ref); if (ref->type == ex_address && (!ref->address.offset || is_constant (ref->address.offset)) && ref->address.lvalue->type == ex_alias @@ -1373,7 +1366,6 @@ ptr_addressing_mode (sblock_t *sblock, const expr_t *ref, } else { alias = new_alias_expr (type, lvalue->alias.expr); } - //expr_file_line (alias, ref); return addressing_mode (sblock, alias, base, offset, mode, target); } else if (ref->type != ex_alias || ref->alias.offset) { // probably just a pointer @@ -1468,6 +1460,7 @@ statement_return (sblock_t *sblock, const expr_t *e) const char *opcode; statement_t *s; + scoped_src_loc (e); debug (e, "RETURN"); opcode = "return"; if (!e->retrn.ret_val) { @@ -1517,7 +1510,6 @@ statement_return (sblock_t *sblock, const expr_t *e) def_t *ret_ptr = new_def (0, 0, 0, sc_local); operand_t *ret_op = def_operand (ret_ptr, &type_void, e); ret_ptr->reg = REG; - //expr_file_line (with, e); sblock = statement_single (sblock, with); sblock = statement_subexpr (sblock, call, &ret_op); } diff --git a/tools/qfcc/test/ptrstructinit.r b/tools/qfcc/test/ptrstructinit.r index d4fca5e11..8600c5f64 100644 --- a/tools/qfcc/test/ptrstructinit.r +++ b/tools/qfcc/test/ptrstructinit.r @@ -86,6 +86,7 @@ copy_joints (armature_t *arm, iqmjoint_t *joints, int num_joints) } } +#pragma code optimize void set_joint_points (armature_t *arm, int joint, float best_dist) { @@ -98,6 +99,7 @@ set_joint_points (armature_t *arm, int joint, float best_dist) arm.points[joint * JOINT_POINTS + j] = (vec4) p; } } +#pragma code optimize float find_best_dist (armature_t *arm, int joint) @@ -120,6 +122,30 @@ find_best_dist (armature_t *arm, int joint) return best_dist / 4; } +#pragma code optimize +void +set_poses (armature_t *arm, iqmjoint_t *joints, int num_joints) +{ + for (int i = 0; i < num_joints; i++) { + if (joints[i].parent >= 0) { + auto p = arm.pose[joints[i].parent]; + arm.pose[i] = { + .translate = p.translate + [(quaternion)p.rotate * joints[i].translate, 0], + .rotate = (quaternion)p.rotate * joints[i].rotate, + .scale = p.scale * [joints[i].scale, 0], + }; + } else { + arm.pose[i] = { + .translate = [joints[i].translate, 0], + .rotate = joints[i].rotate, + .scale = [joints[i].scale, 0], + }; + } + arm.basepose[i] = arm.pose[i]; + } +} +#pragma code optimize + armature_t * make_armature (int num_joints, iqmjoint_t *joints) { @@ -147,29 +173,7 @@ make_armature (int num_joints, iqmjoint_t *joints) return arm; } copy_joints (arm, joints, num_joints); - for (int i = 0; i < num_joints; i++) { - if (joints[i].parent >= 0) { - //auto j = joints[joints[i].parent]; - //auto p = &arm.pose[i]; - //p.translate = [j.translate + j.rotate*joints[i].translate, 0]; - //p.rotate = j.rotate * joints[i].rotate; - //p.scale = [j.scale * joints[i].scale, 0]; - auto p = joints[joints[i].parent]; - printf ("p:%d %q %q %q\n", i, p.translate, p.rotate, p.scale); - arm.pose[i] = { - .translate = [p.translate + p.rotate * joints[i].translate, 0], - .rotate = p.rotate * joints[i].rotate, - .scale = [p.scale * joints[i].scale, 0], - }; - } else { - arm.pose[i] = { - .translate = [joints[i].translate, 0], - .rotate = joints[i].rotate, - .scale = [joints[i].scale, 0], - }; - } - arm.basepose[i] = arm.pose[i]; - } + set_poses (arm, joints, num_joints); for (int i = 0; i < num_joints; i++) { for (int j = 0; j < JOINT_EDGES; j++) { @@ -264,7 +268,7 @@ main () auto e = joint_data[i]; auto a = arm.joints[i]; if (a.translate != e.translate || a.name != e.name - || (vec4)a.rotate != (vec4)e.rotate || a.scale != e.scale + || a.rotate != e.rotate || a.scale != e.scale || a.parent != e.parent) { printf ("%d %v %s %q %v %d\n", i, arm.joints[i].translate, arm.joints[i].name, arm.joints[i].rotate, @@ -303,9 +307,9 @@ main () } float best_dist = find_best_dist (arm, i); vec4 scale = { best_dist, best_dist, best_dist, 1 }; - //printf ("scale: %g\n", best_dist); - //printf ("%d %q %q %q\n", i, arm.basepose[i].translate, - // arm.basepose[i].rotate, arm.basepose[i].scale); + printf ("scale: %g\n", best_dist); + printf ("%d %q %q %q\n", i, arm.basepose[i].translate, + arm.basepose[i].rotate, arm.basepose[i].scale); auto M = make_motor (arm.basepose[i].translate, arm.basepose[i].rotate); for (int j = 0; j < JOINT_POINTS; j++) { auto p = (point_t) (points[j] * scale);