This is an archive of the discontinued LLVM Phabricator instance.

[lld] Allow input files from compatible architectures on EC targets.
ClosedPublic

Authored by jacek on Apr 24 2023, 12:36 PM.

Details

Diff Detail

Event Timeline

jacek created this revision.Apr 24 2023, 12:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2023, 12:36 PM
Herald added a subscriber: zzheng. · View Herald Transcript
jacek requested review of this revision.Apr 24 2023, 12:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2023, 12:36 PM
mstorsjo accepted this revision.Apr 26 2023, 12:22 AM

I think this looks reasonable

lld/COFF/SymbolTable.cpp
42

Just to check that I've understood correctly here... We don't really expect the input of an individual object file to ever be ARM64X, but only ARM64, AMD64 or ARM64EC, while ARM64X is an image level flag to indicate that it contains both ARM64 and (ARM64EC or AMD64), more or less?

(I don't mind keeping the case here just for completeness anyway though.)

This revision is now accepted and ready to land.Apr 26 2023, 12:22 AM
jacek added inline comments.Apr 26 2023, 8:34 AM
lld/COFF/SymbolTable.cpp
42

We do expect ARM64X on input as well. Individual input object file with its machine value of ARM64X are rare and weird, but they do exist and llvm-cvtres can produce them with https://reviews.llvm.org/D148517 I will update patches with tests for them. According to my testing, ARM64X objects are accepted by link.exe for all targets: ARM64, ARM64EC and ARM64X.

In case of linked images, ARM64X is not used as machine type in PE COFF header. ARM64 is used instead and CHPE metadata referenced by load config distinguishes such files from pure ARM64. From linker point of view, CHPE metadata and load config are provided by CRT libraries, but there is a number of synthetic symbols that linker will have to provide that CRT uses to fill it. (I have enough of those symbols prototyped in my tree to get basic ARM64EC working, not yet ARM64X).

jacek updated this revision to Diff 517198.Apr 26 2023, 8:55 AM
jacek edited the summary of this revision. (Show Details)

Overall still looking good, thanks!

lld/COFF/SymbolTable.cpp
42

Oh, right, thanks for clarifying!

lld/test/COFF/arm64ec.test
36

Would it be worthwhile to have a few negative tests too? E.g. to test that arm64ec output refuses to include a regular arm64 input object file, or that arm64 output refuses to include arm64ec and x86_64?

I added negative tests and pushed, thanks.