[Clang][RISCV] Unexpectedly lost writes to C99 flexible array member when guaranteed return copy elision should apply

[resistor]

Consider a function on RV32 that returns this struct by value:

struct FooStruct {
    int a;
    int b;
    int c[];
};

FooStruct foo() {
    FooStruct ret;

    ret.a = 0;
    ret.b = 0;
    ret.c[0] = 0x124512;
    ret.c[1] = 0x535;

    return ret;
}

Because of guaranteed return copy elision in C++17, the caller of foo should be allocating space for the return value, and providing a hidden "out" parameter for foo to write through.

This is complicated in this case because this struct hits the case in the RV ABI where two XLEN-size members are passed in registers rather than on the stack.

What I would expect to happen is that either the flexible array member would force the entire structure to be returned on the stack, or that a and b are returned in registers, and an out-pointer is taken as an argument and used to write to c.

What actually seems to happen is that the writes to c are entirely elided: https://godbolt.org/z/P7b1TzjTq

foo():
        li      a0, 0
        li      a1, 0
        ret

If the types of the a and b fields are changed to long long (thereby avoiding the two-XLEN-sized-fields ABI case), then the writes to c are suddenly preserved!

foo():
        sw      zero, 0(a0)
        sw      zero, 4(a0)
        sw      zero, 8(a0)
        sw      zero, 12(a0)
        lui     a1, 292
        li      a2, 1333
        addi    a1, a1, 1298
        sw      a1, 16(a0)
        sw      a2, 20(a0)
        ret

[resistor]

I believe this is some form of frontend ABI lowering bug, as the generated IR has already dropped the stores through c: https://godbolt.org/z/x8q8Pc5Wd

define dso_local [2 x i32] @foo()() local_unnamed_addr #0 {
  ret [2 x i32] zeroinitializer
}

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: Owen Anderson (resistor)

Consider a function on RV32 that returns this struct by value:

struct FooStruct {
    int a;
    int b;
    int c[];
};

FooStruct foo() {
    FooStruct ret;

    ret.a = 0;
    ret.b = 0;
    ret.c[0] = 0x124512;
    ret.c[1] = 0x535;

    return ret;
}

Because of guaranteed return copy elision in C++17, the caller of foo should be allocating space for the return value, and providing a hidden "out" parameter for foo to write through.

This is complicated in this case because this struct hits the case in the RV ABI where two XLEN-size members are passed in registers rather than on the stack.

What I would expect to happen is that a and b are returned in registers, and an out-pointer is taken as an argument and used to write to c.

What actually seems to happen is that the writes to c are entirely elided: https://godbolt.org/z/P7b1TzjTq

foo():
        li      a0, 0
        li      a1, 0
        ret

If the types of the a and b fields are changed to long long (thereby avoiding the two-XLEN-sized-fields ABI case), then the writes to c are suddenly preserved!

foo():
        sw      zero, 0(a0)
        sw      zero, 4(a0)
        sw      zero, 8(a0)
        sw      zero, 12(a0)
        lui     a1, 292
        li      a2, 1333
        addi    a1, a1, 1298
        sw      a1, 16(a0)
        sw      a2, 20(a0)
        ret
</details>

[resistor]

In some quick research, it looks like most other clang targets have explicit checks for arguments that hasFlexibleArrayMember, and force indirect passing for them.

[resistor]

Fix proposed in #148541

[lenary]

This needs some kind of note in the RISC-V ABI document itself, if there isn't one already.

[resistor]

This needs some kind of note in the RISC-V ABI document itself, if there isn't one already.

Can you link where to propose it?

[lenary]

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#procedure-calling-convention somewhere around here.

[resistor]

Proposed change to the ABI doc in riscv-non-isa/riscv-elf-psabi-doc#471