no lifetime markers emitted for returned aggregates

[nickdesaulniers]

forked from #43598 (and https://reviews.llvm.org/D74094) which is C++ specific; there are different rules for C FWICT.

struct foo {
    long long x;
};

struct foo foo_factory (void);
struct foo div (struct foo, struct foo);

long long bar (void) {
    return div(div(foo_factory(), foo_factory()), div(foo_factory(), foo_factory())).x;
}

We should emit lifetime intrinsics for those temporary aggregates returned from functions

The latest C23 draft § 6.2.4 Storage durations of objects

8 A non-lvalue expression with structure or union type, where the structure or union contains a
member with array type (including, recursively, members of all contained structures and unions)
refers to an object with automatic storage duration and temporary lifetime. 39) Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends
when the evaluation of the containing full expression ends. Any attempt to modify an object with
temporary lifetime results in undefined behavior. An object with temporary lifetime behaves as if it
were declared with the type of its value for the purposes of effective type. Such an object need not
have a unique address.

Speaking more with @AaronBallman , Aaron found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1285.htm introduced the current wording into c11. https://wiki.sei.cmu.edu/confluence/display/c/EXP35-C.+Do+not+modify+objects+with+temporary+lifetime talks more about this. https://web.archive.org/web/20231010182056/https://yarchive.net/comp/struct_return.html talks more about this in regards to arrays within the returned struct.

Also, this is quite painful for 32b ARM (try building the above with -O2 -mabi=aapcs-linux --target=arm-linux-gnueabi) due to ARM AAPCS32 § 6.4 Result Return

A Composite Type larger than 4 bytes, or whose size cannot be determined statically by both caller and callee, is stored in memory at an address passed as an extra argument when the function was called (Parameter Passing (base PCS), Rule A.4). The memory to be used for the result may be modified at any point during the function call.

(I'll tag the ARM backend folks for the CC, though this isn't a backend specific bug) For other targets, clang unwraps the single i64 which results in much better codegen. Seems unfortunate the AAPCS32 didn't limit that rule about "composite types" to types larger than 8B rather than 4B.

So I think if the type contains an array, we need to have the lifetime be emitted after the full expression ends. CodeGenFunction::pushFullExprCleanup and CodeGenFunction::pushCleanupAfterFullExpr look useful, though I'm not sure that's precisely what they do (or what the difference is between them yet, maybe the latter is what I need). Otherwise I guess these temporaries can just be cleaned up immediately after the function call they are passed to.

cc @rjmccall @ilovepi

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: Nick Desaulniers (nickdesaulniers)

forked from #43598 (and https://reviews.llvm.org/D74094) which is C++ specific; there are different rules for C FWICT.

struct foo {
    long long x;
};

struct foo foo_factory (void);
struct foo div (struct foo, struct foo);

long long bar (void) {
    return div(div(foo_factory(), foo_factory()), div(foo_factory(), foo_factory())).x;
}

We should emit lifetime intrinsics for those temporary aggregates returned from functions

The latest C23 draft § 6.2.4 Storage durations of objects

> 8 A non-lvalue expression with structure or union type, where the structure or union contains a
member with array type (including, recursively, members of all contained structures and unions)
refers to an object with automatic storage duration and temporary lifetime. 39) Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends
when the evaluation of the containing full expression ends. Any attempt to modify an object with
temporary lifetime results in undefined behavior. An object with temporary lifetime behaves as if it
were declared with the type of its value for the purposes of effective type. Such an object need not
have a unique address.

Speaking more with @AaronBallman , Aaron found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1285.htm introduced the current wording into c11. https://wiki.sei.cmu.edu/confluence/display/c/EXP35-C.+Do+not+modify+objects+with+temporary+lifetime talks more about this. https://web.archive.org/web/20231010182056/https://yarchive.net/comp/struct_return.html talks more about this in regards to arrays within the returned struct.

Also, this is quite painful for 32b ARM due to ARM AAPCS32 § 6.4 Result Return

> A Composite Type larger than 4 bytes, or whose size cannot be determined statically by both caller and callee, is stored in memory at an address passed as an extra argument when the function was called (Parameter Passing (base PCS), Rule A.4). The memory to be used for the result may be modified at any point during the function call.

(I'll tag the ARM backend folks for the CC, though this isn't a backend specific bug)

So I think if the type contains an array, we need to have the lifetime be emitted after the full expression ends. CodeGenFunction::pushFullExprCleanup and CodeGenFunction::pushCleanupAfterFullExpr look useful, though I'm not sure that's precisely what they do (or what the difference is between them yet). Otherwise I guess these temporaries can just be cleaned up after the function call they are passed to.

