I’m working on the tool llvm-mca. Simply put, this tool takes a block of assembly code and simulates it to provide some performance related analysis. I implemented a class CustomBehaviour that can be used within mca’s pipeline to enforce target specific behaviours that are not expressed well enough within the scheduling models (so mca can’t enforce those behaviours by itself). This class has been pushed upstream already and recently I tried to push an AMDGPU implementation of this class.
The implementation works as desired, except that it causes build errors when trying to build with shared libs on linux. This is because linux builds by default with the flag -fvisibility=hidden. What this means is that symbols within shared libs are not exported by default. I develop on macOS so I didn’t notice this error, but when it was brought to my attention, I reverted the commit and have been trying to come up with a solution since then. Here is the original diff post if you’re curious https://reviews.llvm.org/D104730 .
The main function that is required (and is causing the error) is getNamedOperandIdx. This function is generated by tablegen and is placed in AMDGPUGenInstrInfo.inc (<Subtarget>GenInstrInfo.inc). You provide this function with an opcode and an operand (which is defined in an enum within this same file) and it returns the index of that operand within the provided instruction (or -1 if that instruction does not have that operand). This is a necessary function for my implementation.
When building statically, or building dynamically on mac, this function is available. However, when building dynamically on linux, this function is not exported and so we get an “undefined referenced” error.
I have a few options for how to proceed, but I have no idea which would be best (or if there is a better option that I have yet to consider). So I’m hoping to get some feedback.
======================================================
Option 1:
Add the LLVM_EXTERNAL_VISIBILITY keyword in front of the getNamedOperandIdx definition. This function is defined within a .inc file, but this is generated within InstrInfoEmitter::emitOperandNameMappings (llvm/utils/TableGen/InstrInfoEmitter.cpp).
void InstrInfoEmitter::emitOperandNameMappings(raw_ostream &OS,
const CodeGenTarget &Target,
ArrayRef<const CodeGenInstruction*> NumberedInstructions) {
StringRef Namespace = Target.getInstNamespace();
std::string OpNameNS = "OpName";
// Map of operand names to their enumeration value. This will be used to
// generate the OpName enum.
std::map<std::string, unsigned> Operands;
OpNameMapTy OperandMap;
initOperandMapData(NumberedInstructions, Namespace, Operands, OperandMap);
OS << "#ifdef GET_INSTRINFO_OPERAND_ENUM\n";
OS << "#undef GET_INSTRINFO_OPERAND_ENUM\n";
OS << "namespace llvm {\n";
OS << "namespace " << Namespace << " {\n";
OS << "namespace " << OpNameNS << " {\n";
OS << "enum {\n";
for (const auto &Op : Operands)
OS << " " << Op.first << " = " << Op.second << ",\n";
OS << " OPERAND_LAST";
OS << "\n};\n";
OS << "} // end namespace OpName\n";
OS << "} // end namespace " << Namespace << "\n";
OS << "} // end namespace llvm\n";
OS << "#endif //GET_INSTRINFO_OPERAND_ENUM\n\n";
OS << "#ifdef GET_INSTRINFO_NAMED_OPS\n";
OS << "#undef GET_INSTRINFO_NAMED_OPS\n";
OS << "namespace llvm {\n";
OS << "namespace " << Namespace << " {\n";
OS << "LLVM_READONLY\n";
OS << "int16_t getNamedOperandIdx(uint16_t Opcode, uint16_t NamedIdx) {\n";
...
...
};
If we change the last shown line to
OS << "LLVM_EXTERNAL_VISIBILITY int16_t getNamedOperandIdx(uint16_t Opcode, uint16_t NamedIdx) {\n";
then the keyword is added to the function definition and the error goes away. This is a very simple fix, but I’m not sure if there would be much push-back from the community so I’ve been trying to come up with some different ideas.
======================================================
Option 2:
Right now, the target specific CustomBehaviour classes are implemented within (AMDGPU example) llvm/tools/llvm-mca/lib/AMDGPU/AMDGPUCustomBehaviour.cpp. This is nice because the CustomBehaviour class is only used by mca so it’s good to keep the code contained within the mca directory. However, this leads to a lot of difficulty in using backend functionality and symbols.
A different idea would be to have these implementations exist within the target specific lib directories (for example llvm/lib/Target/ADMGPU/). We could then follow a similar pattern as the MCSubtargetInfo or MCInstrInfo objects where we do something similar to:
// This is how the MCInstrInfo object is initialized within
// llvm-mca.cpp
std::unique_ptr<MCInstrInfo> MCII(TheTarget->createMCInstrInfo());
Since the implementation would exist within the target’s lib directory, it should be able to access all of the backend functionality (and the getNamedOperandIdx function specifically). We’d be following an existing pattern for defining and initializing the AMDGPUCustomBehaviour object, however this object is ONLY used by llvm-mca. The other objects that use this pattern are a lot more generally useful and are required by many tools. So this option would result in polluting the library and may not be the best idea.
======================================================
Option 3:
Expose the getNamedOperandIdx function / functionality through an existing MC interface. For example, it would be really nice if the MCInstrInfo object could be used to access the getNamedOperandIdx function.
std::unique_ptr<MCInstrInfo> MCII(TheTarget->createMCInstrInfo());
MCII->getNamedOperandIdx(opcode, opname);
I’m not really sure what this would require to make happen. I’m guessing that we’d need to modify the function signature for:
// This is in MCInstrInfo.h
void InitMCInstrInfo(const MCInstrDesc *D, const unsigned *NI, const char *ND,
const uint8_t *DF,
const ComplexDeprecationPredicate *CDI, unsigned NO) {
Desc = D;
InstrNameIndices = NI;
InstrNameData = ND;
DeprecatedFeatures = DF;
ComplexDeprecationInfos = CDI;
NumOpcodes = NO;
}
so that the target could pass a pointer to its getNamedOperandIdx function. Changing this function signature would most likely require changing a few other function signatures which would likely end up affecting every single target so this could be quite disruptive.
======================================================
To add some extra complexity to the problem, my AMDGPUCustomBehaviour also uses a few other backend AMDGPU functions. AMDGPU::decodeWaitcnt() and AMDGPU::getMUBUFIsBufferInv(). Both of which also cause the “undefined reference” error when building with shared libs on linux. They are both defined within llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp.
If we went with option 1 or option 3, I’d probably have to ask the AMDGPU team if we could add the LLVM_EXTERNAL_VISIBILITY keyword to those functions. That’s not ideal though. If we go with option 2, we should get access to these functions for free so that’s great.
======================================================
I’m sure there are many other options and I really hope that you can help me come up with the best option for myself and for llvm.
I hope that I’ve explained the problem and the potential solutions well enough, but feel free to ask for clarification on anything.