-
Notifications
You must be signed in to change notification settings - Fork 18.1k
[Clang] Forward incoming Indirect parameters across musttail calls #199351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
xroche
wants to merge
26
commits into
llvm:main
Choose a base branch
from
xroche:musttail-byval-temp-fix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+559
−8
Open
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
cf30821
[Clang] Forward incoming Indirect parameters across musttail calls
xroche a1c3fd3
[Clang][NFC] Trim comments on musttail Indirect forwarding helper
xroche 11cdbbf
[Clang] Address review on musttail Indirect forwarding
xroche 4359083
[Clang] Extend musttail Indirect forwarding to C++ trivial-copy args
xroche 9b1aa8b
[Clang][test] Mirror C test cases in musttail-indirect-arg.cpp
xroche 99dfa79
[Clang] Switch musttail Indirect to a two-phase general algorithm
xroche 88607e8
[Clang][test] Strengthen P3 and P17 to catch in-place-write regression
xroche 2dd4ce8
Merge branch 'main' into musttail-byval-temp-fix
xroche a27213a
Merge branch 'main' into musttail-byval-temp-fix
xroche d12920e
[Clang] Diagnose musttail Indirect args with no addressable source
xroche 7231b68
Merge remote-tracking branch 'origin/main' into musttail-byval-temp-fix
xroche f008bf5
[Clang][test] Make musttail-indirect-arg scratch checks SROA-robust
xroche 1248313
Merge remote-tracking branch 'origin/main' into musttail-byval-temp-fix
xroche 40a7edf
Merge branch 'main' into musttail-byval-temp-fix
xroche 6964a03
Merge branch 'main' into musttail-byval-temp-fix
xroche 3a58836
[Clang][test] Pin slot operands and swap data-flow in musttail-indire…
xroche 6f56dba
[Clang] Drop redundant freeze of forwarded musttail Indirect pointer
xroche cb7efcb
[Clang] Forward trivially-copyable by-value args beyond musttail
xroche 3a8db70
[Clang][test] Cover musttail Indirect arg with no in-memory source
xroche 2a770e5
[Clang] Exclude CUDA surface/texture types from by-value arg forwarding
xroche 5be3d32
[Clang] Exclude ObjC GC object-member records from by-value arg forwa…
xroche 9905d11
Merge remote-tracking branch 'origin/main' into musttail-byval-temp-fix
xroche 8c5c3be
[Clang] Re-gate trivial-copy arg forwarding to musttail calls
xroche beda9ba
[Clang] Accept any same-type glvalue as musttail forwarding source
xroche d6fe44f
[Clang][test] Note the unsupported musttail Indirect case is liftable
xroche c3ba863
[Clang] Restrict musttail forwarding sources to pure lvalue chains
xroche File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| // Test that Clang forwards incoming Indirect parameters across musttail calls | ||
| // instead of creating a byval-temp alloca that would dangle after the tail call | ||
| // deallocates the caller's frame. | ||
| // | ||
| // Companion to musttail-sret.cpp (commit a96c14eeb8fc): same idea, applied to | ||
| // incoming arguments rather than the sret return slot. | ||
|
|
||
| // RUN: %clang_cc1 -triple=riscv64-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
| // RUN: %clang_cc1 -triple=aarch64-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
| // RUN: %clang_cc1 -triple=loongarch64-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
| // RUN: %clang_cc1 -triple=s390x-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
|
|
||
| // A struct large enough to land on the indirect-arg path on RV64 (>2*XLEN=16 | ||
| // bytes), AArch64 (>16 bytes), LoongArch64, SystemZ. | ||
| struct Big { | ||
| unsigned long long a, b, c, d; | ||
| }; | ||
|
|
||
| // Plain forward: caller(B) musttails callee(B). The fix should emit no | ||
| // alloca for the forwarded arg; the call should forward the incoming | ||
| // parameter %a. | ||
| struct Big C1(struct Big a); | ||
| struct Big P1(struct Big a) { | ||
| __attribute__((musttail)) return C1(a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P1( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON-NOT: = alloca [32 x i8] | ||
| // COMMON: musttail call {{.*}} @C1({{.*}} %a) | ||
|
|
||
| // Two indirect args, same forwarding: each forwards its own incoming param. | ||
| struct Big C2(struct Big a, struct Big b); | ||
| struct Big P2(struct Big a, struct Big b) { | ||
| __attribute__((musttail)) return C2(a, b); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P2( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @C2({{.*}} %a, {{.*}} %b) | ||
|
|
||
| // Swapped args: caller(a, b) musttails callee(b, a). Each forwarded slot | ||
| // must resolve to the correct incoming Argument, not by position. | ||
| struct Big C3(struct Big x, struct Big y); | ||
| struct Big P3(struct Big a, struct Big b) { | ||
| __attribute__((musttail)) return C3(b, a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P3( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @C3({{.*}} %b, {{.*}} %a) | ||
|
|
||
| // Mixed direct + indirect: only the indirect arg is affected by the fix. | ||
| struct Big C4(int n, struct Big a); | ||
| struct Big P4(int n, struct Big a) { | ||
| __attribute__((musttail)) return C4(n, a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P4( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @C4({{.*}} %n, {{.*}} %a) | ||
|
|
||
| // Caller modifies the parameter before the musttail. Clang lowers the | ||
| // write through the incoming pointer, and the fix forwards the same | ||
| // pointer to the callee. No byval-temp. | ||
| struct Big C5(struct Big a); | ||
| struct Big P5(struct Big a) { | ||
| a.a += 1; | ||
| __attribute__((musttail)) return C5(a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P5( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @C5({{.*}} %a) | ||
|
|
||
| // musttail behind a branch: the forwarded pointer must remain live across | ||
| // the basic block transition. Tests that the helper does not assume the | ||
| // musttail is in the entry block. | ||
| struct Big C6(struct Big a, int cond); | ||
| struct Big P6(struct Big a, int cond) { | ||
| if (cond) | ||
| __attribute__((musttail)) return C6(a, cond); | ||
| return a; | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P6( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @C6({{.*}} %a, | ||
|
|
||
| // Same Argument forwarded to two slots: the helper engages for both. The | ||
| // noalias deduplication, if it ever fired, would force the second slot | ||
| // back to a byval-temp; but incoming Indirect params under the Linux C | ||
| // ABI are not noalias, so both slots forward %a directly. This pins the | ||
| // behavior so a future change introducing noalias on Indirect params | ||
| // would surface here. (musttail requires matching prototypes, so caller | ||
| // and callee both take two Big args.) | ||
| struct Big C7(struct Big x, struct Big y); | ||
| struct Big P7(struct Big a, struct Big b) { | ||
| __attribute__((musttail)) return C7(a, a); | ||
|
xroche marked this conversation as resolved.
|
||
| } | ||
| // COMMON-LABEL: define {{.*}} @P7( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @C7({{.*}} %a, {{.*}} %a) | ||
|
|
||
| // Negative: local source. Caller takes Big a, but musttails with a LOCAL | ||
| // Big initialized in caller's frame. The byval-temp must remain because the | ||
| // source lives in caller's frame and would dangle if forwarded. | ||
| struct Big C8(struct Big a); | ||
| struct Big P8(struct Big a) { | ||
| struct Big local = {1, 2, 3, 4}; | ||
| __attribute__((musttail)) return C8(local); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P8( | ||
| // COMMON: = alloca | ||
| // COMMON: musttail call {{.*}} @C8( | ||
|
|
||
| // Non-musttail tail call: the fix must NOT engage. Existing path emits | ||
| // the byval-temp as before, no musttail in the IR. | ||
| struct Big C9(struct Big a); | ||
| struct Big P9(struct Big a) { | ||
| return C9(a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @P9( | ||
| // COMMON-NOT: musttail | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // Test that Clang forwards incoming Indirect parameters across musttail calls | ||
| // for C++ struct-by-value arguments with trivially-copyable types. Companion to | ||
| // musttail-indirect-arg.c (the C side of the same fix) and musttail-sret.cpp | ||
| // (the SRet precedent in a96c14eeb8fc). | ||
| // | ||
| // C++ goes through a different EmitCallArg path than C: the call argument is a | ||
| // CXXConstructExpr invoking the implicit copy constructor, which would | ||
| // otherwise materialize an agg.tmp before EmitCall. For musttail with a | ||
| // trivially-copyable parameter forwarded directly, the copy is elided so the | ||
| // helper in EmitCall can forward the incoming llvm::Argument. | ||
|
|
||
| // RUN: %clang_cc1 -triple=riscv64-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
| // RUN: %clang_cc1 -triple=aarch64-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
| // RUN: %clang_cc1 -triple=loongarch64-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
| // RUN: %clang_cc1 -triple=s390x-linux-gnu %s -emit-llvm -O1 -o - | FileCheck %s --check-prefix=COMMON | ||
|
|
||
| // A trivially-copyable struct large enough to land on the indirect-arg path | ||
| // on RV64, AArch64, LoongArch64, SystemZ. | ||
| struct Big { | ||
| unsigned long long a, b, c, d; | ||
| }; | ||
|
|
||
| // Plain forward: caller(B) musttails callee(B). No agg.tmp copy, no | ||
| // byval-temp; the incoming parameter %a is forwarded directly. | ||
| struct Big C1(struct Big a); | ||
| struct Big P1(struct Big a) { | ||
| [[clang::musttail]] return C1(a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P13Big( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @_Z2C13Big({{.*}} %a) | ||
|
|
||
| // Two args, same forwarding. | ||
| struct Big C2(struct Big a, struct Big b); | ||
| struct Big P2(struct Big a, struct Big b) { | ||
| [[clang::musttail]] return C2(a, b); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P23BigS_( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @_Z2C23BigS_({{.*}} %a, {{.*}} %b) | ||
|
|
||
| // Swapped args. | ||
| struct Big C3(struct Big x, struct Big y); | ||
| struct Big P3(struct Big a, struct Big b) { | ||
| [[clang::musttail]] return C3(b, a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P33BigS_( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @_Z2C33BigS_({{.*}} %b, {{.*}} %a) | ||
|
|
||
| // Non-trivial copy constructor: the trivial-copy elision must NOT engage. | ||
| // Existing path materializes the agg.tmp (the user-defined copy ctor has | ||
| // observable behavior). | ||
| struct NonTrivial { | ||
| unsigned long long parts[4]; | ||
| NonTrivial(const NonTrivial &); | ||
| }; | ||
| NonTrivial C4(NonTrivial a); | ||
| NonTrivial P4(NonTrivial a) { | ||
| [[clang::musttail]] return C4(a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P410NonTrivial( | ||
| // The user-defined copy ctor IS called (the agg.tmp pattern still happens): | ||
| // COMMON: call {{.*}} @_ZN10NonTrivialC1ERKS_ | ||
|
|
||
| // Caller modifies the parameter before the musttail. The trivial-copy | ||
| // elision still engages because the source LValue is still the parameter; | ||
| // any mutation flowed through the incoming pointer is observed by the | ||
| // forwarded call. | ||
| struct Big C5(struct Big a); | ||
| struct Big P5(struct Big a) { | ||
| a.a += 1; | ||
| [[clang::musttail]] return C5(a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P53Big( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @_Z2C53Big({{.*}} %a) | ||
|
|
||
| // musttail behind a branch: trivial-copy elision must work across BBs. | ||
| struct Big C6(struct Big a, int cond); | ||
| struct Big P6(struct Big a, int cond) { | ||
| if (cond) | ||
| [[clang::musttail]] return C6(a, cond); | ||
| return a; | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P63Bigi( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @_Z2C63Bigi({{.*}} %a, | ||
|
|
||
| // Same Argument forwarded to two slots: both engage. Incoming Indirect | ||
| // params are not noalias under the Linux C++ ABI so the dedup in the | ||
| // helper does not fire and both slots forward %a directly. | ||
| struct Big C7(struct Big x, struct Big y); | ||
| struct Big P7(struct Big a, struct Big b) { | ||
| [[clang::musttail]] return C7(a, a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P73BigS_( | ||
| // COMMON-NOT: = alloca {{.*}}struct.Big | ||
| // COMMON: musttail call {{.*}} @_Z2C73BigS_({{.*}} %a, {{.*}} %a) | ||
|
|
||
| // Negative: source is a local, not a parameter. The trivial-copy elision | ||
| // must NOT engage; the byval-temp pattern remains. | ||
| struct Big C8(struct Big a); | ||
| struct Big P8(struct Big a) { | ||
| struct Big local = {1, 2, 3, 4}; | ||
| [[clang::musttail]] return C8(local); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P83Big( | ||
| // COMMON: = alloca | ||
| // COMMON: musttail call {{.*}} @_Z2C83Big( | ||
|
|
||
| // Non-musttail tail call: trivial-copy elision must NOT engage; the regular | ||
| // agg.tmp copy is still emitted. | ||
| struct Big C9(struct Big a); | ||
| struct Big P9(struct Big a) { | ||
| return C9(a); | ||
| } | ||
| // COMMON-LABEL: define {{.*}} @_Z2P93Big( | ||
| // COMMON-NOT: musttail |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.