-
Notifications
You must be signed in to change notification settings - Fork 15k
[WebAssembly] Enable musttail only when tail-call is enabled #163618
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -374,6 +374,8 @@ def err_ppc_impossible_musttail: Error< | |
| >; | ||
| def err_aix_musttail_unsupported: Error< | ||
| "'musttail' attribute is not supported on AIX">; | ||
| def err_wasm_musttail_unsupported: Error< | ||
| "'musttail' attribute is not supported on this target without tail-call feature">; | ||
|
Comment on lines
+377
to
+378
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You shouldn't need to add a new diagnostic, we have automatic checking for whether the attribute is supported by the target (that's the code in Attr.td) which will diagnose the attribute as being unknown. |
||
|
|
||
| // Source manager | ||
| def err_cannot_open_file : Error<"cannot open file '%0': %1">, DefaultFatal; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -695,7 +695,6 @@ bool Sema::checkMustTailAttr(const Stmt *St, const Attr &MTA) { | |
|
|
||
| const Expr *E = cast<ReturnStmt>(St)->getRetValue(); | ||
| const auto *CE = dyn_cast_or_null<CallExpr>(IgnoreParenImplicitAsWritten(E)); | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can back out this change (it's the only change to the file). |
||
| if (!CE) { | ||
| Diag(St->getBeginLoc(), diag::err_musttail_needs_call) << &MTA; | ||
| return false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -672,6 +672,12 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A, | |
| !(A.existsInTarget(S.Context.getTargetInfo()) || | ||
| (S.Context.getLangOpts().SYCLIsDevice && Aux && | ||
| A.existsInTarget(*Aux)))) { | ||
| // Special case: musttail on WebAssembly without tail-call feature | ||
| if (A.getKind() == ParsedAttr::AT_MustTail && | ||
| !S.Context.getTargetInfo().hasMustTail()) { | ||
| S.Diag(A.getLoc(), diag::err_wasm_musttail_unsupported); | ||
| return nullptr; | ||
| } | ||
|
Comment on lines
+675
to
+680
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You shouldn't need to add this code, either |
||
| if (A.isRegularKeywordAttribute()) { | ||
| S.Diag(A.getLoc(), diag::err_keyword_not_supported_on_target) | ||
| << A << A.getRange(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -target-feature +tail-call -o /dev/null -emit-llvm -verify=tail | ||
| // RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -o /dev/null -emit-llvm -verify=notail | ||
|
|
||
| int foo(int x) { | ||
| return x; | ||
| } | ||
|
|
||
| #if __has_attribute(musttail) | ||
| // tail-warning@+1 {{HAS IT}} | ||
| #warning HAS IT | ||
| #else | ||
| // notail-warning@+1 {{DOES NOT HAVE}} | ||
| #warning DOES NOT HAVE | ||
| #endif | ||
|
|
||
| int bar(int x) | ||
| { | ||
| // notail-error@+1 {{'musttail' attribute is not supported on this target without tail-call feature}} | ||
| [[clang::musttail]] return foo(1); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is close to what I was thinking, but I was going for something more like this:
(so no need to list the architectures)