Skip to content
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

[Object][Wasm] Move wasm Object tests into their own directory (NFC) #81072

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Feb 8, 2024

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 8, 2024

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

Author: Derek Schuff (dschuff)

Changes

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

12 Files Affected:

  • (renamed) llvm/test/Object/Wasm/wasm-bad-data-symbol.yaml ()
  • (renamed) llvm/test/Object/Wasm/wasm-bad-metadata-version.yaml ()
  • (renamed) llvm/test/Object/Wasm/wasm-bad-reloc-type.test ()
  • (renamed) llvm/test/Object/Wasm/wasm-bad-symbol-type.test ()
  • (renamed) llvm/test/Object/Wasm/wasm-duplicate-name.test ()
  • (renamed) llvm/test/Object/Wasm/wasm-invalid-file.yaml ()
  • (renamed) llvm/test/Object/Wasm/wasm-invalid-section-order.test ()
  • (renamed) llvm/test/Object/Wasm/wasm-invalid-start.test ()
  • (renamed) llvm/test/Object/Wasm/wasm-missing-version.test ()
  • (renamed) llvm/test/Object/Wasm/wasm-obj2yaml-tables.test ()
  • (renamed) llvm/test/Object/Wasm/wasm-relocs-and-producers.yaml ()
  • (renamed) llvm/test/Object/Wasm/wasm-string-outside-section.test ()
diff --git a/llvm/test/Object/wasm-bad-data-symbol.yaml b/llvm/test/Object/Wasm/wasm-bad-data-symbol.yaml
similarity index 100%
rename from llvm/test/Object/wasm-bad-data-symbol.yaml
rename to llvm/test/Object/Wasm/wasm-bad-data-symbol.yaml
diff --git a/llvm/test/Object/wasm-bad-metadata-version.yaml b/llvm/test/Object/Wasm/wasm-bad-metadata-version.yaml
similarity index 100%
rename from llvm/test/Object/wasm-bad-metadata-version.yaml
rename to llvm/test/Object/Wasm/wasm-bad-metadata-version.yaml
diff --git a/llvm/test/Object/wasm-bad-reloc-type.test b/llvm/test/Object/Wasm/wasm-bad-reloc-type.test
similarity index 100%
rename from llvm/test/Object/wasm-bad-reloc-type.test
rename to llvm/test/Object/Wasm/wasm-bad-reloc-type.test
diff --git a/llvm/test/Object/wasm-bad-symbol-type.test b/llvm/test/Object/Wasm/wasm-bad-symbol-type.test
similarity index 100%
rename from llvm/test/Object/wasm-bad-symbol-type.test
rename to llvm/test/Object/Wasm/wasm-bad-symbol-type.test
diff --git a/llvm/test/Object/wasm-duplicate-name.test b/llvm/test/Object/Wasm/wasm-duplicate-name.test
similarity index 100%
rename from llvm/test/Object/wasm-duplicate-name.test
rename to llvm/test/Object/Wasm/wasm-duplicate-name.test
diff --git a/llvm/test/Object/wasm-invalid-file.yaml b/llvm/test/Object/Wasm/wasm-invalid-file.yaml
similarity index 100%
rename from llvm/test/Object/wasm-invalid-file.yaml
rename to llvm/test/Object/Wasm/wasm-invalid-file.yaml
diff --git a/llvm/test/Object/wasm-invalid-section-order.test b/llvm/test/Object/Wasm/wasm-invalid-section-order.test
similarity index 100%
rename from llvm/test/Object/wasm-invalid-section-order.test
rename to llvm/test/Object/Wasm/wasm-invalid-section-order.test
diff --git a/llvm/test/Object/wasm-invalid-start.test b/llvm/test/Object/Wasm/wasm-invalid-start.test
similarity index 100%
rename from llvm/test/Object/wasm-invalid-start.test
rename to llvm/test/Object/Wasm/wasm-invalid-start.test
diff --git a/llvm/test/Object/wasm-missing-version.test b/llvm/test/Object/Wasm/wasm-missing-version.test
similarity index 100%
rename from llvm/test/Object/wasm-missing-version.test
rename to llvm/test/Object/Wasm/wasm-missing-version.test
diff --git a/llvm/test/Object/wasm-obj2yaml-tables.test b/llvm/test/Object/Wasm/wasm-obj2yaml-tables.test
similarity index 100%
rename from llvm/test/Object/wasm-obj2yaml-tables.test
rename to llvm/test/Object/Wasm/wasm-obj2yaml-tables.test
diff --git a/llvm/test/Object/wasm-relocs-and-producers.yaml b/llvm/test/Object/Wasm/wasm-relocs-and-producers.yaml
similarity index 100%
rename from llvm/test/Object/wasm-relocs-and-producers.yaml
rename to llvm/test/Object/Wasm/wasm-relocs-and-producers.yaml
diff --git a/llvm/test/Object/wasm-string-outside-section.test b/llvm/test/Object/Wasm/wasm-string-outside-section.test
similarity index 100%
rename from llvm/test/Object/wasm-string-outside-section.test
rename to llvm/test/Object/Wasm/wasm-string-outside-section.test

