Skip to content

Conversation

@aheejin
Copy link
Member

@aheejin aheejin commented Oct 25, 2024

This has not been emitted since
3f34e1b.

The corresponding proposed tool-conventions change:
WebAssembly/tool-conventions#236

@llvmbot
Copy link
Member

llvmbot commented Oct 25, 2024

@llvm/pr-subscribers-backend-webassembly
@llvm/pr-subscribers-objectyaml

@llvm/pr-subscribers-llvm-binary-utilities

Author: Heejin Ahn (aheejin)

Changes

This has not been emitted since
3f34e1b.


Full diff: https://github.com/llvm/llvm-project/pull/113729.diff

9 Files Affected:

  • (removed) lld/test/wasm/Inputs/require-feature-foo.yaml (-13)
  • (removed) lld/test/wasm/target-feature-required.yaml (-90)
  • (modified) lld/test/wasm/target-feature-used.yaml (-11)
  • (modified) lld/wasm/Writer.cpp (+2-13)
  • (modified) llvm/include/llvm/BinaryFormat/Wasm.h (-1)
  • (modified) llvm/lib/Object/WasmObjectFile.cpp (-1)
  • (modified) llvm/lib/ObjectYAML/WasmYAML.cpp (-1)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp (-1)
  • (modified) llvm/test/ObjectYAML/wasm/target-features-section.yaml (-4)
