https://bugs.kde.org/show_bug.cgi?id=385409

--- Comment #74 from Julian Seward <jsew...@acm.org> ---
(In reply to Andreas Arnez from comment #73)
> Created attachment 115231 [details]
> Fixes based on Julian's comment #71
> 
> For reference, here's what I changed based on the comments above.  I'll look
> more into the disassembler tomorrow and commit the patches by end of
> tomorrow if no more findings appear.

Looks fine to me.  Just two minor nits:

--- a/VEX/priv/host_s390_isel.c
+++ b/VEX/priv/host_s390_isel.c

-      UInt vec_op = 0;
+      s390_unop_t vec_op;

Myself, I would have left the redundant initialiser there.  I like
initialisation :-)

+         IRExpr *low64     = IRExpr_Unop(Iop_V128to64, arg);
+         IRExpr *high64    = IRExpr_Unop(Iop_V128HIto64, arg);
+         IRExpr *both      = IRExpr_Binop(Iop_Or64, low64, high64);
+         IRExpr *anyNonZ   = IRExpr_Unop(Iop_CmpNEZ64, both);
+         IRExpr *anyNonZ64 = IRExpr_Unop(Iop_1Sto64, anyNonZ);

Can you put the * next to the type?  Yeah, I know it's technically
wrong/misleading etc.  But in the tree we use "T* v" (non-ugly, but
misleading) when there's only one variable, and "T *v1, *v2" in the
case where we have to be Technically Correct :-)

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to