RISC-V Inefficient Codegen with Scoped VLAs

[lenary]

https://godbolt.org/z/Tfn6s5rrc

The code here has a VLA inside an inner scope, so uses @llvm.stacksave and @llvm.stackrestore to save/restore the stack inside the code. The variable sized frame object is added during isel (fi#0).

At the end of the function, there is this perculiar sequence:

        mv      sp, a0
        li      a0, 1
        addi    sp, s0, -16
        qc.cm.pop       {ra, s0}, 16

The mv and the addi both put the same value back into sp. We don't need both from what I can see.

The mv comes from a COPY introduced by isel for @llvm.stackrestore. The addi is introduced by PEI.

---

I have also seen examples where the initial update of SP for the vla's dynamic allocation is done inefficiently: https://godbolt.org/z/3f3cec9jx

        qc.cm.pushfp    {ra, s0}, -16
        mv      a0, sp
        mv      a3, sp
        addi    a2, a3, -16
        mv      sp, a2
...
        mv      sp, a0
        li      a0, 1
        addi    sp, s0, -16
        qc.cm.pop       {ra, s0}, 16

The mv; addi; mv sequence could just be addi sp, sp, -16. It looks to be happening during ISel as well, rather than in PEI. I hope that can be made more efficient.

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: Sam Elliott (lenary)

https://godbolt.org/z/Tfn6s5rrc

The code here has a VLA inside an inner scope, so uses @<!-- -->llvm.stacksave and @<!-- -->llvm.stackrestore to save/restore the stack inside the code. The variable sized frame object is added during isel (fi#<!-- -->0).

At the end of the function, there is this perculiar sequence:

        mv      sp, a0
        li      a0, 1
        addi    sp, s0, -16
        qc.cm.pop       {ra, s0}, 16

The mv and the addi both put the same value back into sp. We don't need both from what I can see.

The mv comes from a COPY introduced by isel for @<!-- -->llvm.stackrestore. The addi is introduced by PEI.

---

I have also seen examples where the initial update of SP for the vla's dynamic allocation is done inefficiently: https://godbolt.org/z/3f3cec9jx

        qc.cm.pushfp    {ra, s0}, -16
        mv      a0, sp
        mv      a3, sp
        addi    a2, a3, -16
        mv      sp, a2
...
        mv      sp, a0
        li      a0, 1
        addi    sp, s0, -16
        qc.cm.pop       {ra, s0}, 16

The mv; addi; mv sequence could just be addi sp, sp, -16. It looks to be happening during ISel as well, rather than in PEI. I hope that can be made more efficient.

[lenary]

The other weird (maybe required?) thing is that in the second example fi#0 is marked as dynamic, when we actually know the size in IR: %vla16 = alloca [3 x i32].

[topperc]

The mv; addi; mv sequence could just be addi sp, sp, -16. It looks to be happening during ISel as well, rather than in PEI. I hope that can be made more efficient.

Isn't the a3 result from the first mv used a second time? The result from the addi is also used again.

[lenary]

The mv; addi; mv sequence could just be addi sp, sp, -16. It looks to be happening during ISel as well, rather than in PEI. I hope that can be made more efficient.

Isn't the a3 result from the first mv used a second time? The result from the addi is also used again.

Yes, I should have looked closer.

I have seen examples where the result of the first mv is only used again in the mv sp, ... near the end of the function, which I think is redundant (from above).

For the addi, I don't quite understand why this cannot just use sp, but I am only looking at an example with one VLA, maybe examples with two (differently-) scoped VLAs would clarify.

[efriedma-quic]

fi#0 is marked as dynamic

The definition of a "static alloca" in LLVM IR is an a alloca with constant size in the entry block. The alloca in question isn't in the entry block, so it's dynamic. (We could hoist the alloca into the entry block here to make it static, but that's complicated enough that nobody has bothered.)

The mv comes from a COPY introduced by isel for @llvm.stackrestore. The addi is introduced by PEI.

Right, we'd need some late peephole to remove the extra stack restore. We can't really tell the stackrestore is redundant where we lower it.

The mv; addi; mv sequence could just be addi sp, sp, -16.

In general we need to do some more complicated math to compute the correct stack adjustment. In this specific case, we could just adjust SP then copy SP to a register, sure.