From 8a805123d4890d0a4fe7aa5413480146dd315102 Mon Sep 17 00:00:00 2001 From: angie Date: Sun, 16 Apr 2023 10:05:01 -0400 Subject: [PATCH 1/7] Implement workaround for R5900's trunc.w.s issue --- .../instructions/UniqueId_enum_class.table.h | 1 - include/common/RabbitizerConfig.h | 9 +++++- include/instructions/InstrId_enum.table.h | 1 - .../instr_id/r5900/r5900_cop1_fpu_s.inc | 13 -------- rabbitizer/Config.pyi | 3 +- rabbitizer/rabbitizer_global_config.c | 2 ++ rust/src/config.rs | 1 + src/common/RabbitizerConfig.c | 1 + .../InstrDescriptor_Descriptors_array.table.h | 1 - src/instructions/InstrId_Names_array.table.h | 1 - ...bbitizerInstructionR5900_ProcessUniqueId.c | 31 +++++++++++++++++++ tests/asm/r5900/trunc_w_c.s | 15 +++++++++ 12 files changed, 60 insertions(+), 19 deletions(-) create mode 100644 tests/asm/r5900/trunc_w_c.s diff --git a/cplusplus/include/instructions/UniqueId_enum_class.table.h b/cplusplus/include/instructions/UniqueId_enum_class.table.h index f5d2379a..8ac59bef 100644 --- a/cplusplus/include/instructions/UniqueId_enum_class.table.h +++ b/cplusplus/include/instructions/UniqueId_enum_class.table.h @@ -537,7 +537,6 @@ r5900_madda_s, r5900_msuba_s, r5900_max_s, r5900_min_s, -r5900_trunc_w_s, r5900_c_lt_s, r5900_c_le_s, r5900_qmfc2, diff --git a/include/common/RabbitizerConfig.h b/include/common/RabbitizerConfig.h index ed4caa01..6eafa860 100644 --- a/include/common/RabbitizerConfig.h +++ b/include/common/RabbitizerConfig.h @@ -48,7 +48,14 @@ typedef struct RabbitizerConfig_ToolchainTweaks { * However, the break it generates is different than the one it generates with `break N` * So we replace break instrutions for SN64 with the exact word that the assembler generates when expanding div */ - bool sn64DivFix; + bool sn64DivFix; //! @deprecated + /** + * Enables various tweaks to allow building matching with GNU as which + * break original compiler behavior and what's specified in the manuals. + * + * Turning this option off implies turning sn64DivFix on. + */ + bool gnuMode; } RabbitizerConfig_ToolchainTweaks; typedef struct RabbitizerConfig_Misc { diff --git a/include/instructions/InstrId_enum.table.h b/include/instructions/InstrId_enum.table.h index ac74617d..78baa7b7 100644 --- a/include/instructions/InstrId_enum.table.h +++ b/include/instructions/InstrId_enum.table.h @@ -537,7 +537,6 @@ RABBITIZER_INSTR_ID_r5900_madda_s, RABBITIZER_INSTR_ID_r5900_msuba_s, RABBITIZER_INSTR_ID_r5900_max_s, RABBITIZER_INSTR_ID_r5900_min_s, -RABBITIZER_INSTR_ID_r5900_trunc_w_s, RABBITIZER_INSTR_ID_r5900_c_lt_s, RABBITIZER_INSTR_ID_r5900_c_le_s, RABBITIZER_INSTR_ID_r5900_qmfc2, diff --git a/include/instructions/instr_id/r5900/r5900_cop1_fpu_s.inc b/include/instructions/instr_id/r5900/r5900_cop1_fpu_s.inc index 2e65ae60..4589ed22 100644 --- a/include/instructions/instr_id/r5900/r5900_cop1_fpu_s.inc +++ b/include/instructions/instr_id/r5900/r5900_cop1_fpu_s.inc @@ -103,19 +103,6 @@ RABBITIZER_DEF_INSTR_ID_ALTNAME( .readsFt=true ) // floating point MINimum -// Due to the R5900's FPU being non properly complaint the instruction cvt.w.s always behaves as trunc.w.s because is because EE can only do round-to-zero. -// Assemblers like GAS workaround this issue by decoding cvt.w.s as trunc.w.s, so we mimic that behaviour to allow assembling with GAS. -// Here's some reading about the binutils rationale: -// https://sourceware.org/legacy-ml/binutils/2012-11/msg00360.html -// https://sourceware.org/pipermail/binutils/2013-January/079863.html -RABBITIZER_DEF_INSTR_ID_ALTNAME( - r5900, 0x24, trunc_w_s, trunc.w.s, - .operands={RAB_OPERAND_cpu_fd, RAB_OPERAND_cpu_fs}, - .isFloat=true, - .modifiesFd=true, - .readsFs=true -) // Floating-Point Truncate to Word Fixed-Point - RABBITIZER_DEF_INSTR_ID_ALTNAME( r5900, 0x34, c_lt_s, c.lt.s, .operands={RAB_OPERAND_cpu_fs, RAB_OPERAND_cpu_ft}, diff --git a/rabbitizer/Config.pyi b/rabbitizer/Config.pyi index c639b4f1..2e9b87a8 100644 --- a/rabbitizer/Config.pyi +++ b/rabbitizer/Config.pyi @@ -35,8 +35,9 @@ class _RabbitizerConfig: pseudos_pseudoNegu: bool = True pseudos_pseudoBal: bool = True - toolchainTweaks_sn64DivFix: bool = False toolchainTweaks_treatJAsUnconditionalBranch: bool = False + toolchainTweaks_sn64DivFix: bool = False + toolchainTweaks_gnuMode: bool = True misc_opcodeLJust: int = 11 misc_unknownInstrComment: bool = True diff --git a/rabbitizer/rabbitizer_global_config.c b/rabbitizer/rabbitizer_global_config.c index d02fce37..9f559774 100644 --- a/rabbitizer/rabbitizer_global_config.c +++ b/rabbitizer/rabbitizer_global_config.c @@ -118,6 +118,7 @@ DEF_MEMBER_GET_SET_BOOL(pseudos, pseudoBal) DEF_MEMBER_GET_SET_BOOL(toolchainTweaks, treatJAsUnconditionalBranch) DEF_MEMBER_GET_SET_BOOL(toolchainTweaks, sn64DivFix) +DEF_MEMBER_GET_SET_BOOL(toolchainTweaks, gnuMode) DEF_MEMBER_GET_SET_INT(misc, opcodeLJust, false, 0, 0) DEF_MEMBER_GET_SET_BOOL(misc, unknownInstrComment) @@ -144,6 +145,7 @@ static PyGetSetDef rabbitizer_global_config_GetSets[] = { MEMBER_GET_SET(toolchainTweaks, treatJAsUnconditionalBranch, "", NULL), MEMBER_GET_SET(toolchainTweaks, sn64DivFix, "", NULL), + MEMBER_GET_SET(toolchainTweaks, gnuMode, "", NULL), MEMBER_GET_SET(misc, opcodeLJust, "", NULL), MEMBER_GET_SET(misc, unknownInstrComment, "", NULL), diff --git a/rust/src/config.rs b/rust/src/config.rs index bab549e8..6614d4ba 100644 --- a/rust/src/config.rs +++ b/rust/src/config.rs @@ -35,6 +35,7 @@ pub struct PseudoInstr { pub struct ToolchainTweaks { pub treat_j_as_unconditional_branch: bool, pub sn64_div_fix: bool, + pub gnu_mode: bool, } #[repr(C)] diff --git a/src/common/RabbitizerConfig.c b/src/common/RabbitizerConfig.c index bddaf0f4..e09c48fa 100644 --- a/src/common/RabbitizerConfig.c +++ b/src/common/RabbitizerConfig.c @@ -40,6 +40,7 @@ RabbitizerConfig RabbitizerConfig_Cfg = { .toolchainTweaks = { .treatJAsUnconditionalBranch = true, .sn64DivFix = false, + .gnuMode = true, }, .misc = { .opcodeLJust = 7+4, diff --git a/src/instructions/InstrDescriptor_Descriptors_array.table.h b/src/instructions/InstrDescriptor_Descriptors_array.table.h index d4d8ee0f..0d8a6bad 100644 --- a/src/instructions/InstrDescriptor_Descriptors_array.table.h +++ b/src/instructions/InstrDescriptor_Descriptors_array.table.h @@ -537,7 +537,6 @@ const RabbitizerInstrDescriptor RabbitizerInstrDescriptor_Descriptors[] = { [RABBITIZER_INSTR_ID_r5900_msuba_s] = { .operands={RAB_OPERAND_cpu_fs, RAB_OPERAND_cpu_ft}, .isFloat=true, .readsFs=true, .readsFt=true }, [RABBITIZER_INSTR_ID_r5900_max_s] = { .operands={RAB_OPERAND_cpu_fd, RAB_OPERAND_cpu_fs, RAB_OPERAND_cpu_ft}, .isFloat=true, .modifiesFd=true, .readsFs=true, .readsFt=true }, [RABBITIZER_INSTR_ID_r5900_min_s] = { .operands={RAB_OPERAND_cpu_fd, RAB_OPERAND_cpu_fs, RAB_OPERAND_cpu_ft}, .isFloat=true, .modifiesFd=true, .readsFs=true, .readsFt=true }, -[RABBITIZER_INSTR_ID_r5900_trunc_w_s] = { .operands={RAB_OPERAND_cpu_fd, RAB_OPERAND_cpu_fs}, .isFloat=true, .modifiesFd=true, .readsFs=true }, [RABBITIZER_INSTR_ID_r5900_c_lt_s] = { .operands={RAB_OPERAND_cpu_fs, RAB_OPERAND_cpu_ft}, .isFloat=true, .readsFs=true, .readsFt=true }, [RABBITIZER_INSTR_ID_r5900_c_le_s] = { .operands={RAB_OPERAND_cpu_fs, RAB_OPERAND_cpu_ft}, .isFloat=true, .readsFs=true, .readsFt=true }, [RABBITIZER_INSTR_ID_r5900_qmfc2] = { .operands={RAB_OPERAND_cpu_rt, RAB_OPERAND_r5900_vfs}, .modifiesRt=true }, diff --git a/src/instructions/InstrId_Names_array.table.h b/src/instructions/InstrId_Names_array.table.h index 4ec29201..7300db81 100644 --- a/src/instructions/InstrId_Names_array.table.h +++ b/src/instructions/InstrId_Names_array.table.h @@ -537,7 +537,6 @@ const char *RabbitizerInstrId_Names[] = { [RABBITIZER_INSTR_ID_r5900_msuba_s] = "msuba.s", [RABBITIZER_INSTR_ID_r5900_max_s] = "max.s", [RABBITIZER_INSTR_ID_r5900_min_s] = "min.s", -[RABBITIZER_INSTR_ID_r5900_trunc_w_s] = "trunc.w.s", [RABBITIZER_INSTR_ID_r5900_c_lt_s] = "c.lt.s", [RABBITIZER_INSTR_ID_r5900_c_le_s] = "c.le.s", [RABBITIZER_INSTR_ID_r5900_qmfc2] = "qmfc2", diff --git a/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c b/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c index 4f91780d..998a5447 100644 --- a/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c +++ b/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c @@ -2,6 +2,7 @@ /* SPDX-License-Identifier: MIT */ #include "instructions/RabbitizerInstructionR5900.h" +#include "common/RabbitizerConfig.h" #define RABBITIZER_DEF_INSTR_ID(prefix, caseBits, name, ...) \ case (caseBits): \ @@ -132,6 +133,36 @@ void RabbitizerInstructionR5900_processUniqueId_Coprocessor1_FpuS(RabbitizerInst RabbitizerInstruction_processUniqueId_Coprocessor1_FpuS(self); break; } + + if (RabbitizerConfig_Cfg.toolchainTweaks.gnuMode) { + switch (self->uniqueId) { + case RABBITIZER_INSTR_ID_cpu_trunc_w_s: + case RABBITIZER_INSTR_ID_cpu_cvt_w_s: + /** + * Due to the R5900's FPU being non properly complaint, the instruction cvt.w.s always behaves as + * trunc.w.s because EE can only do round-to-zero. + * + * Assemblers like GAS workaround this issue by decoding cvt.w.s as trunc.w.s, but other assemblers just + * use trunc.w.s and cvt.w.s as-is. + * + * Here's some reading about the binutils rationale: + * - https://sourceware.org/legacy-ml/binutils/2012-11/msg00360.html + * - https://sourceware.org/pipermail/binutils/2013-January/079863.html + * + * Because of this, building with GAS with the -march=r5900 flag produces: + * - trunc.w.s is built as the cvt.w.s instruction. + * - cvt.w.s errors complaining as not being supported by the processor. + * + * To ensure the produced disassembly will still match when built with GAS, we decode this two + * instructions as .word + */ + self->_mandatorybits = 0x0; + break; + + default: + break; + } + } } void RabbitizerInstructionR5900_processUniqueId_Coprocessor1(RabbitizerInstruction *self) { diff --git a/tests/asm/r5900/trunc_w_c.s b/tests/asm/r5900/trunc_w_c.s new file mode 100644 index 00000000..527cc4a0 --- /dev/null +++ b/tests/asm/r5900/trunc_w_c.s @@ -0,0 +1,15 @@ +.set noreorder + +.section .text + +.global test +trunc.w.s $f0, $f12 +jr $31 +nop + +.global test2 +cvt.w.s $f0, $f12 +jr $31 +nop + + From bb23e65556fa15dc3734415ae02d184b18133dd6 Mon Sep 17 00:00:00 2001 From: angie Date: Sun, 16 Apr 2023 10:24:08 -0400 Subject: [PATCH 2/7] Add test for trunc on r5900 --- tests/c/instruction_checks/r5900_trunc_cvt.c | 58 ++++++++++++++++++++ tests/run_instruction_checks.sh | 3 +- 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tests/c/instruction_checks/r5900_trunc_cvt.c diff --git a/tests/c/instruction_checks/r5900_trunc_cvt.c b/tests/c/instruction_checks/r5900_trunc_cvt.c new file mode 100644 index 00000000..d2eed6e6 --- /dev/null +++ b/tests/c/instruction_checks/r5900_trunc_cvt.c @@ -0,0 +1,58 @@ +/* SPDX-FileCopyrightText: © 2023 Decompollaborate */ +/* SPDX-License-Identifier: MIT */ + +#include "rabbitizer.h" + +#include +#include +#include + + +typedef struct TestEntry { + uint32_t word; + bool gnuMode; + const char *expectedStr; +} TestEntry; + + +const TestEntry entries[] = { + { 0x4600600D, true, ".word 0x4600600D # trunc.w.s $f0, $f12 # 4600000D" }, + { 0x46006024, true, ".word 0x46006024 # cvt.w.s $f0, $f12 # 46000024" }, + { 0x4600600D, false, "trunc.w.s $f0, $f12" }, + { 0x46006024, false, "cvt.w.s $f0, $f12" }, +}; + +int main() { + int errorCount = 0; + size_t i; + + for (i = 0; i < ARRAY_COUNT(entries); i++) { + const TestEntry *entry = &entries[i]; + RabbitizerConfig_Cfg.toolchainTweaks.gnuMode = entry->gnuMode; + RabbitizerInstruction instr; + char *buffer; + size_t bufferSize; + + RabbitizerInstructionR5900_init(&instr, entry->word, 0x80000000); + RabbitizerInstructionR5900_processUniqueId(&instr); + + bufferSize = RabbitizerInstruction_getSizeForBuffer(&instr, 0, 0); + buffer = malloc(bufferSize + 1); + assert(buffer != NULL); + + RabbitizerInstruction_disassemble(&instr, buffer, NULL, 0, 0); + + if (entry->expectedStr == NULL) { + printf("Word '0x%08X' gnuMode '%s' doesn't have a expected str, got '%s'\n", entry->word, entry->gnuMode ? "true" : "false", buffer); + errorCount++; + } else if (strcmp(buffer, entry->expectedStr) != 0) { + fprintf(stderr, "Error on word '0x%08X' gnuMode '%s'. Expected '%s', got '%s'\n", entry->word, entry->gnuMode ? "true" : "false", entry->expectedStr, buffer); + errorCount++; + } + + free(buffer); + RabbitizerInstructionR5900_destroy(&instr); + } + + return errorCount; +} diff --git a/tests/run_instruction_checks.sh b/tests/run_instruction_checks.sh index 8ab0c006..a3e55f57 100755 --- a/tests/run_instruction_checks.sh +++ b/tests/run_instruction_checks.sh @@ -1,9 +1,10 @@ #!/bin/bash -# SPDX-FileCopyrightText: © 2022 Decompollaborate +# SPDX-FileCopyrightText: © 2022-2023 Decompollaborate # SPDX-License-Identifier: MIT set -e ./build/tests/c/instruction_checks/jalr.elf ./build/tests/c/instruction_checks/plain_disassembly.elf +./build/tests/c/instruction_checks/r5900_trunc_cvt.elf From ef43c1eb775f35e00a0abc0f62260afc8591edaa Mon Sep 17 00:00:00 2001 From: angie Date: Sun, 16 Apr 2023 10:35:47 -0400 Subject: [PATCH 3/7] cleanup implementation of gnuMode --- include/common/RabbitizerConfig.h | 4 +-- .../RabbitizerInstruction_Disassemble.c | 30 +++++++++++++++++++ .../RabbitizerInstruction_ProcessUniqueId.c | 6 ++-- ...bbitizerInstructionR5900_ProcessUniqueId.c | 30 ------------------- tests/c/instruction_checks/r5900_trunc_cvt.c | 4 +-- 5 files changed, 37 insertions(+), 37 deletions(-) diff --git a/include/common/RabbitizerConfig.h b/include/common/RabbitizerConfig.h index 6eafa860..404216a2 100644 --- a/include/common/RabbitizerConfig.h +++ b/include/common/RabbitizerConfig.h @@ -48,12 +48,10 @@ typedef struct RabbitizerConfig_ToolchainTweaks { * However, the break it generates is different than the one it generates with `break N` * So we replace break instrutions for SN64 with the exact word that the assembler generates when expanding div */ - bool sn64DivFix; //! @deprecated + bool sn64DivFix; /** * Enables various tweaks to allow building matching with GNU as which * break original compiler behavior and what's specified in the manuals. - * - * Turning this option off implies turning sn64DivFix on. */ bool gnuMode; } RabbitizerConfig_ToolchainTweaks; diff --git a/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c b/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c index fb21cfbb..6075a7af 100644 --- a/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c +++ b/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c @@ -95,6 +95,36 @@ bool RabbitizerInstruction_mustDisasmAsData(const RabbitizerInstruction *self) { } } + if (RabbitizerConfig_Cfg.toolchainTweaks.gnuMode) { + switch (self->uniqueId) { + case RABBITIZER_INSTR_ID_cpu_trunc_w_s: + case RABBITIZER_INSTR_ID_cpu_cvt_w_s: + /** + * Due to the R5900's FPU being non properly complaint, the instruction cvt.w.s always behaves as + * trunc.w.s because EE can only do round-to-zero. + * + * Assemblers like GAS workaround this issue by decoding cvt.w.s as trunc.w.s, but other assemblers just + * use trunc.w.s and cvt.w.s as-is. + * + * Here's some reading about the binutils rationale: + * - https://sourceware.org/legacy-ml/binutils/2012-11/msg00360.html + * - https://sourceware.org/pipermail/binutils/2013-January/079863.html + * + * Because of this, building with GAS with the -march=r5900 flag produces: + * - trunc.w.s is built as the cvt.w.s instruction. + * - cvt.w.s errors complaining as not being supported by the processor. + * + * To ensure the produced disassembly will still match when built with GAS, we decode this two + * instructions as .word + */ + return true; + break; + + default: + break; + } + } + if (!RabbitizerInstruction_isValid(self)) { return true; } diff --git a/src/instructions/RabbitizerInstruction/RabbitizerInstruction_ProcessUniqueId.c b/src/instructions/RabbitizerInstruction/RabbitizerInstruction_ProcessUniqueId.c index 46dccecd..e97c1257 100644 --- a/src/instructions/RabbitizerInstruction/RabbitizerInstruction_ProcessUniqueId.c +++ b/src/instructions/RabbitizerInstruction/RabbitizerInstruction_ProcessUniqueId.c @@ -101,13 +101,15 @@ void RabbitizerInstruction_processUniqueId_Special(RabbitizerInstruction *self) switch (self->uniqueId) { case RABBITIZER_INSTR_ID_cpu_div: - if (RabbitizerConfig_Cfg.toolchainTweaks.sn64DivFix && !self->inHandwrittenFunction) { + if ((!RabbitizerConfig_Cfg.toolchainTweaks.gnuMode) || + (RabbitizerConfig_Cfg.toolchainTweaks.sn64DivFix && !self->inHandwrittenFunction)) { self->descriptor = &RabbitizerInstrDescriptor_Descriptors[RABBITIZER_INSTR_ID_cpu_sn64_div]; } break; case RABBITIZER_INSTR_ID_cpu_divu: - if (RabbitizerConfig_Cfg.toolchainTweaks.sn64DivFix && !self->inHandwrittenFunction) { + if ((!RabbitizerConfig_Cfg.toolchainTweaks.gnuMode) || + (RabbitizerConfig_Cfg.toolchainTweaks.sn64DivFix && !self->inHandwrittenFunction)) { self->descriptor = &RabbitizerInstrDescriptor_Descriptors[RABBITIZER_INSTR_ID_cpu_sn64_divu]; } break; diff --git a/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c b/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c index 998a5447..a5f59bbe 100644 --- a/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c +++ b/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_ProcessUniqueId.c @@ -133,36 +133,6 @@ void RabbitizerInstructionR5900_processUniqueId_Coprocessor1_FpuS(RabbitizerInst RabbitizerInstruction_processUniqueId_Coprocessor1_FpuS(self); break; } - - if (RabbitizerConfig_Cfg.toolchainTweaks.gnuMode) { - switch (self->uniqueId) { - case RABBITIZER_INSTR_ID_cpu_trunc_w_s: - case RABBITIZER_INSTR_ID_cpu_cvt_w_s: - /** - * Due to the R5900's FPU being non properly complaint, the instruction cvt.w.s always behaves as - * trunc.w.s because EE can only do round-to-zero. - * - * Assemblers like GAS workaround this issue by decoding cvt.w.s as trunc.w.s, but other assemblers just - * use trunc.w.s and cvt.w.s as-is. - * - * Here's some reading about the binutils rationale: - * - https://sourceware.org/legacy-ml/binutils/2012-11/msg00360.html - * - https://sourceware.org/pipermail/binutils/2013-January/079863.html - * - * Because of this, building with GAS with the -march=r5900 flag produces: - * - trunc.w.s is built as the cvt.w.s instruction. - * - cvt.w.s errors complaining as not being supported by the processor. - * - * To ensure the produced disassembly will still match when built with GAS, we decode this two - * instructions as .word - */ - self->_mandatorybits = 0x0; - break; - - default: - break; - } - } } void RabbitizerInstructionR5900_processUniqueId_Coprocessor1(RabbitizerInstruction *self) { diff --git a/tests/c/instruction_checks/r5900_trunc_cvt.c b/tests/c/instruction_checks/r5900_trunc_cvt.c index d2eed6e6..28e37cd7 100644 --- a/tests/c/instruction_checks/r5900_trunc_cvt.c +++ b/tests/c/instruction_checks/r5900_trunc_cvt.c @@ -16,8 +16,8 @@ typedef struct TestEntry { const TestEntry entries[] = { - { 0x4600600D, true, ".word 0x4600600D # trunc.w.s $f0, $f12 # 4600000D" }, - { 0x46006024, true, ".word 0x46006024 # cvt.w.s $f0, $f12 # 46000024" }, + { 0x4600600D, true, ".word 0x4600600D # trunc.w.s $f0, $f12 # 00000000" }, + { 0x46006024, true, ".word 0x46006024 # cvt.w.s $f0, $f12 # 00000000" }, { 0x4600600D, false, "trunc.w.s $f0, $f12" }, { 0x46006024, false, "cvt.w.s $f0, $f12" }, }; From ab7ce5fa9d9379c649be4a187cfeb8cb6d0e8292 Mon Sep 17 00:00:00 2001 From: angie Date: Sun, 16 Apr 2023 10:48:19 -0400 Subject: [PATCH 4/7] Add `linguist-generated=true` to autogenerated files --- .gitattributes | 23 +++++++++++++++++++++++ rust/src/instr_id_enum.rs | 1 - 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/.gitattributes b/.gitattributes index dfe07704..b8df85c3 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,25 @@ # Auto detect text files and perform LF normalization * text=auto + +src/instructions/RabbitizerInstruction/instrOpercandCallbacks_array.table.h linguist-generated=true + +src/instructions/InstrCategory_Names_array.table.h linguist-generated=true +src/instructions/InstrDescriptor_Descriptors_array.table.h linguist-generated=true +src/instructions/InstrId_Names_array.table.h linguist-generated=true +src/instructions/RegisterDescriptor_Descriptors_arrays.table.h linguist-generated=true +src/instructions/Registers_Names_arrays.table.h linguist-generated=true + +include/common/Abi_enum.table.h linguist-generated=true + +include/instructions/AccessType_enum.table.h linguist-generated=true +include/instructions/InstrCategory_enum.table.h linguist-generated=true +include/instructions/InstrId_enum.table.h linguist-generated=true +include/instructions/InstrSuffix_enum.table.h linguist-generated=true +include/instructions/OperandType_enum.table.h linguist-generated=true +include/instructions/OperandType_function_declarations.table.h linguist-generated=true +include/instructions/Registers_enums.table.h linguist-generated=true + +cplusplus/include/instructions/AccessType_enum_class.table.h linguist-generated=true +cplusplus/include/instructions/OperandType_enum_class.table.h linguist-generated=true +cplusplus/include/instructions/Registers_enum_classes.table.h linguist-generated=true +cplusplus/include/instructions/UniqueId_enum_class.table.h linguist-generated=true diff --git a/rust/src/instr_id_enum.rs b/rust/src/instr_id_enum.rs index c6b83742..f2e01ebe 100644 --- a/rust/src/instr_id_enum.rs +++ b/rust/src/instr_id_enum.rs @@ -537,7 +537,6 @@ r5900_madda_s, r5900_msuba_s, r5900_max_s, r5900_min_s, -r5900_trunc_w_s, r5900_c_lt_s, r5900_c_le_s, r5900_qmfc2, From 29c09e2355733251e4af438d16421de501ed3756 Mon Sep 17 00:00:00 2001 From: angie Date: Sun, 16 Apr 2023 10:50:20 -0400 Subject: [PATCH 5/7] whoops --- .gitattributes | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.gitattributes b/.gitattributes index b8df85c3..4889dcdd 100644 --- a/.gitattributes +++ b/.gitattributes @@ -23,3 +23,11 @@ cplusplus/include/instructions/AccessType_enum_class.table.h linguist-generated= cplusplus/include/instructions/OperandType_enum_class.table.h linguist-generated=true cplusplus/include/instructions/Registers_enum_classes.table.h linguist-generated=true cplusplus/include/instructions/UniqueId_enum_class.table.h linguist-generated=true + +rust/src/abi_enum.rs linguist-generated=true +rust/src/access_type_enum.rs linguist-generated=true +rust/src/instr_category_enum.rs linguist-generated=true +rust/src/instr_id_enum.rs linguist-generated=true +rust/src/instr_suffix_enum.rs linguist-generated=true +rust/src/operand_type_enum.rs linguist-generated=true +rust/src/registers_enum.rs linguist-generated=true From 9705bc8e57702c89f4fafa968673b84e96d666ed Mon Sep 17 00:00:00 2001 From: Angie Date: Sun, 16 Apr 2023 17:03:17 -0400 Subject: [PATCH 6/7] I, Q, R and ACC --- .../RabbitizerInstruction_Disassemble.c | 40 ++++++++++--------- .../RabbitizerInstructionR5900_OperandType.c | 8 ++-- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c b/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c index 6075a7af..9fc1e1fb 100644 --- a/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c +++ b/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Disassemble.c @@ -99,25 +99,27 @@ bool RabbitizerInstruction_mustDisasmAsData(const RabbitizerInstruction *self) { switch (self->uniqueId) { case RABBITIZER_INSTR_ID_cpu_trunc_w_s: case RABBITIZER_INSTR_ID_cpu_cvt_w_s: - /** - * Due to the R5900's FPU being non properly complaint, the instruction cvt.w.s always behaves as - * trunc.w.s because EE can only do round-to-zero. - * - * Assemblers like GAS workaround this issue by decoding cvt.w.s as trunc.w.s, but other assemblers just - * use trunc.w.s and cvt.w.s as-is. - * - * Here's some reading about the binutils rationale: - * - https://sourceware.org/legacy-ml/binutils/2012-11/msg00360.html - * - https://sourceware.org/pipermail/binutils/2013-January/079863.html - * - * Because of this, building with GAS with the -march=r5900 flag produces: - * - trunc.w.s is built as the cvt.w.s instruction. - * - cvt.w.s errors complaining as not being supported by the processor. - * - * To ensure the produced disassembly will still match when built with GAS, we decode this two - * instructions as .word - */ - return true; + if (self->category == RABBITIZER_INSTRCAT_R5900) { + /** + * Due to the R5900's FPU being non properly complaint, the instruction cvt.w.s always behaves as + * trunc.w.s because EE can only do round-to-zero. + * + * Assemblers like GAS workaround this issue by decoding cvt.w.s as trunc.w.s, but other assemblers + * just use trunc.w.s and cvt.w.s as-is. + * + * Here's some reading about the binutils rationale: + * - https://sourceware.org/legacy-ml/binutils/2012-11/msg00360.html + * - https://sourceware.org/pipermail/binutils/2013-January/079863.html + * + * Because of this, building with GAS with the -march=r5900 flag produces: + * - trunc.w.s is built as the cvt.w.s instruction. + * - cvt.w.s errors complaining as not being supported by the processor. + * + * To ensure the produced disassembly will still match when built with GAS, we decode this two + * instructions as .word + */ + return true; + } break; default: diff --git a/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_OperandType.c b/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_OperandType.c index d420e1c5..5ed7ce5f 100644 --- a/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_OperandType.c +++ b/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_OperandType.c @@ -15,7 +15,7 @@ size_t RabbitizerOperandType_process_r5900_I(UNUSED const RabbitizerInstruction UNUSED const char *immOverride, UNUSED size_t immOverrideLength) { size_t totalSize = 0; - RABUTILS_BUFFER_CPY(dst, totalSize, "$I"); + RABUTILS_BUFFER_CPY(dst, totalSize, "I"); return totalSize; } @@ -24,7 +24,7 @@ size_t RabbitizerOperandType_process_r5900_Q(UNUSED const RabbitizerInstruction UNUSED const char *immOverride, UNUSED size_t immOverrideLength) { size_t totalSize = 0; - RABUTILS_BUFFER_CPY(dst, totalSize, "$Q"); + RABUTILS_BUFFER_CPY(dst, totalSize, "Q"); return totalSize; } @@ -33,7 +33,7 @@ size_t RabbitizerOperandType_process_r5900_R(UNUSED const RabbitizerInstruction UNUSED const char *immOverride, UNUSED size_t immOverrideLength) { size_t totalSize = 0; - RABUTILS_BUFFER_CPY(dst, totalSize, "$R"); + RABUTILS_BUFFER_CPY(dst, totalSize, "R"); return totalSize; } @@ -42,7 +42,7 @@ size_t RabbitizerOperandType_process_r5900_ACC(UNUSED const RabbitizerInstructio UNUSED const char *immOverride, UNUSED size_t immOverrideLength) { size_t totalSize = 0; - RABUTILS_BUFFER_CPY(dst, totalSize, "$ACC"); + RABUTILS_BUFFER_CPY(dst, totalSize, "ACC"); return totalSize; } From 96402dac2ab7c81ee74a97f800777f339c1a4ca2 Mon Sep 17 00:00:00 2001 From: angie Date: Mon, 17 Apr 2023 15:30:38 -0400 Subject: [PATCH 7/7] version bump --- Cargo.toml | 2 +- include/common/RabbitizerVersion.h | 4 ++-- pyproject.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 79db3be7..82b00f9f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ [package] name = "rabbitizer" # Version should be synced with include/common/RabbitizerVersion.h -version = "1.5.11" +version = "1.6.0" edition = "2021" authors = ["Anghelo Carvajal "] description = "MIPS instruction decoder" diff --git a/include/common/RabbitizerVersion.h b/include/common/RabbitizerVersion.h index 2d8760a5..82da890d 100644 --- a/include/common/RabbitizerVersion.h +++ b/include/common/RabbitizerVersion.h @@ -13,8 +13,8 @@ extern "C" { // Header version #define RAB_VERSION_MAJOR 1 -#define RAB_VERSION_MINOR 5 -#define RAB_VERSION_PATCH 11 +#define RAB_VERSION_MINOR 6 +#define RAB_VERSION_PATCH 0 #define RAB_VERSION_STR RAB_STRINGIFY(RAB_VERSION_MAJOR) "." RAB_STRINGIFY(RAB_VERSION_MINOR) "." RAB_STRINGIFY(RAB_VERSION_PATCH) diff --git a/pyproject.toml b/pyproject.toml index 73e60916..6e134eb0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ [project] name = "rabbitizer" # Version should be synced with include/common/RabbitizerVersion.h -version = "1.5.11" +version = "1.6.0" description = "MIPS instruction decoder" # license = "MIT" readme = "README.md"