SUMMARY gcc with compiler flag -Wduplicated-cond says: priv/guest_ppc_toIR.c: In function 'disInstr_PPC_WRK': priv/guest_ppc_toIR.c:36198:31: warning: duplicated 'if' condition [-Wduplicated-cond] 36198 | } else if (is_prefix && ( ptype == pType1 ) ) { // plbz: load instruction | ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~ priv/guest_ppc_toIR.c:36193:24: note: previously used here 36193 | if (is_prefix && ( ptype == pType1 ) ) { | ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~ EXPECTED RESULT SOFTWARE/OS VERSIONS Windows: macOS: Linux/KDE Plasma: (available in About System) KDE Plasma Version: KDE Frameworks Version: Qt Version: ADDITIONAL INFORMATION
Still there, over a year later.
This is probably harmless, but I don't know whether the else should be removed or the condition changed. I've added Carl to the cc list.
Thanks for bringing this to my attention. Yes, something looks wrong. I think there is an issue with the condition, will need to look at it a bit more. Don't think removing the if statement is correct. I will work on it shortly, just getting back from vacation at the moment.
The current code: if (is_prefix && ( ptype == pType1 ) ) { if ( !(allow_isa_3_1) ) goto decode_noIsa3_1; // splat instructions: xxpermx if (dis_vector_permute_prefix( prefix, theInstr, abiinfo )) goto decode_success; } else if (is_prefix && ( ptype == pType1 ) ) { // plbz: load instruction if ( !(allow_isa_3_1) ) goto decode_noIsa3_1; if (dis_int_load_prefix( prefix, theInstr )) goto decode_success; } else { // lbz: load instruction if (dis_int_load_prefix( prefix, theInstr )) goto decode_success; } The above code is wrong. The else if should be checking for the plbz instruction. The plbz instruction is pType2 not 1. The else if case fails for the plbz instruction and we hit the else statement. The else statement is supposed to only succeed for the lpz instruction. However, it is actually handling both the plbz and lbz instructions. The same function dis_int_load_prefix() handles both the prefix and non-prefixed versions of the instruction. They are distinquished by the prefix argument. The else statement should only succeed for the lbz instruction, i.e. not a prefixed instruction. So we really have two errors, the else if should check for pType2 and the else should really be else if (!prefix). The code should be: if (is_prefix && ( ptype == pType1 ) ) { if ( !(allow_isa_3_1) ) goto decode_noIsa3_1; // splat instructions: xxpermx if (dis_vector_permute_prefix( prefix, theInstr, abiinfo )) goto decode_success; } else if (is_prefix && ( ptype == pType2 ) ) { // plbz: load instruction if ( !(allow_isa_3_1) ) goto decode_noIsa3_1; if (dis_int_load_prefix( prefix, theInstr )) goto decode_success; } else if (!is_prefix) { // lbz: load instruction if (dis_int_load_prefix( prefix, theInstr )) goto decode_success; } Tested the original code and verified the else was handling both the prefixed and non-prefixed lbz instructions. Tested the correct code to verify that the prefixed instruction matches is_prefix and ptype == pType2. Ran the regression tests to verify the correct code does not introduce any failures. In this case, I just got lucky and the code did the right thing in spite of the coding error. Better to be lucky than good! :-)
Patch committed. commit 873f3766955c4f5caacc014dbe3d4fa0a4f6f712 (HEAD -> master, origin/master, origin/HEAD) Author: Carl Love <cel@us.ibm.com> Date: Mon Oct 31 13:29:31 2022 -0400 Bug 444110 priv/guest_ppc_toIR.c: warning: duplicated 'if' condition The compiler reported a duplicated condition in VEX/priv/guest_ppc_toIR.c The handling of the plbz and xxpermx instructions have the same if/elseif conditions. The else if condition for the plbz instruction was wrong. The elseif statement should be checking for pType2 not pType1. The plbz instruction was inadvertently being handled by the else statement for the lbz instruction. This patch fixes the checking for the plbz and lbz instructions.