https://bugs.kde.org/show_bug.cgi?id=363858
--- Comment #5 from Julian Seward <jsew...@acm.org> ---
(In reply to Carl Love from comment #1)
> Created attachment 99327 [details]
> Patch 4 of 5 to add VEX support for the POWER ISA 3.0 instructions
Main comments are:
* does this have any performance effect on plain boring integer
code? I ask because it seems like it might affect the IR generated
for CR321 and CR0, which are used a lot for comparisons, but I am
not sure.
"make perf", then
perf/vg_perf --vg=/path/to/old/tree --vg=/path/to/new/tree --reps=5
gives you some idea
* I am somewhat concerned that some of these translations are
going to generate a huge amount of IR and hence a huge amount of
code at the back end, and I wonder if the more complex ones
would be better done with C helper functions.
Can you trigger any JIT assertion failures with worst-case code-size
settings, by running all the test cases with
--tool=memcheck --track-origins=yes --vex-iropt-level=0
?
* And some minor things below.
VEX/priv/guest_ppc_toIR.c
You swapped putCR321 and putCR0 ? Any actual change? Is confusing.
Please un-swap.
+static IRExpr * CmpGT128U (IRExpr *src1, IRExpr *src2)
+ /* Do and unsigend compare of two 128-bit values */
Fix typos
+static IRTemp increment_BCDstring (IRExpr *src, IRExpr *carry_in)
This seems like it will generate a huge amount of IR. Is it
something you can do in a helper function, in the style of
amd64g_dirtyhelper_AESKEYGENASSIST ?
+static Bool dis_int_misc ( UInt theInstr )
+ * to be suspende. Instruction fetching and execution are resumed
suspende += d
Un-indent the default: case one level. It is misleadingly indented.
+ case 0x12D: // lxvll (Load VSX Vector Left-Justified with Length XX1 form)
Without looking at the context I can't tell, but .. does this need "gating"
(ie is only allowed for some hardware variants) ?
+ /* nb_zero is 0xFF..FF if the nb_field = 0 */
+ assign( nb_zero, binop( Iop_64HLtoV128,
+ unop( Iop_1Sto64,
+ binop( Iop_CmpEQ64,
+ mkexpr( nb ),
+ mkU64( 0 ) ) ),
+ unop( Iop_1Sto64,
+ binop( Iop_CmpEQ64,
+ mkexpr( nb ),
+ mkU64( 0 ) ) ) ) );
Lift the duplicated 1Sto64(CmpEQ64, nb, 0) term onto its own IRTemp
and use that. Or maybe we should enable CSE by default in iropt for
POWER? That has a JIT-time cost, though.
+ case 0x1AD: // stxvll (Store VSX Vector Left-justified with length
XX1-form)
Comments as for lxvll
+static Bool dis_av_bcd_misc ( UInt theInstr )
+ switch (opc2) {
+ case 0x341: // bcdcpsgn. Decimal Copy Sign VX-form
+ {
Please fix indentation to match house style
--
You are receiving this mail because:
You are watching all bug changes.
