On 2020/7/14 21:59, Frank Chang wrote:
On Tue, Jul 14, 2020 at 9:21 PM Richard Henderson <richard.hender...@linaro.org <mailto:richard.hender...@linaro.org>> wrote:

    On 7/13/20 7:59 PM, Frank Chang wrote:
    > The latest spec specified:
    >
    > Only the low *lg2(SEW) bits* are read to obtain the shift amount
    from a
    > *register value*.
    > The *immediate* is treated as an *unsigned shift amount*, with a
    *maximum shift
    > amount of 31*.

    Which, I hope you will agree is underspecified, and should be
    reported as a bug
    in the manual.


Yes, you're correct.
I found out I missed the MASK part in GEN_VEXT_SHIFT_VV() macro,
which this macro is shared between OPIVV and OPIVI format of instructions.
So the same masking methodology should be applied to shift amounts coming from both register and immediate value.
Spike also does something like:
/vs2 >> (zimm5 & (sew - 1) & 0x1f);/ for SEW = 8.

I think spec is kind of misleading to the reader by the way it expresses.


    > Looks like the shift amount in the immediate value is not
    relevant with SEW
    > setting.

    How can it not be?  It is when the value comes from a register...

    > If so, is it better to just use do_opivi_gvec() and implement
    the logic by our
    > own rather than using gvec IR?

    No, it is not.  What is the logic you would apply on your own? 
    There should be
    a right answer.


I was talking about just calling GEN_OPIVI_TRANS() to generate the helper functions
defined in vector_helper.c as what I'm doing in the original patch.
But as long as the immediate value should also apply *lg2(SEW) bits* truncating,
I think I should update GEN_OPIVI_GVEC_TRANS() to utilize gvec IR.


    If the answer is that out-of-range shift produces zero, which some
    architectures use, then you can look at the immediate value, see
    that you must
    supply zero, and then fill the vector with zeros from translate. 
    You need not
    call a helper to perform N shifts when you know the result a-priori.

    If the answer is that shift values are truncated, which riscv uses
    *everywhere
    else*, then you should truncate the immediate value during translate.


I think vsll.vi <http://vsll.vi>, vsrl.vi <http://vsrl.vi> and vsra.vi <http://vsra.vi> truncate the out-of-range shift as other riscv instructions.
I will double confirm that, thanks for the advice.

Perhaps the reason is that the assembler can't identify if an imm is legal according to log(SEW),
because the assembler can't get the SEW.

To make more compliance with assembler directly users'  intuition, just let the imm encoding to insn as itself(truncate to 31)
and work as itself.

Zhiwei
Frank Chang



    r~


Reply via email to