[PATCH] D61694: Boilerplate for producing XCOFF object files from the PowerPC backend.
Stefan Pintilie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 27 10:38:04 PDT 2019
stefanp added a comment.
I'm not super familiar with the MC layer but overall this looks good to me.
I have a couple of minor comments.
Also, it seems that there is a part of that patch that is missing. The test added: `aix-xcoff-basic.ll` causes an assert with top of trunk.
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.h:41
+ explicit PPCXCOFFMCAsmInfo();
Why don't we pass in the `is64Bit` flag and the `Triple` into the function?
I know that we don't intend to support 64 bit from the start but I assume we will want to support it at one point right? We can check the flag at the start and if it is 64 bit we can emit a fatal_error or unreachable as you have done in other places.
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:87
+ MAI = new PPCXCOFFMCAsmInfo();
So, previously the default was ELF now it is XCOFF.
This probably won't make much of a difference but ideally we should be spelling out what we support.
MAI = new PPCMCAsmInfoDarwin(isPPC64, TheTriple);
else if (TheTriple.isOSBinFormatELF())
MAI = new PPCELFMCAsmInfo(isPPC64, TheTriple);
else if (TheTriple.isOSBinFormatXCOFF())
MAI = new PPCXCOFFMCAsmInfo();
llvm_unreachable("Unknown binary format.");
However, I don't know if the above is safe. Some code may rely on having ELF as default. Either way, I don't think we can leave it as XCOFF default.
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.h:59
+/// Construct a PPC XCOFF object writer.
Here too maybe we want to keep the 64 bit flag?
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the llvm-commits