@sbc100
Copy link
Collaborator

sbc100 commented Feb 8, 2024

This makes some logical sense but the other sub-directories here represent architectures, and not object formats. For example note that there are no COFF or ELF sub-directories here. Should we move those tests too?

@dschuff
Copy link
Member Author

dschuff commented Feb 8, 2024

True. I'm not sure I want to reorganize the whole thing but we could rename the directory from Wasm (which the object format is usually called) to WebAssembly (the architecture)😄

@sbc100
Copy link
Collaborator

sbc100 commented Feb 8, 2024

True. I'm not sure I want to reorganize the whole thing but we could rename the directory from Wasm (which the object format is usually called) to WebAssembly (the architecture)😄

I think maybe go for a full rename for ELF and COFF too? See what the other maintainers think about it?

Also, we have not yet used "Wasm" as a file or directory name in llvm. We have WebAssembly and wasm and WASM in different places. I'd like to fix that last one especially but I'm worried about such renames for case insensitive filesystem folks.

@dwblaikie
Copy link
Collaborator

I think maybe go for a full rename for ELF and COFF too? See what the other maintainers think about it?
I assume there are sufficiently interesting differences to test between AArch64 and ARM, etc. but there's no architecture variants for wasm or webassembly - Yeah, there might be different ways of naming things between the format and the architecture, but I think the distinction isn't especially important to make - but I think it's fine/suitable for it to have a separate directory (I don't too much mind what name - but I agree consistency is nice to have, and if we've mostly used WebAssembly eswhere, let's use it here).

@MaskRay
Copy link
Member

MaskRay commented Feb 9, 2024

Creating test/Object/wasm looks good to me. I'd prefer wasm/ instead of Wasm/ since wasm/ is used more common. The move will enable ninja check-llvm-object-wasm

We've moved ELF/COFF/Mach-O tests for at least llvm-readobj/llvm-objcopy/llvm-objdump.

@dschuff
Copy link
Member Author

dschuff commented Apr 10, 2024

I looked at this a little more.
Regarding Wasm vs WebAssembly: I don't think naming the directory WebAssembly is a good idea. We have several files and directories named Wasm or wasm (e.g. lib/Object/Wasm, lld/wasm) and they (almost) all refer to the binary/object format, as opposed to the architecture. Naming just this test directory WebAssembly would make things worse rather than better IMO (because regardless of whether you believe it's good that the bin format doesn't match the architecture, at least right now we are mostly consistent).

Regarding Wasm vs wasm: It looks like actually Wasm is more common than wasm (notable exceptions being lld/wasm and llvm/lib/ObjCopy/wasm). It also looks like ninja check-llvm-object-wasm works with Wasm as the directory name.

Therefore for this PR I'm going to suggest keeping the name as Wasm. Fixing some wasm and WASM directories to match can be a followup PR.

@dschuff dschuff merged commit 0d96422 into llvm:main Apr 11, 2024
4 checks passed
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.

None yet

5 participants