https://bugs.kde.org/show_bug.cgi?id=429375
--- Comment #3 from Julian Seward <jsew...@acm.org> --- ================== https://bugs.kde.org/attachment.cgi?id=133479 test cases for VSX-PCV generate operations OK to land ================== https://bugs.kde.org/attachment.cgi?id=134757 VSX- PCV Generate Operation functional support Some concerns here. /*---------------------------------------------------------------*/ /*--- Misc integer helpers. ---*/ /*---------------------------------------------------------------*/ +void write_VSX_entry (VexGuestPPC64State* gst, UInt reg_offset, + ULong *vsx_entry); Mark this as static or give it a comment that it is called from generated code (one or the other) Also it seems like this function prototype is in a .c file, not a header. Seems not right. +/*--------------------------------------------------*/ +/*---- VSX Vector Generate PCV from Mask helpers ---*/ +/*--------------------------------------------------*/ +void write_VSX_entry (VexGuestPPC64State* gst, UInt reg_offset, + ULong *vsx_entry) comment or 'static' pls +void vector_gen_pvc_byte_mask_dirty_helper( VexGuestPPC64State* gst, + ULong src_hi, ULong src_lo, + UInt reg_offset, UInt imm ) { and here + } else { + vex_printf("ERROR, vector_gen_pvc_byte_mask_dirty_helper, imm value %u not supported.\n", + imm); + } After printing, do vassert(0); so the system stops, rather than continues with incorrect data. + return; +} the return is redundant +void vector_gen_pvc_hword_mask_dirty_helper( VexGuestPPC64State* gst, + ULong src_hi, ULong src_lo, + UInt reg_offset, UInt imm ) { comment or 'static' + } else { + vex_printf("ERROR, vector_gen_pvc_hword_dirty_mask_helper, imm value %u not supported.\n", + imm); + } and assert, and rm redundant return +void vector_gen_pvc_word_mask_dirty_helper( VexGuestPPC64State* gst, + ULong src_hi, ULong src_lo, + UInt reg_offset, UInt imm ) { same comments +void vector_gen_pvc_dword_mask_dirty_helper( VexGuestPPC64State* gst, + ULong src_hi, ULong src_lo, + UInt reg_offset, UInt imm ) { same comments + IREffect VSX_fx = Ifx_Write; Inline this at its single use point, since it is never changed. + IRExpr** args = mkIRExprVec_5( + IRExpr_GSPTR(), + mkexpr( src_hi ), + mkexpr( src_lo ), + mkU32( reg_offset ), + mkU64( IMM ) ); + + + No need for three blank lines here. + d->nFxState = 1; + vex_bzero(&d->fxState, sizeof(d->fxState)); + d->fxState[0].fx = VSX_fx; + d->fxState[0].size = sizeof(U128); + d->fxState[0].offset = reg_offset; You're sure this declaration of effects is correct, right? The called helpers really only writes one of the vector registers and doesn't read any? + default: + vex_printf("dis_vector_generate_pvc_from_mask (opc2)\n"); + return False; + } Don't print in failure paths; just return False. ================== -- You are receiving this mail because: You are watching all bug changes.
