[lld][WebAssembly] Add `--extra-features` flag to add addional features

[sbc100]

Summary

This flag acts just like the existing --features flag but instead
of replacing the set of inferred features it adds to it.

This is useful for example if you want to --export a mutable global
but none of the input of object were built with mutable global support.
In that case you can do --extra-features=mutable-globals to avoid the
linker error that would otherwise be generated in this case:

wasm-ld: error: mutable global exported but 'mutable-globals' feature not present in inputs: __stack_pointer. Use --no-check-features to suppress.

[sbc100]

• revert part

[tlively]

It seems kind of strange to have features be free form strings rather than a set of flags, but maybe that's appropriate at this stage. It's certainly more flexible.

[sbc100]

In D135831#3856019, @tlively wrote:

It seems kind of strange to have features be free form strings rather than a set of flags, but maybe that's appropriate at this stage. It's certainly more flexible.

But that direction was already taken with the existing --features= flag right? I'm not adding that concept here.

[tlively]

In D135831#3856024, @sbc100 wrote:

In D135831#3856019, @tlively wrote:

It seems kind of strange to have features be free form strings rather than a set of flags, but maybe that's appropriate at this stage. It's certainly more flexible.

But that direction was already taken with the existing --features= flag right? I'm not adding that concept here.

Ah right, I forgot about that flag.

lld/test/wasm/mutable-global-exports.s

 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
 #
 # Should fail without mutable globals feature enabled.
 # RUN: not wasm-ld --export-all %t.o -o %t.wasm 2>&1 | FileCheck -check-prefix=CHECK-ERR %s
 # RUN: not wasm-ld --export=foo_global %t.o -o %t.wasm 2>&1 | FileCheck -check-prefix=CHECK-ERR %s
-#
-# RUN: wasm-ld --features=mutable-globals --export=foo_global %t.o -o %t.wasm
+
+# RUN: wasm-ld --extra-features=mutable-globals --export=foo_global %t.o -o %t.wasm
 # RUN: obj2yaml %t.wasm | FileCheck %s
 
 # Explcitly check that __stack_pointer can be exported
-# RUN: wasm-ld --features=mutable-globals --export=__stack_pointer %t.o -o %t.wasm
+# RUN: wasm-ld --extra-features=mutable-globals --export=__stack_pointer %t.o -o %t.wasm
 # RUN: obj2yaml %t.wasm | FileCheck -check-prefix=CHECK-SP %s
 
-# RUN: wasm-ld --features=mutable-globals --export-all %t.o -o %t.wasm
+# RUN: wasm-ld --extra-features=mutable-globals --export-all %t.o -o %t.wasm
 # RUN: obj2yaml %t.wasm | FileCheck -check-prefix=CHECK-ALL %s
 
-
 .globl _start
 .globl foo_global
 
 .globaltype foo_global, i32
 foo_global:
 
 _start:
   .functype _start () -> ()
   end_function
 
+# Add a target feature and ensure that it is preserved when --extra-features is
+# used above.
+.section  .custom_section.target_features,"",@
+  .int8 1
+  .int8 43
+  .int8 7
+  .ascii  "atomics"
+
 # CHECK-ERR: mutable global exported but 'mutable-globals' feature not present in inputs: `foo_global`. Use --no-check-features to suppress
 
 #      CHECK:  - Type:            EXPORT
 # CHECK-NEXT:    Exports:
 # CHECK-NEXT:      - Name:            memory
 # CHECK-NEXT:        Kind:            MEMORY
 # CHECK-NEXT:        Index:           0
 # CHECK-NEXT:      - Name:            _start

 # CHECK-ALL-NEXT:        Index:           5
 # CHECK-ALL-NEXT:      - Name:            __memory_base
 # CHECK-ALL-NEXT:        Kind:            GLOBAL
 # CHECK-ALL-NEXT:        Index:           6
 # CHECK-ALL-NEXT:      - Name:            __table_base
 # CHECK-ALL-NEXT:        Kind:            GLOBAL
 # CHECK-ALL-NEXT:        Index:           7
 # CHECK-ALL-NEXT:  - Type:            CODE
