Without this patch, MAttrs are not set.
Details
Diff Detail
- Repository
- rLLD LLVM Linker
Event Timeline
The MAttrs part is fine but we don't want CGOptLevel to be set based on the LTO opt level but rather on the opt level passed at compile time. See mailing list thread "[RFC] Adding function attributes to represent codegen optimization level" for more information.
Common/TargetOptionsCommandFlags.cpp | ||
---|---|---|
36 | return std::vector<std::string>(MAttrs.begin(), MAttrs.end()); |
Common/TargetOptionsCommandFlags.cpp | ||
---|---|---|
38 | Where is this MAttrs defined? |
Common/TargetOptionsCommandFlags.cpp | ||
---|---|---|
38 | But I can't find that code in this patch. Is that is another patch? |
Common/TargetOptionsCommandFlags.cpp | ||
---|---|---|
38 | I see what you are asking for. it is defined in LLVM. include/llvm/CodeGen/CommandFlags.inc:40: MAttrs("mattr", cl::CommaSeparated, |
Common/TargetOptionsCommandFlags.cpp | ||
---|---|---|
36 | Please format carefully -- you need a space between () and the following {. I'd recommend clang-format-diff if you are not using it already. | |
38 | Oh, I missed that. So MAttrs is not even in any namespace? Not sure if that is a good practice. Anyway, please add a comment saying that "MAttrs is defined in llvm/CodeGen/CommandFlags.inc." I believe you can simply return MAttrs from this function. |
Common/TargetOptionsCommandFlags.cpp | ||
---|---|---|
38 | Or, maybe the easiest way of writing this function is just this: std::vector<std::string> lld::GetMAttrs() { return ::MAttrs; } |
ELF/LTO.cpp | ||
---|---|---|
91–113 | I don't think we need this comment as we don't have one for GetCPUStr() and others. For lld, they are just opaque arguments passed from users to the LTO backend. |
COFF/LTO.cpp | ||
---|---|---|
63 | Delete the empty line? |
Delete the empty line?