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.

Reply via email to