r352930 - [WebAssembly] Add an import_field function attribute
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 1 14:30:48 PST 2019
On Fri, Feb 1, 2019 at 5:25 PM Dan Gohman via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Author: djg
> Date: Fri Feb 1 14:25:23 2019
> New Revision: 352930
>
> URL: http://llvm.org/viewvc/llvm-project?rev=352930&view=rev
> Log:
> [WebAssembly] Add an import_field function attribute
>
> This is similar to import_module, but sets the import field name
> instead.
>
> By default, the import field name is the same as the C/asm/.o symbol
> name. However, there are situations where it's useful to have it be
> different. For example, suppose I have a wasm API with a module named
> "pwsix" and a field named "read". There's no risk of namespace
> collisions with user code at the wasm level because the generic name
> "read" is qualified by the module name "pwsix". However in the C/asm/.o
> namespaces, the module name is not used, so if I have a global function
> named "read", it is intruding on the user's namespace.
>
> With the import_field module, I can declare my function (in libc) to be
> "__read", and then set the wasm import module to be "pwsix" and the wasm
> import field to be "read". So at the C/asm/.o levels, my symbol is
> outside the user namespace.
>
> Differential Revision: https://reviews.llvm.org/D57602
Btw, this review never went to cfe-commits, but it should have. :-)
> Added:
> cfe/trunk/test/CodeGen/wasm-import-name.c
> - copied, changed from r352781, cfe/trunk/test/CodeGen/wasm-import-module.c
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=352930&r1=352929&r2=352930&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Fri Feb 1 14:25:23 2019
> @@ -1514,11 +1514,19 @@ def AMDGPUNumVGPR : InheritableAttr {
> def WebAssemblyImportModule : InheritableAttr,
> TargetSpecificAttr<TargetWebAssembly> {
> let Spellings = [Clang<"import_module">];
> - let Args = [StringArgument<"ImportModuleName">];
> + let Args = [StringArgument<"ImportModule">];
> let Documentation = [WebAssemblyImportModuleDocs];
> let Subjects = SubjectList<[Function], ErrorDiag>;
> }
>
> +def WebAssemblyImportName : InheritableAttr,
> + TargetSpecificAttr<TargetWebAssembly> {
> + let Spellings = [Clang<"import_name">];
> + let Args = [StringArgument<"ImportName">];
> + let Documentation = [WebAssemblyImportNameDocs];
> + let Subjects = SubjectList<[Function], ErrorDiag>;
> +}
> +
> def NoSplitStack : InheritableAttr {
> let Spellings = [GCC<"no_split_stack">];
> let Subjects = SubjectList<[Function], ErrorDiag>;
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=352930&r1=352929&r2=352930&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Fri Feb 1 14:25:23 2019
> @@ -3730,6 +3730,23 @@ reuqest a specific module name be used i
> }];
> }
>
> +def WebAssemblyImportNameDocs : Documentation {
> + let Category = DocCatFunction;
> + let Content = [{
> +Clang supports the ``__attribute__((import_name(<name>)))``
> +attribute for the WebAssembly target. This attribute may be attached to a
> +function declaration, where it modifies how the symbol is to be imported
> +within the WebAssembly linking environment.
> +
> +WebAssembly imports use a two-level namespace scheme, consisting of a module
> +name, which typically identifies a module from which to import, and a field
> +name, which typically identifies a field from that module to import. By
> +default, field names for C/C++ symbols are the same as their C/C++ symbol
> +names. This attribute can be used to override the default behavior, and
> +reuqest a specific field name be used instead.
> + }];
> +}
> +
> def ArtificialDocs : Documentation {
> let Category = DocCatFunction;
> let Content = [{
>
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=352930&r1=352929&r2=352930&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Fri Feb 1 14:25:23 2019
> @@ -765,7 +765,13 @@ public:
> if (const auto *Attr = FD->getAttr<WebAssemblyImportModuleAttr>()) {
> llvm::Function *Fn = cast<llvm::Function>(GV);
> llvm::AttrBuilder B;
> - B.addAttribute("wasm-import-module", Attr->getImportModuleName());
> + B.addAttribute("wasm-import-module", Attr->getImportModule());
> + Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
> + }
> + if (const auto *Attr = FD->getAttr<WebAssemblyImportNameAttr>()) {
> + llvm::Function *Fn = cast<llvm::Function>(GV);
> + llvm::AttrBuilder B;
> + B.addAttribute("wasm-import-name", Attr->getImportName());
> Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
> }
> }
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=352930&r1=352929&r2=352930&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Feb 1 14:25:23 2019
> @@ -5732,6 +5732,29 @@ static void handleWebAssemblyImportModul
> AL.getAttributeSpellingListIndex()));
> }
>
> +static void handleWebAssemblyImportNameAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
> + if (!isFunctionOrMethod(D)) {
> + S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
> + << "'import_name'" << ExpectedFunction;
> + return;
> + }
This code is redundant and can be removed.
> +
> + auto *FD = cast<FunctionDecl>(D);
> + if (FD->isThisDeclarationADefinition()) {
> + S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
> + return;
> + }
This diagnostic does not make sense to me -- what does this have to do
with aliases?
> +
> + StringRef Str;
> + SourceLocation ArgLoc;
> + if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &ArgLoc))
> + return;
> +
> + FD->addAttr(::new (S.Context) WebAssemblyImportNameAttr(
> + AL.getRange(), S.Context, Str,
> + AL.getAttributeSpellingListIndex()));
> +}
> +
> static void handleRISCVInterruptAttr(Sema &S, Decl *D,
> const ParsedAttr &AL) {
> // Warn about repeated attributes.
> @@ -6489,6 +6512,9 @@ static void ProcessDeclAttribute(Sema &S
> case ParsedAttr::AT_WebAssemblyImportModule:
> handleWebAssemblyImportModuleAttr(S, D, AL);
> break;
> + case ParsedAttr::AT_WebAssemblyImportName:
> + handleWebAssemblyImportNameAttr(S, D, AL);
> + break;
> case ParsedAttr::AT_IBAction:
> handleSimpleAttribute<IBActionAttr>(S, D, AL);
> break;
>
> Copied: cfe/trunk/test/CodeGen/wasm-import-name.c (from r352781, cfe/trunk/test/CodeGen/wasm-import-module.c)
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/wasm-import-name.c?p2=cfe/trunk/test/CodeGen/wasm-import-name.c&p1=cfe/trunk/test/CodeGen/wasm-import-module.c&r1=352781&r2=352930&rev=352930&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/wasm-import-module.c (original)
> +++ cfe/trunk/test/CodeGen/wasm-import-name.c Fri Feb 1 14:25:23 2019
> @@ -1,6 +1,6 @@
> // RUN: %clang_cc1 -triple wasm32-unknown-unknown-wasm -emit-llvm -o - %s | FileCheck %s
>
> -void __attribute__((import_module("bar"))) foo(void);
> +void __attribute__((import_name("bar"))) foo(void);
>
> void call(void) {
> foo();
> @@ -8,4 +8,4 @@ void call(void) {
>
> // CHECK: declare void @foo() [[A:#[0-9]+]]
>
> -// CHECK: attributes [[A]] = {{{.*}} "wasm-import-module"="bar" {{.*}}}
> +// CHECK: attributes [[A]] = {{{.*}} "wasm-import-name"="bar" {{.*}}}
>
> Modified: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test?rev=352930&r1=352929&r2=352930&view=diff
> ==============================================================================
> --- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test (original)
> +++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test Fri Feb 1 14:25:23 2019
> @@ -138,6 +138,7 @@
> // CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType)
> // CHECK-NEXT: Weak (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
> // CHECK-NEXT: WeakRef (SubjectMatchRule_variable, SubjectMatchRule_function)
> +// CHECK-NEXT: WebAssemblyImportName (SubjectMatchRule_function)
> // CHECK-NEXT: WebAssemblyImportModule (SubjectMatchRule_function)
> // CHECK-NEXT: WorkGroupSizeHint (SubjectMatchRule_function)
> // CHECK-NEXT: XRayInstrument (SubjectMatchRule_function, SubjectMatchRule_objc_method)
This is missing all the Sema tests that I would have expected. You
should add tests for the situations you are diagnosing, including
applying the attribute to the wrong entity, giving it zero args, 2 or
more args, etc.
~Aaron
More information about the cfe-commits
mailing list