[clang] UEFI ABI fixes for X86_64 #124992
Conversation
UEFI targets for X86_64 architecture must use appropriate mangling, calling convention and integer size schemes.
|
Besides the places I have updated "isOSWindows()" with "isWindowsOrUEFI()", there are quite a few occurrences of isOSWindows() checks within Clang code. Which one of these must be included to make sure the mangling and other ABI related decisions made for X86_64 UEFI targets are done correctly? |
| return CCM_WasmMainArgcArgv; | ||
|
|
||
| if (!Triple.isOSWindows() || !Triple.isX86()) | ||
| if (!Triple.isOSWindowsOrUEFI() || !Triple.isX86()) |
There was a problem hiding this comment.
As we discussed offline, I'd really rather see all sites like this use a predicate that's specific to what matters to the call site in precise terms. This predicate is really an answer to the question rather than the question that should be asked. It presumes the answer is "Windows or UEFI targets", when the question it should be asking is something more like, "What C++ ABI name mangling regime is in use by this target?"
There was a problem hiding this comment.
To make things more complicated, consider mingw. I am totally unfamiliar to UEFI, but it seems like folks want it to inherit lots of MSVC ABI features: the mangling scheme, the vftable ABI rules, the struct layout, etc. These rules are usually conditional on isWindowsMSVCEnvironment(), so you'd need to audit those conditions.
While explicit triple checks are convenient and necessary in many cases, we generally try to abstract these details with the various CXXABI interfaces, and if you can find a way to do that, it may help with readability.
| case ParsedAttr::AT_MSABI: | ||
| CC = Context.getTargetInfo().getTriple().isOSWindows() ? CC_C : | ||
| CC_Win64; | ||
| CC = Context.getTargetInfo().getTriple().isOSWindowsOrUEFI() ? CC_C |
There was a problem hiding this comment.
This really highlights how getTargetInfo().getCallingConv() is what we ought to have. For now it can be defined just this way, but in future it might be controlled by switches within a given target like UEFI. But right now, it makes code like this and checkVAStartABI much clearer.
There was a problem hiding this comment.
I'm not sure your comment makes sense in the context of the __attribute__((msabi)) implementation. I would interpret your suggestion instead as something like getTargetInfo().getCcForMsAbiAttribute(), and the target would return the appropriate convention.
I think in general this will only matter on Win / non-Win x64 platforms where we have divergence between the Win/Posix worlds, while the are aligned in the ARM world.
3 participants
UEFI targets for X86_64 architecture must use appropriate mangling,
calling convention and integer size schemes.