diff --git a/lld/test/wasm/Inputs/require-feature-foo.yaml b/lld/test/wasm/Inputs/require-feature-foo.yaml
deleted file mode 100644
index cf42b79fa4bba2..00000000000000
--- a/lld/test/wasm/Inputs/require-feature-foo.yaml
+++ /dev/null
@@ -1,13 +0,0 @@
---- !WASM
-FileHeader:
-  Version:         0x00000001
-Sections:
-  - Type:            CUSTOM
-    Name:            linking
-    Version:         2
-  - Type:            CUSTOM
-    Name:            target_features
-    Features:
-      - Prefix:        REQUIRED
-        Name:          "foo"
-...
diff --git a/lld/test/wasm/target-feature-required.yaml b/lld/test/wasm/target-feature-required.yaml
deleted file mode 100644
index 9300f88983ae79..00000000000000
--- a/lld/test/wasm/target-feature-required.yaml
+++ /dev/null
@@ -1,90 +0,0 @@
-# RUN: yaml2obj %s -o %t1.o
-
-# RUN: wasm-ld --no-entry --features=foo,bar,baz -o - %t1.o | obj2yaml | FileCheck %s --check-prefix SPECIFIED
-
-# RUN: not wasm-ld --no-entry --features=bar,baz,quux -o /dev/null %t1.o 2>&1 | FileCheck %s --check-prefix UNSPECIFIED
-
-# RUN: wasm-ld --no-entry --no-check-features --features=bar,baz,quux -o - %t1.o | obj2yaml | FileCheck %s --check-prefix UNSPECIFIED-NOCHECK
-
-# RUN: yaml2obj %S/Inputs/require-feature-foo.yaml -o %t.required.o
-# RUN: wasm-ld --no-entry -o - %t1.o %t.required.o | obj2yaml | FileCheck %s --check-prefix REQUIRED
-
-# RUN: yaml2obj %S/Inputs/disallow-feature-foo.yaml -o %t.disallowed.o
-# RUN: not wasm-ld --no-entry -o /dev/null %t1.o %t.disallowed.o 2>&1 | FileCheck %s --check-prefix DISALLOWED
-
-# RUN: wasm-ld --no-entry --no-check-features -o - %t1.o %t.disallowed.o | obj2yaml | FileCheck %s --check-prefix DISALLOWED-NOCHECK
-
-# RUN: yaml2obj %S/Inputs/no-feature-foo.yaml -o %t.none.o
-# RUN: not wasm-ld --no-entry -o /dev/null %t1.o %t.none.o 2>&1 | FileCheck %s --check-prefix NONE
-
-# RUN: wasm-ld --no-entry --no-check-features -o - %t1.o %t.none.o | obj2yaml | FileCheck %s --check-prefix NONE-NOCHECK
-
-# Check that the following combinations of feature linkage policies
-# give the expected results:
-#
-#     REQUIRED x REQUIRED => USED
-#     REQUIRED x DISALLOWED => Error
-#     REQUIRED x NONE => Error
-
---- !WASM
-FileHeader:
-  Version:         0x00000001
-Sections:
-  - Type:            CUSTOM
-    Name:            linking
-    Version:         2
-  - Type:            CUSTOM
-    Name:            target_features
-    Features:
-      - Prefix:        REQUIRED
-        Name:          "foo"
-...
-
-# SPECIFIED:        - Type:            CUSTOM
-# SPECIFIED:          Name:            target_features
-# SPECIFIED-NEXT:     Features:
-# SPECIFIED-NEXT:       - Prefix:          USED
-# SPECIFIED-NEXT:         Name:            bar
-# SPECIFIED-NEXT:       - Prefix:          USED
-# SPECIFIED-NEXT:         Name:            baz
-# SPECIFIED-NEXT:       - Prefix:          USED
-# SPECIFIED-NEXT:         Name:            foo
-# SPECIFIED-NEXT: ...
-
-# UNSPECIFIED: Target feature 'foo' used by {{.*}}target-feature-required.yaml.tmp1.o is not allowed.{{$}}
-
-# UNSPECIFIED-NOCHECK:        - Type:            CUSTOM
-# UNSPECIFIED-NOCHECK:          Name:            target_features
-# UNSPECIFIED-NOCHECK-NEXT:     Features:
-# UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
-# UNSPECIFIED-NOCHECK-NEXT:         Name:            bar
-# UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
-# UNSPECIFIED-NOCHECK-NEXT:         Name:            baz
-# UNSPECIFIED-NOCHECK-NEXT:       - Prefix:          USED
-# UNSPECIFIED-NOCHECK-NEXT:         Name:            quux
-# UNSPECIFIED-NOCHECK-NEXT: ...
-
-# REQUIRED:        - Type:            CUSTOM
-# REQUIRED:          Name:            target_features
-# REQUIRED-NEXT:     Features:
-# REQUIRED-NEXT:       - Prefix:          USED
-# REQUIRED-NEXT:         Name:            foo
-# REQUIRED-NEXT: ...
-
-# DISALLOWED: Target feature 'foo' used in {{.*}}target-feature-required.yaml.tmp1.o is disallowed by {{.*}}target-feature-required.yaml.tmp.disallowed.o. Use --no-check-features to suppress.{{$}}
-
-# DISALLOWED-NOCHECK:        - Type:            CUSTOM
-# DISALLOWED-NOCHECK:          Name:            target_features
-# DISALLOWED-NOCHECK-NEXT:     Features:
-# DISALLOWED-NOCHECK-NEXT:       - Prefix:          USED
-# DISALLOWED-NOCHECK-NEXT:         Name:            foo
-# DISALLOWED-NOCHECK-NEXT: ...
-
-# NONE: Missing target feature 'foo' in {{.*}}target-feature-required.yaml.tmp.none.o, required by {{.*}}target-feature-required.yaml.tmp1.o. Use --no-check-features to suppress.{{$}}
-
-# NONE-NOCHECK:        - Type:            CUSTOM
-# NONE-NOCHECK:          Name:            target_features
-# NONE-NOCHECK-NEXT:     Features:
-# NONE-NOCHECK-NEXT:       - Prefix:          USED
-# NONE-NOCHECK-NEXT:         Name:            foo
-# NONE-NOCHECK-NEXT: ...
diff --git a/lld/test/wasm/target-feature-used.yaml b/lld/test/wasm/target-feature-used.yaml
index 5b3d47fc6ae3ad..d60f88dde664bb 100644
--- a/lld/test/wasm/target-feature-used.yaml
+++ b/lld/test/wasm/target-feature-used.yaml
@@ -9,9 +9,6 @@
 # RUN: yaml2obj %S/Inputs/use-feature-foo.yaml -o %t.used.o
 # RUN: wasm-ld --no-entry -o - %t1.o %t.used.o | obj2yaml | FileCheck %s --check-prefix USED
 
