On Fri, Feb 1, 2019 at 5:25 PM Dan Gohman via cfe-commits <cfe-commits@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits