On 18 January 2011 14:34, Christophe Lyon <christophe.l...@st.com> wrote:
> +
> + /* gen_helper_neon_mull_[su]{8|16} do not free their parameters.
> + Don't forget to clean them now. */
> + switch ((size << 1) | u) {
> + case 0:
> + case 1:
> + case 2:
> + case 3:
> + dead_tmp(a);
> + dead_tmp(b);
> + break;
> + }
> }
This seems a rather convoluted way to write "if (size < 2) { ... }"
> @@ -5235,9 +5245,12 @@ static int disas_neon_data_insn(CPUState * env,
> DisasContext *s, uint32_t insn)
> tmp = neon_load_reg(rn, 0);
> } else {
> tmp = tmp3;
> + /* tmp2 has been discarded in
> + gen_neon_mull during pass 0, we need to
> + recreate it. */
> + tmp2 = neon_get_scalar(size, rm);
> }
I think this will give the wrong results for instructions where the
scalar operand is in the same Neon register as the destination
for the first pass, because calling neon_get_scalar() again will
do a reload from the Neon register and it might have changed.
(Also loading it once at the start rather than in every pass is
more efficient as well as being correct :-))
Also your patch has hard-coded tabs in it (please see
CODING_STYLE on the subject of whitespace) and your
mail client or server has line-wrapped long lines in the patch
so it doesn't apply cleanly...
-- PMM
