Vineet,
Your understanding of the ABI is correct; both int and unsigned int
arguments must already be sign-extended. The sext.w is semantically
unnecessary; the bltu could correctly reference a1 instead of a6.
Good luck eliminating it!
Andrew
On Mon, Sep 18, 2023 at 12:45 PM Vineet Gupta <vine...@rivosinc.com> wrote:
>
> Hi Jeff, Andrew
>
> I've been looking into redundant sign extension and while there are
> things to be improved in REE, there's something I wanted to confirm
> before heading off into the weeds.
>
> Consider the test below:
>
> int foo(int unused, int n, unsigned y, unsigned delta){
> int s = 0;
> unsigned int x = 0; // if int, sext elided
> for (;x<n;x +=delta)
> s += x+y;
> return s;
> }
>
> -O2 -march=rv64gc_zba_zbb_zbs
>
> foo2:
> sext.w a6,a1 # 1
> beq a1,zero,.L4
> li a5,0
> li a0,0
> .L3:
> addw a4,a2,a5
> addw a5,a3,a5
> addw a0,a4,a0
> bltu a5,a6,.L3
> ret
> .L4:
> li a0,0
> ret
>
> I believe the SEXT.W is not semantically needed as a1 is supposed to be
> sign extended already at call site as per psABI [1]. I quote
>
> "When passed in registers or on the stack, integer scalars narrower
> than XLEN bits are widened according to the sign of their type up to 32
> bits, then sign-extended to XLEN bits"
>
> However currently RISC-V backend thinks otherwise: changing @x to int,
> causes the the sign extend to go away. I think both the cases should
> behave the same (and not generate SEXT.w) given the ABI clause above.
> Note that this manifests in initial RTL expand itself
> generating/or-not-generating the sign_extend so if it is unnecessary we
> can avoid late fixups in REE.
>
> So the questions is for you to confirm if my conclusions above are correct.
>
> For the cases which do require sign extends, but not being eliminated
> due to "missing definition(s)" I'm working on adapting Ajit's REE ABI
> interfaces work [2] to work for RISC-V as well.
>
> Thx,
> -Vineet
>
> [1]
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc
> [2] https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630713.html