[llvmbot]

@llvm/issue-subscribers-c

Author: Nick Desaulniers (nickdesaulniers)

forked from #43598 (and https://reviews.llvm.org/D74094) which is C++ specific; there are different rules for C FWICT.

struct foo {
    long long x;
};

struct foo foo_factory (void);
struct foo div (struct foo, struct foo);

long long bar (void) {
    return div(div(foo_factory(), foo_factory()), div(foo_factory(), foo_factory())).x;
}

We should emit lifetime intrinsics for those temporary aggregates returned from functions

The latest C23 draft § 6.2.4 Storage durations of objects

> 8 A non-lvalue expression with structure or union type, where the structure or union contains a
member with array type (including, recursively, members of all contained structures and unions)
refers to an object with automatic storage duration and temporary lifetime. 39) Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends
when the evaluation of the containing full expression ends. Any attempt to modify an object with
temporary lifetime results in undefined behavior. An object with temporary lifetime behaves as if it
were declared with the type of its value for the purposes of effective type. Such an object need not
have a unique address.

Speaking more with @AaronBallman , Aaron found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1285.htm introduced the current wording into c11. https://wiki.sei.cmu.edu/confluence/display/c/EXP35-C.+Do+not+modify+objects+with+temporary+lifetime talks more about this. https://web.archive.org/web/20231010182056/https://yarchive.net/comp/struct_return.html talks more about this in regards to arrays within the returned struct.

Also, this is quite painful for 32b ARM due to ARM AAPCS32 § 6.4 Result Return

> A Composite Type larger than 4 bytes, or whose size cannot be determined statically by both caller and callee, is stored in memory at an address passed as an extra argument when the function was called (Parameter Passing (base PCS), Rule A.4). The memory to be used for the result may be modified at any point during the function call.

(I'll tag the ARM backend folks for the CC, though this isn't a backend specific bug)

So I think if the type contains an array, we need to have the lifetime be emitted after the full expression ends. CodeGenFunction::pushFullExprCleanup and CodeGenFunction::pushCleanupAfterFullExpr look useful, though I'm not sure that's precisely what they do (or what the difference is between them yet). Otherwise I guess these temporaries can just be cleaned up after the function call they are passed to.

[llvmbot]

@llvm/issue-subscribers-backend-arm

Author: Nick Desaulniers (nickdesaulniers)

forked from #43598 (and https://reviews.llvm.org/D74094) which is C++ specific; there are different rules for C FWICT.

struct foo {
    long long x;
};

struct foo foo_factory (void);
struct foo div (struct foo, struct foo);

long long bar (void) {
    return div(div(foo_factory(), foo_factory()), div(foo_factory(), foo_factory())).x;
}

We should emit lifetime intrinsics for those temporary aggregates returned from functions

The latest C23 draft § 6.2.4 Storage durations of objects

> 8 A non-lvalue expression with structure or union type, where the structure or union contains a
member with array type (including, recursively, members of all contained structures and unions)
refers to an object with automatic storage duration and temporary lifetime. 39) Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends
when the evaluation of the containing full expression ends. Any attempt to modify an object with
temporary lifetime results in undefined behavior. An object with temporary lifetime behaves as if it
were declared with the type of its value for the purposes of effective type. Such an object need not
have a unique address.

Speaking more with @AaronBallman , Aaron found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1285.htm introduced the current wording into c11. https://wiki.sei.cmu.edu/confluence/display/c/EXP35-C.+Do+not+modify+objects+with+temporary+lifetime talks more about this. https://web.archive.org/web/20231010182056/https://yarchive.net/comp/struct_return.html talks more about this in regards to arrays within the returned struct.

Also, this is quite painful for 32b ARM due to ARM AAPCS32 § 6.4 Result Return

> A Composite Type larger than 4 bytes, or whose size cannot be determined statically by both caller and callee, is stored in memory at an address passed as an extra argument when the function was called (Parameter Passing (base PCS), Rule A.4). The memory to be used for the result may be modified at any point during the function call.

(I'll tag the ARM backend folks for the CC, though this isn't a backend specific bug)

So I think if the type contains an array, we need to have the lifetime be emitted after the full expression ends. CodeGenFunction::pushFullExprCleanup and CodeGenFunction::pushCleanupAfterFullExpr look useful, though I'm not sure that's precisely what they do (or what the difference is between them yet). Otherwise I guess these temporaries can just be cleaned up after the function call they are passed to.

[nickdesaulniers]

pushCleanupAfterFullExpr<CallLifetimeEnd> looks like it might be reusable here. (or pushFullExprCleanup or CallLifetimeEnd )