-# RUN: yaml2obj %S/Inputs/require-feature-foo.yaml -o %t.required.o
-# RUN: wasm-ld --no-entry -o - %t1.o %t.required.o | obj2yaml | FileCheck %s --check-prefix REQUIRED
-
 # RUN: yaml2obj %S/Inputs/disallow-feature-foo.yaml -o %t.disallowed.o
 # RUN: not wasm-ld --no-entry -o /dev/null %t1.o %t.disallowed.o 2>&1 | FileCheck %s --check-prefix DISALLOWED
 
@@ -24,7 +21,6 @@
 # give the expected results:
 #
 #     USED x USED => USED
-#     USED x REQUIRED => USED
 #     USED x DISALLOWED => Error
 #     USED x NONE => USED
 
@@ -73,13 +69,6 @@ Sections:
 # USED-NEXT:         Name:            foo
 # USED-NEXT: ...
 
-# REQUIRED:        - Type:            CUSTOM
-# REQUIRED:          Name:            target_features
-# REQUIRED-NEXT:     Features:
-# REQUIRED-NEXT:       - Prefix:          USED
-# REQUIRED-NEXT:         Name:            foo
-# REQUIRED-NEXT: ...
-
 # DISALLOWED: Target feature 'foo' used in {{.*}}target-feature-used.yaml.tmp1.o is disallowed by {{.*}}target-feature-used.yaml.tmp.disallowed.o. Use --no-check-features to suppress.{{$}}
 
 # DISALLOWED-NOCHECK:        - Type:            CUSTOM
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 77cddfc34389c3..aeac1a51824f51 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -572,7 +572,6 @@ void Writer::finalizeSections() {
 
 void Writer::populateTargetFeatures() {
   StringMap<std::string> used;
-  StringMap<std::string> required;
   StringMap<std::string> disallowed;
   SmallSet<std::string, 8> &allowed = out.targetFeaturesSec->features;
   bool tlsUsed = false;
@@ -599,7 +598,7 @@ void Writer::populateTargetFeatures() {
       goto done;
   }
 
-  // Find the sets of used, required, and disallowed features
+  // Find the sets of used and disallowed features
   for (ObjFile *file : ctx.objectFiles) {
     StringRef fileName(file->getName());
     for (auto &feature : file->getWasmObj()->getTargetFeatures()) {
@@ -607,10 +606,6 @@ void Writer::populateTargetFeatures() {
       case WASM_FEATURE_PREFIX_USED:
         used.insert({feature.Name, std::string(fileName)});
         break;
-      case WASM_FEATURE_PREFIX_REQUIRED:
-        used.insert({feature.Name, std::string(fileName)});
-        required.insert({feature.Name, std::string(fileName)});
-        break;
       case WASM_FEATURE_PREFIX_DISALLOWED:
         disallowed.insert({feature.Name, std::string(fileName)});
         break;
@@ -662,7 +657,7 @@ void Writer::populateTargetFeatures() {
     }
   }
 
-  // Validate the required and disallowed constraints for each file
+  // Validate the disallowed constraints for each file
   for (ObjFile *file : ctx.objectFiles) {
     StringRef fileName(file->getName());
     SmallSet<std::string, 8> objectFeatures;
@@ -675,12 +670,6 @@ void Writer::populateTargetFeatures() {
               fileName + " is disallowed by " + disallowed[feature.Name] +
               ". Use --no-check-features to suppress.");
     }
-    for (const auto &feature : required.keys()) {
-      if (!objectFeatures.count(std::string(feature)))
-        error(Twine("Missing target feature '") + feature + "' in " + fileName +
-              ", required by " + required[feature] +
-              ". Use --no-check-features to suppress.");
-    }
   }
 
 done:
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index 9b21d6d65c2a8e..759e4321250912 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -175,7 +175,6 @@ const unsigned WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND = 0x3;
 // Feature policy prefixes used in the custom "target_features" section
 enum : uint8_t {
   WASM_FEATURE_PREFIX_USED = '+',
-  WASM_FEATURE_PREFIX_REQUIRED = '=',
   WASM_FEATURE_PREFIX_DISALLOWED = '-',
 };
 
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index f244099d664dab..35f152d5ece8ba 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -999,7 +999,6 @@ Error WasmObjectFile::parseTargetFeaturesSection(ReadContext &Ctx) {
     Feature.Prefix = readUint8(Ctx);
     switch (Feature.Prefix) {
     case wasm::WASM_FEATURE_PREFIX_USED:
-    case wasm::WASM_FEATURE_PREFIX_REQUIRED:
     case wasm::WASM_FEATURE_PREFIX_DISALLOWED:
       break;
     default:
diff --git a/llvm/lib/ObjectYAML/WasmYAML.cpp b/llvm/lib/ObjectYAML/WasmYAML.cpp
index 7ad338f65706d5..0636e19e05353d 100644
--- a/llvm/lib/ObjectYAML/WasmYAML.cpp
+++ b/llvm/lib/ObjectYAML/WasmYAML.cpp
@@ -342,7 +342,6 @@ void ScalarEnumerationTraits<WasmYAML::FeaturePolicyPrefix>::enumeration(
     IO &IO, WasmYAML::FeaturePolicyPrefix &Kind) {
 #define ECase(X) IO.enumCase(Kind, #X, wasm::WASM_FEATURE_PREFIX_##X);
   ECase(USED);
-  ECase(REQUIRED);
   ECase(DISALLOWED);
 #undef ECase
 }
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
index 14c0eaac17daaa..2be2c01d795ab5 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -518,7 +518,6 @@ void WebAssemblyAsmPrinter::EmitTargetFeatures(Module &M) {
 
     // Silently ignore invalid metadata
     if (Entry.Prefix != wasm::WASM_FEATURE_PREFIX_USED &&
-        Entry.Prefix != wasm::WASM_FEATURE_PREFIX_REQUIRED &&
         Entry.Prefix != wasm::WASM_FEATURE_PREFIX_DISALLOWED)
       return;
 
diff --git a/llvm/test/ObjectYAML/wasm/target-features-section.yaml b/llvm/test/ObjectYAML/wasm/target-features-section.yaml
index 22d2645095d771..0f361da2a63b0f 100644
--- a/llvm/test/ObjectYAML/wasm/target-features-section.yaml
+++ b/llvm/test/ObjectYAML/wasm/target-features-section.yaml
@@ -8,8 +8,6 @@ Sections:
     Features:
       - Prefix:        USED
         Name:          "foo"
-      - Prefix:        REQUIRED
-        Name:          "bar"
       - Prefix:        DISALLOWED
         Name:          ""
 ...
@@ -19,7 +17,5 @@ Sections:
 # CHECK-NEXT:       Features:
 # CHECK-NEXT:         - Prefix:        USED
 # CHECK-NEXT:           Name:          foo
-# CHECK-NEXT:         - Prefix:        REQUIRED
-# CHECK-NEXT:           Name:          bar
 # CHECK-NEXT:         - Prefix:        DISALLOWED
 # CHECK-NEXT:           Name:          ''

@aheejin aheejin changed the title [WebAssembly] Remove WASM_FEATURE_PREFIX_REQUIRED [WebAssembly] Remove WASM_FEATURE_PREFIX_REQUIRED (NFC) Oct 25, 2024
@tlively
Copy link
Collaborator

tlively commented Oct 25, 2024

We should wait to see if there are objections on the tool-conventions change first. It might also be good to link to the tool-conventions PR from the description here.

@aheejin
Copy link
Member Author

aheejin commented Oct 25, 2024

It might also be good to link to the tool-conventions PR from the description here.

Done

@aheejin
Copy link
Member Author

aheejin commented Nov 5, 2024

The Ci failures are irrelevant. Merging.

@aheejin aheejin merged commit be64ca9 into llvm:main Nov 5, 2024
12 of 14 checks passed
@aheejin aheejin deleted the remove_required branch November 5, 2024 00:13
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
This has not been emitted since

llvm@3f34e1b.

The corresponding proposed tool-conventions change:
WebAssembly/tool-conventions#236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants