https://bugs.kde.org/show_bug.cgi?id=385409
--- Comment #75 from Andreas Arnez <ar...@linux.ibm.com> --- (In reply to Julian Seward from comment #74) > 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 :-) Hm, I'd really prefer to drop this initialization, because (1) zero translates to S390_ZERO_EXTEND_8, which I don't consider a useful initialization value in this case and (2) the initialization would prevent the compiler from identifying code paths where the variable may be used before assigned to. > > + 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 :-) I can surely do that. The reason I put the * next to the name is because the vast majority of "IRExpr *" declarations in this file is written like that. -- You are receiving this mail because: You are watching all bug changes.
