From b3eb05e37f449ccc42391b1ecb92ef64ff760a6e Mon Sep 17 00:00:00 2001 From: water111 <48171810+water111@users.noreply.github.com> Date: Fri, 14 May 2021 14:33:08 -0400 Subject: [PATCH] [decompiler] fix `(gpr->fpr` when an integer arg is converted to float (#482) * fix gpr fpr bug * remove unused variable --- decompiler/IR2/Env.cpp | 6 ++++ decompiler/IR2/Env.h | 6 +--- decompiler/IR2/FormExpressionAnalysis.cpp | 11 ++++-- decompiler/analysis/variable_naming.cpp | 21 ++++++++++-- decompiler/analysis/variable_naming.h | 1 + scripts/update_decomp_reference.py | 34 +++++++++++++++++++ .../reference/all_forward_declarations.gc | 3 -- .../reference/engine/gfx/font-h_REF.gc | 16 +++------ .../reference/engine/gfx/hw/gs_REF.gc | 8 +---- .../reference/engine/math/math_REF.gc | 5 +-- .../reference/engine/ps2/pad_REF.gc | 7 +--- test/decompiler/test_FormExpressionBuild2.cpp | 19 +++++++++++ 12 files changed, 95 insertions(+), 42 deletions(-) create mode 100644 scripts/update_decomp_reference.py diff --git a/decompiler/IR2/Env.cpp b/decompiler/IR2/Env.cpp index 0443ad6800..9c0b202d74 100644 --- a/decompiler/IR2/Env.cpp +++ b/decompiler/IR2/Env.cpp @@ -489,6 +489,12 @@ void Env::disable_def(const RegisterAccess& access, DecompWarnings& warnings) { } } +void Env::disable_use(const RegisterAccess& access) { + if (has_local_vars()) { + m_var_names.disable_use(access); + } +} + /*! * Set the stack hints. This must be done before type analysis. * This actually parses the types, so it should be done after the dts is set up. diff --git a/decompiler/IR2/Env.h b/decompiler/IR2/Env.h index bacb7d0e92..34b792e1e1 100644 --- a/decompiler/IR2/Env.h +++ b/decompiler/IR2/Env.h @@ -162,11 +162,7 @@ class Env { const std::vector& stack_var_hints() const { return m_stack_vars; } const UseDefInfo& get_use_def_info(const RegisterAccess& ra) const; - void disable_use(const RegisterAccess& access) { - if (has_local_vars()) { - m_var_names.disable_use(access); - } - } + void disable_use(const RegisterAccess& access); void disable_def(const RegisterAccess& access, DecompWarnings& warnings); diff --git a/decompiler/IR2/FormExpressionAnalysis.cpp b/decompiler/IR2/FormExpressionAnalysis.cpp index 337ea05bea..cb2f577a59 100644 --- a/decompiler/IR2/FormExpressionAnalysis.cpp +++ b/decompiler/IR2/FormExpressionAnalysis.cpp @@ -143,9 +143,14 @@ void pop_helper(const std::vector& vars, submit_reg_to_var.push_back(var_idx); submit_regs.push_back(var.reg()); } else { - // fmt::print("Unsafe to pop {}: used {} times, def {} times, expected use {}\n", - // var.to_string(env), use_def.use_count(), use_def.def_count(), - // times); + /*auto var_id = env.get_program_var_id(var); + fmt::print( + "Unsafe to pop {}: used {} times, def {} times, expected use {} ({} {} rd: {}) ({} + {})\n", var.to_string(env), use_def.use_count(), use_def.def_count(), times, + var.reg().to_string(), var.idx(), var.mode() == AccessMode::READ, + var_id.reg.to_string(), var_id.id); + */ + // if (var.to_string(env) == "a3-0") { // for (auto& use : use_def.uses) { // if (!use.disabled) { diff --git a/decompiler/analysis/variable_naming.cpp b/decompiler/analysis/variable_naming.cpp index 8c40cff181..225c157c65 100644 --- a/decompiler/analysis/variable_naming.cpp +++ b/decompiler/analysis/variable_naming.cpp @@ -252,6 +252,11 @@ bool is_arg_reg(Register r) { } } +int arg_reg_idx(Register r) { + assert(is_arg_reg(r)); + return (int)r.get_gpr() - (int)Reg::A0; +} + bool is_saved_reg(Register r) { if (r.get_kind() == Reg::GPR) { if (r.get_gpr() == Reg::GP) { @@ -349,8 +354,19 @@ SSA make_rc_ssa(const Function& function, const RegUsageInfo& rui, const Functio if (is_possible_coloring_move(dst, src) && rui.op.at(op_id).consumes.find(src) != rui.op.at(op_id).consumes.end()) { - ssa_i.is_arg_coloring_move = true; - got_not_arg_coloring = false; + // an integer argument going into a fpr for int->float conversion shouldn't + // be recognized as a coloring move. + if (function.type.arg_count() > 0) { + auto arg_idx = arg_reg_idx(src); + if (dst.get_kind() != Reg::FPR || + function.type.get_arg(arg_idx) == TypeSpec("float")) { + ssa_i.is_arg_coloring_move = true; + if (dst.get_kind() == Reg::FPR) { + ssa_i.is_gpr_fpr_coloring_move = true; + } + got_not_arg_coloring = false; + } + } } } } @@ -758,6 +774,7 @@ std::unordered_map SSA::get_use_def_info( if (instr.is_dead_set) { continue; } + if (instr.dst.has_value()) { // get the SSA var: auto ssa_var_id = diff --git a/decompiler/analysis/variable_naming.h b/decompiler/analysis/variable_naming.h index c9f24af3d4..a31fe15d95 100644 --- a/decompiler/analysis/variable_naming.h +++ b/decompiler/analysis/variable_naming.h @@ -111,6 +111,7 @@ struct SSA { std::vector src; int op_id = -1; bool is_arg_coloring_move = false; + bool is_gpr_fpr_coloring_move = false; bool is_dead_set = false; std::string print(const VarMapSSA& var_map) const; diff --git a/scripts/update_decomp_reference.py b/scripts/update_decomp_reference.py new file mode 100644 index 0000000000..dc6f6f7c6c --- /dev/null +++ b/scripts/update_decomp_reference.py @@ -0,0 +1,34 @@ +import os +import glob +import argparse +import shutil + +## Script to update failing _REF.gc files +## Instructions: +## run offline-test with the `--dump-mode` flag set. This generates a "failures" folder. +## update reference like this +## python3 ../scripts/update_decomp_reference.py ./failures ../test/decompiler/reference + +def get_goal_files(root_dir): + return [f for file in os.walk(root_dir) for f in glob.glob(os.path.join(file[0], '*.gc'))] + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument(dest='diff', help='the failures folder') + parser.add_argument(dest='reference', help='the test/decompiler/reference folder') + args = parser.parse_args() + + location_map = {os.path.basename(x) : x for x in get_goal_files(args.reference)} + + for replacement in get_goal_files(args.diff): + base = os.path.basename(replacement) + if base not in location_map: + print("Could not find file {}".format(base)) + exit(-1) + print("replace {} with {}".format(location_map[base], replacement)) + shutil.copyfile(replacement, location_map[base]) + + + +if __name__ == "__main__": + main() diff --git a/test/decompiler/reference/all_forward_declarations.gc b/test/decompiler/reference/all_forward_declarations.gc index 58f2e1d866..14bcf52f67 100644 --- a/test/decompiler/reference/all_forward_declarations.gc +++ b/test/decompiler/reference/all_forward_declarations.gc @@ -97,9 +97,6 @@ '(none) ) -(defmacro gpr->fpr (in) - in) - (define-extern get-current-time (function uint)) (define-extern get-integral-current-time (function uint)) diff --git a/test/decompiler/reference/engine/gfx/font-h_REF.gc b/test/decompiler/reference/engine/gfx/font-h_REF.gc index 8e6d89cc8b..fe98377e90 100644 --- a/test/decompiler/reference/engine/gfx/font-h_REF.gc +++ b/test/decompiler/reference/engine/gfx/font-h_REF.gc @@ -119,18 +119,14 @@ ;; definition for method 10 of type font-context (defmethod set-origin! font-context ((obj font-context) (x int) (y int)) - (let ((x (gpr->fpr x))) - (set! (-> obj origin x) (the float x)) - ) + (set! (-> obj origin x) (the float x)) (set! (-> obj origin y) (the float y)) obj ) ;; definition for method 11 of type font-context (defmethod set-depth! font-context ((obj font-context) (z int)) - (let ((z (gpr->fpr z))) - (set! (-> obj origin z) (the float z)) - ) + (set! (-> obj origin z) (the float z)) obj ) @@ -142,17 +138,13 @@ ;; definition for method 13 of type font-context (defmethod set-width! font-context ((obj font-context) (width int)) - (let ((width (gpr->fpr width))) - (set! (-> obj width) (the float width)) - ) + (set! (-> obj width) (the float width)) obj ) ;; definition for method 14 of type font-context (defmethod set-height! font-context ((obj font-context) (height int)) - (let ((height (gpr->fpr height))) - (set! (-> obj height) (the float height)) - ) + (set! (-> obj height) (the float height)) obj ) diff --git a/test/decompiler/reference/engine/gfx/hw/gs_REF.gc b/test/decompiler/reference/engine/gfx/hw/gs_REF.gc index 098c7c9edf..07b573c84e 100644 --- a/test/decompiler/reference/engine/gfx/hw/gs_REF.gc +++ b/test/decompiler/reference/engine/gfx/hw/gs_REF.gc @@ -1159,15 +1159,9 @@ ;; definition for function draw-context-set-xy ;; INFO: Return type mismatch int vs none. (defun draw-context-set-xy ((ctxt draw-context) (x int) (y int)) - (let* ((y (gpr->fpr y)) - (v0-0 (the int (* (the float y) (-> *video-parms* relative-y-scale)))) - ) + (let ((v0-0 (the int (* (the float y) (-> *video-parms* relative-y-scale))))) (set! (-> ctxt orgx) x) (set! (-> ctxt orgy) v0-0) ) (none) ) - - - - diff --git a/test/decompiler/reference/engine/math/math_REF.gc b/test/decompiler/reference/engine/math/math_REF.gc index 9e82580cd1..9bcbadb7e6 100644 --- a/test/decompiler/reference/engine/math/math_REF.gc +++ b/test/decompiler/reference/engine/math/math_REF.gc @@ -46,9 +46,7 @@ ;; definition for function log2 (defun log2 ((arg0 int)) - (let ((arg0 (gpr->fpr arg0))) - (+ (sar (the-as int (the float arg0)) 23) -127) - ) + (+ (sar (the-as int (the float arg0)) 23) -127) ) ;; definition for function seek @@ -226,4 +224,3 @@ ;; failed to figure out what this is: (let ((v0-6 0)) ) - diff --git a/test/decompiler/reference/engine/ps2/pad_REF.gc b/test/decompiler/reference/engine/ps2/pad_REF.gc index 1f5f43d816..505ba9a645 100644 --- a/test/decompiler/reference/engine/ps2/pad_REF.gc +++ b/test/decompiler/reference/engine/ps2/pad_REF.gc @@ -180,8 +180,7 @@ (defun analog-input ((in int) (offset float) (center-val float) (max-val float) (out-range float)) - (let* ((in (gpr->fpr in)) - (offset-in (- (the float in) offset)) + (let* ((offset-in (- (the float in) offset)) (magnitude (- (fabs offset-in) center-val)) (max-magnitude (- max-val center-val)) ) @@ -367,7 +366,3 @@ ;; failed to figure out what this is: (let ((v0-4 0)) ) - - - - diff --git a/test/decompiler/test_FormExpressionBuild2.cpp b/test/decompiler/test_FormExpressionBuild2.cpp index 6c973dc7ee..078c42f67b 100644 --- a/test/decompiler/test_FormExpressionBuild2.cpp +++ b/test/decompiler/test_FormExpressionBuild2.cpp @@ -1135,3 +1135,22 @@ TEST_F(FormRegressionTest, StupidFloatMove) { " )"; test_with_expr(func, type, expected); } + +// gpr->fpr not being propagated +TEST_F(FormRegressionTest, Method11FontContext) { + std::string func = + "sll r0, r0, 0\n" + " mtc1 f0, a1\n" + " cvt.s.w f0, f0\n" + " swc1 f0, 20(a0)\n" + " or v0, a0, r0\n" + " jr ra\n" + " daddu sp, sp, r0"; + std::string type = "(function font-context int font-context)"; + std::string expected = + "(begin\n" + " (set! (-> arg0 origin z) (the float arg1))\n" + " arg0\n" + " )"; + test_with_expr(func, type, expected); +} \ No newline at end of file