+
+# CHECK-ALL:         Name:            target_features
+# CHECK-ALL-NEXT:    Features:
+# CHECK-ALL-NEXT:      - Prefix:          USED
+# CHECK-ALL-NEXT:        Name:            atomics
+# CHECK-ALL-NEXT:      - Prefix:          USED
+# CHECK-ALL-NEXT:        Name:            mutable-globals
+# CHECK-ALL-NEXT: ...
+

lld/wasm/Config.h

struct Configuration {
   llvm::StringRef thinLTOCacheDir;
 
   llvm::StringSet<> allowUndefinedSymbols;
   llvm::StringSet<> exportedSymbols;
   std::vector<llvm::StringRef> requiredExports;
   llvm::SmallVector<llvm::StringRef, 0> searchPaths;
   llvm::CachePruningPolicy thinLTOCachePolicy;
   llvm::Optional<std::vector<std::string>> features;
+  llvm::Optional<std::vector<std::string>> extraFeatures;
 
   // The following config options do not directly correspond to any
   // particular command line options.
 
   // True if we are creating position-independent code.
   bool isPic;
 
   // True if we have an MVP input that uses __indirect_function_table and which

lld/wasm/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
 
   if (auto *arg = args.getLastArg(OPT_features)) {
     config->features =
         llvm::Optional<std::vector<std::string>>(std::vector<std::string>());
     for (StringRef s : arg->getValues())
       config->features->push_back(std::string(s));
   }
 
+  if (auto *arg = args.getLastArg(OPT_extra_features)) {
+    config->extraFeatures =
+        llvm::Optional<std::vector<std::string>>(std::vector<std::string>());
+    for (StringRef s : arg->getValues())
+      config->extraFeatures->push_back(std::string(s));
+  }
+
   // Legacy --allow-undefined flag which is equivalent to
   // --unresolve-symbols=ignore + --import-undefined
   if (args.hasArg(OPT_allow_undefined)) {
     config->importUndefined = true;
     config->unresolvedSymbols = UnresolvedPolicy::Ignore;
   }
 
   if (args.hasArg(OPT_print_map))

lld/wasm/Options.td

 
 defm check_features: BB<"check-features",
     "Check feature compatibility of linked objects (default)",
     "Ignore feature compatibility of linked objects">;
 
 def features: CommaJoined<["--", "-"], "features=">,
   HelpText<"Comma-separated used features, inferred from input objects by default.">;
 
+def extra_features: CommaJoined<["--", "-"], "extra-features=">,
+  HelpText<"Comma-separated list of features to add to the default set of features inferred from input objects.">;
+
 // Aliases
 def: JoinedOrSeparate<["-"], "e">, Alias<entry>;
 def: J<"entry=">, Alias<entry>;
 def: F<"call_shared">, Alias<Bdynamic>, HelpText<"Alias for --Bdynamic">;
 def: F<"dy">, Alias<Bdynamic>, HelpText<"Alias for --Bdynamic">;
 def: F<"dn">, Alias<Bstatic>, HelpText<"Alias for --Bstatic">;
 def: F<"non_shared">, Alias<Bstatic>, HelpText<"Alias for --Bstatic">;
 def: F<"static">, Alias<Bstatic>, HelpText<"Alias for --Bstatic">;

lld/wasm/Writer.cpp

void Writer::populateTargetFeatures() {
 
   if (config->isPic) {
     // This should not be necessary because all PIC objects should
     // contain the mutable-globals feature.
     // TODO(https://bugs.llvm.org/show_bug.cgi?id=52339)
     allowed.insert("mutable-globals");
   }
 
+  if (config->extraFeatures.has_value()) {
+    auto &extraFeatures = config->extraFeatures.value();
+    allowed.insert(extraFeatures.begin(), extraFeatures.end());
+  }
+
   // Only infer used features if user did not specify features
   bool inferFeatures = !config->features.has_value();
 
   if (!inferFeatures) {
     auto &explicitFeatures = config->features.value();
     allowed.insert(explicitFeatures.begin(), explicitFeatures.end());
     if (!config->checkFeatures)
       goto done;