From 3f1165a9cad73c1d17ad13190ec2d03d98ea858e Mon Sep 17 00:00:00 2001 From: Christian Gmeiner Date: Sun, 14 Dec 2025 13:21:34 +0100 Subject: [PATCH] etnaviv: isa: Remove dual16 mode parameter from parser API The dual16 mode is now fully encoded in the disassembly output through type suffixes (:f16 vs :f20) and register naming (th vs t), making the explicit dual16 mode parameter redundant. Previously, the parser needed a dual16_mode flag to determine whether float immediates should use imm_type=0 (f32) or imm_type=3 (f16). Now that the disassembler emits explicit :f16/:f20 suffixes, the parser can determine the correct encoding directly from the input text. This simplifies the API by removing the dual16_mode parameter from: - isa_parse_str() - isa_parse_file() - Internal parsing functions Signed-off-by: Christian Gmeiner Reviewed-by: @LingMan Part-of: --- src/etnaviv/isa/api.rs | 19 ++++++----------- src/etnaviv/isa/assembler.c | 8 ++------ src/etnaviv/isa/isa.h | 4 ++-- src/etnaviv/isa/parser.rs | 35 +++++++++++++++++++++----------- src/etnaviv/isa/tests/disasm.cpp | 19 ++++++++--------- 5 files changed, 42 insertions(+), 43 deletions(-) diff --git a/src/etnaviv/isa/api.rs b/src/etnaviv/isa/api.rs index f0b390fa3cd..e80e63cffd4 100644 --- a/src/etnaviv/isa/api.rs +++ b/src/etnaviv/isa/api.rs @@ -16,8 +16,7 @@ use std::os::raw::c_char; /// See https://doc.rust-lang.org/std/ffi/struct.CStr.html#safety for further details. unsafe fn isa_parse( c_str: *const c_char, - dual_16_mode: bool, - func: impl FnOnce(&str, bool, &mut etna_asm_result), + func: impl FnOnce(&str, &mut etna_asm_result), ) -> *mut etna_asm_result { let mut result = Box::new(etna_asm_result::default()); @@ -30,7 +29,7 @@ unsafe fn isa_parse( let c_str_safe = unsafe { CStr::from_ptr(c_str) }; if let Ok(str) = c_str_safe.to_str() { - func(str, dual_16_mode, &mut result); + func(str, &mut result); } else { result.set_error("Failed to convert CStr to &str"); result.success = false; @@ -44,12 +43,9 @@ unsafe fn isa_parse( /// `c_str` must point to a valid nul-termintated string not longer than `isize::MAX` bytes. /// See https://doc.rust-lang.org/std/ffi/struct.CStr.html#safety for further details. #[no_mangle] -pub unsafe extern "C" fn isa_parse_str( - c_str: *const c_char, - dual_16_mode: bool, -) -> *mut etna_asm_result { +pub unsafe extern "C" fn isa_parse_str(c_str: *const c_char) -> *mut etna_asm_result { // SAFETY: As per the safety section, the caller has to uphold these requirements - unsafe { isa_parse(c_str, dual_16_mode, asm_process_str) } + unsafe { isa_parse(c_str, asm_process_str) } } /// # Safety @@ -57,12 +53,9 @@ pub unsafe extern "C" fn isa_parse_str( /// `c_filepath` must point to a valid nul-termintated string not longer than `isize::MAX` bytes. /// See https://doc.rust-lang.org/std/ffi/struct.CStr.html#safety for further details. #[no_mangle] -pub unsafe extern "C" fn isa_parse_file( - c_filepath: *const c_char, - dual_16_mode: bool, -) -> *mut etna_asm_result { +pub unsafe extern "C" fn isa_parse_file(c_filepath: *const c_char) -> *mut etna_asm_result { // SAFETY: As per the safety section, the caller has to uphold these requirements - unsafe { isa_parse(c_filepath, dual_16_mode, asm_process_file) } + unsafe { isa_parse(c_filepath, asm_process_file) } } /// # Safety diff --git a/src/etnaviv/isa/assembler.c b/src/etnaviv/isa/assembler.c index acca92d50ee..090f4b91ad8 100644 --- a/src/etnaviv/isa/assembler.c +++ b/src/etnaviv/isa/assembler.c @@ -55,12 +55,11 @@ int main(int argc, char *argv[]) { bool show_disasm = false; - bool dual_16_mode = false; const char *in = NULL; const char *out = NULL; int opt = 0; - while ((opt = getopt(argc, argv, "i:o:sd")) != -1) { + while ((opt = getopt(argc, argv, "i:o:s")) != -1) { switch (opt) { case 'i': in = optarg; @@ -71,9 +70,6 @@ main(int argc, char *argv[]) case 's': show_disasm = true; break; - case 'd': - dual_16_mode = true; - break; default: print_usage(); exit(EXIT_FAILURE); @@ -86,7 +82,7 @@ main(int argc, char *argv[]) return EXIT_FAILURE; } - struct etna_asm_result *result = isa_parse_file(in, dual_16_mode); + struct etna_asm_result *result = isa_parse_file(in); if (!result->success) { fprintf(stderr, "Failed to parse %s\n%s\n", in, result->error); diff --git a/src/etnaviv/isa/isa.h b/src/etnaviv/isa/isa.h index 631287446e0..ef69e709aef 100644 --- a/src/etnaviv/isa/isa.h +++ b/src/etnaviv/isa/isa.h @@ -17,8 +17,8 @@ extern "C" { void isa_assemble_instruction(uint32_t *out, const struct etna_inst *instr); -extern struct etna_asm_result *isa_parse_str(const char *str, bool dual_16_mode); -extern struct etna_asm_result *isa_parse_file(const char *filepath, bool dual_16_mode); +extern struct etna_asm_result *isa_parse_str(const char *str); +extern struct etna_asm_result *isa_parse_file(const char *filepath); extern void isa_asm_result_destroy(struct etna_asm_result *result); #ifdef __cplusplus diff --git a/src/etnaviv/isa/parser.rs b/src/etnaviv/isa/parser.rs index d1725e12a07..d3f21944d2c 100644 --- a/src/etnaviv/isa/parser.rs +++ b/src/etnaviv/isa/parser.rs @@ -111,7 +111,7 @@ fn fill_tex(pair: Pair, tex: &mut etna_inst_tex) { } } -fn fill_source(pair: Pair, src: &mut etna_inst_src, dual_16_mode: bool) { +fn fill_source(pair: Pair, src: &mut etna_inst_src) { src.set_use(1); for item in pair.into_inner() { @@ -148,17 +148,28 @@ fn fill_source(pair: Pair, src: &mut etna_inst_src, dual_16_mode: bool) { imm_struct.set_imm_type(0); imm_struct.set_imm_val(0xfffff); } - Rule::Immediate_float | Rule::Immediate_half_float => { + Rule::Immediate_float => { let value: f32 = parse_numeric(item); let bits = value.to_bits(); assert!((bits & 0xfff) == 0); /* 12 lsb cut off */ src.set_rgroup(isa_reg_group::ISA_REG_GROUP_IMMED); - let imm_type = if dual_16_mode { 3 } else { 0 }; let imm_struct = unsafe { &mut src.__bindgen_anon_1.__bindgen_anon_2 }; - imm_struct.set_imm_type(imm_type); + imm_struct.set_imm_type(0); + imm_struct.set_imm_val(bits >> 12); + } + Rule::Immediate_half_float => { + let value: f32 = parse_numeric(item); + let bits = value.to_bits(); + + assert!((bits & 0xfff) == 0); /* 12 lsb cut off */ + src.set_rgroup(isa_reg_group::ISA_REG_GROUP_IMMED); + + let imm_struct = unsafe { &mut src.__bindgen_anon_1.__bindgen_anon_2 }; + + imm_struct.set_imm_type(3); imm_struct.set_imm_val(bits >> 12); } Rule::Immediate_int => { @@ -200,7 +211,7 @@ fn fill_source(pair: Pair, src: &mut etna_inst_src, dual_16_mode: bool) { } } -fn process(input: Pair, dual_16_mode: bool) -> Option { +fn process(input: Pair) -> Option { // The assembler and disassembler are both using the // 'full' form of the ISA which contains void's and // use the HW ordering of instruction src arguments. @@ -273,7 +284,7 @@ fn process(input: Pair, dual_16_mode: bool) -> Option { // Nothing to do } Rule::SrcRegister => { - fill_source(p, &mut instr.src[src_index], dual_16_mode); + fill_source(p, &mut instr.src[src_index]); src_index += 1; } Rule::TexSrc => { @@ -290,13 +301,13 @@ fn process(input: Pair, dual_16_mode: bool) -> Option { Some(instr) } -fn parse(rule: Rule, content: &str, dual_16_mode: bool, asm_result: &mut etna_asm_result) { +fn parse(rule: Rule, content: &str, asm_result: &mut etna_asm_result) { let result = Isa::parse(rule, content); match result { Ok(program) => { for line in program { - if let Some(result) = process(line, dual_16_mode) { + if let Some(result) = process(line) { asm_result.append_instruction(result); } } @@ -310,12 +321,12 @@ fn parse(rule: Rule, content: &str, dual_16_mode: bool, asm_result: &mut etna_as } } -pub fn asm_process_str(string: &str, dual_16_mode: bool, asm_result: &mut etna_asm_result) { - parse(Rule::instruction, string, dual_16_mode, asm_result) +pub fn asm_process_str(string: &str, asm_result: &mut etna_asm_result) { + parse(Rule::instruction, string, asm_result) } -pub fn asm_process_file(file: &str, dual_16_mode: bool, asm_result: &mut etna_asm_result) { +pub fn asm_process_file(file: &str, asm_result: &mut etna_asm_result) { let content = fs::read_to_string(file).expect("cannot read file"); - parse(Rule::instructions, &content, dual_16_mode, asm_result) + parse(Rule::instructions, &content, asm_result) } diff --git a/src/etnaviv/isa/tests/disasm.cpp b/src/etnaviv/isa/tests/disasm.cpp index 27933437832..81ca19e988b 100644 --- a/src/etnaviv/isa/tests/disasm.cpp +++ b/src/etnaviv/isa/tests/disasm.cpp @@ -18,7 +18,6 @@ struct encoded_instr { uint32_t word[4]; }; -static const uint32_t FLAG_DUAL_16 = BITFIELD_BIT(0); static const uint32_t FLAG_FAILING_PARSE = BITFIELD_BIT(1); static const uint32_t FLAG_FAILING_ASM = BITFIELD_BIT(2); @@ -68,7 +67,7 @@ TEST_P(DisasmTest, basicOpCodes) EXPECT_STREQ(as.disasm, disasm_output); #ifndef HAVE_ETNAVIV_NO_PEST - struct etna_asm_result *result = isa_parse_str(disasm_output, as.flags & FLAG_DUAL_16); + struct etna_asm_result *result = isa_parse_str(disasm_output); EXPECT_TRUE(result); if (as.flags & FLAG_FAILING_PARSE) @@ -270,13 +269,13 @@ INSTANTIATE_TEST_SUITE_P(Threads, DisasmTest, disasm_state{ {0x01021001, 0x00002804, 0x00000020, 0xa1400008}, "add.hp.t1 t2._y__, th2.xxxx, void, -u0.xxxx\n"}, // full dual-16 shader from a deqp3 run on GC3000 - disasm_state{ {0x0101102e, 0x00201804, 0x80000020, 0x00002000}, "f2i.u32.t0 t1._y__, th1.xxxx, void, void\n", FLAG_DUAL_16}, - disasm_state{ {0x0101102e, 0x00202804, 0x80000020, 0x01000000}, "f2i.u32.t1 t1._y__, th2.xxxx, void, void\n", FLAG_DUAL_16}, - disasm_state{ {0x00811171, 0x15601804, 0x80000040, 0x76fffffa}, "cmp.eq.u32.t0 t1.x___, t1.yyyy, u0.xxxx, -1:s20\n", FLAG_DUAL_16}, - disasm_state{ {0x00811171, 0x15601804, 0x80000040, 0x77ffdffa}, "cmp.eq.u32.t1 t1.x___, t1.yyyy, u0.xxxx, -1:s20\n", FLAG_DUAL_16}, - disasm_state{ {0x0081158f, 0x00201804, 0x700000c0, 0x7c00000f}, "select.selmsb.s16 t1.x___, t1.xxxx, 0.000000:f16, 0.000000:f16\n", FLAG_DUAL_16 | FLAG_FAILING_ASM}, - disasm_state{ {0x0381102d, 0x00201804, 0x40000000, 0x00000000}, "i2f.s16 t1.xyz_, t1.xxxx, void, void\n", FLAG_DUAL_16}, - disasm_state{ {0x04011009, 0x00000004, 0x00000000, 0x20154008}, "mov t1.___w, void, void, u0.yyyy\n", FLAG_DUAL_16} + disasm_state{ {0x0101102e, 0x00201804, 0x80000020, 0x00002000}, "f2i.u32.t0 t1._y__, th1.xxxx, void, void\n"}, + disasm_state{ {0x0101102e, 0x00202804, 0x80000020, 0x01000000}, "f2i.u32.t1 t1._y__, th2.xxxx, void, void\n"}, + disasm_state{ {0x00811171, 0x15601804, 0x80000040, 0x76fffffa}, "cmp.eq.u32.t0 t1.x___, t1.yyyy, u0.xxxx, -1:s20\n"}, + disasm_state{ {0x00811171, 0x15601804, 0x80000040, 0x77ffdffa}, "cmp.eq.u32.t1 t1.x___, t1.yyyy, u0.xxxx, -1:s20\n"}, + disasm_state{ {0x0081158f, 0x00201804, 0x700000c0, 0x7c00000f}, "select.selmsb.s16 t1.x___, t1.xxxx, 0.000000:f16, 0.000000:f16\n", FLAG_FAILING_ASM}, + disasm_state{ {0x0381102d, 0x00201804, 0x40000000, 0x00000000}, "i2f.s16 t1.xyz_, t1.xxxx, void, void\n"}, + disasm_state{ {0x04011009, 0x00000004, 0x00000000, 0x20154008}, "mov t1.___w, void, void, u0.yyyy\n"} ) ); // clang-format on @@ -416,7 +415,7 @@ INSTANTIATE_TEST_SUITE_P(SwizzleVariants, DisasmTest, disasm_state{ {0x0782102b, 0x00002804, 0xa0000040, 0x7800002f}, "swizzle.s8 t2, t2.xxxx, 0:u20, 2:u20\n"}, disasm_state{ {0x0780102b, 0x00002804, 0xa0000040, 0x7800001f}, "swizzle.s8 t0, t2.xxxx, 0:u20, 1:u20\n"}, disasm_state{ {0x0781102b, 0x00002804, 0xa00000c0, 0x7800001f}, "swizzle.s8 t1, t2.xxxx, 1:u20, 1:u20\n"}, - disasm_state{ {0x0781102b, 0x00200804, 0x6000007e, 0x7800003f}, "swizzle.s16 t1, 0.000000:f16, 0:u20, 3:u20\n", FLAG_FAILING_ASM}, + disasm_state{ {0x0781102b, 0x00200804, 0x6000007e, 0x7800003f}, "swizzle.s16 t1, 0.000000:f16, 0:u20, 3:u20\n"}, disasm_state{ {0x0781102b, 0x00201804, 0x60000040, 0x780000cf}, "swizzle.s16 t1, t1.xxxx, 0:u20, 12:u20\n"}, disasm_state{ {0x0780102b, 0x00201804, 0x60000040, 0x7800003f}, "swizzle.s16 t0, t1.xxxx, 0:u20, 3:u20\n"}, disasm_state{ {0x0781102b, 0x00202804, 0x600000c0, 0x7800003f}, "swizzle.s16 t1, t2.xxxx, 1:u20, 3:u20\n"}