[RISCV][WIP] Add support for the Zcmt Extension#981
[RISCV][WIP] Add support for the Zcmt Extension#981svs-quic wants to merge 4 commits intoqualcomm:mainfrom
Conversation
lenary
left a comment
lenary
left a comment
There was a problem hiding this comment.
Some comments here, I'm not sure if any of these are a surprise but maybe they point at the next steps.
| def relax_tbljal | ||
| : Flag<["--"], "relax-tbljal">, | ||
| HelpText<"Enable conversion of call/jump instructions to Zcmt table jump instructions (cm.jt/cm.jalt)">, | ||
| Group<grp_riscv_linker>; |
There was a problem hiding this comment.
Can we have both positive and negative versions of this flag for users? That allows us to change the default in the future (this is what I did for the xqci relaxation flag)
| m_Module.getInternalInput(Module::InternalInputType::Sections), | ||
| JvtName, ResolveInfo::NoType, ResolveInfo::Define, | ||
| ResolveInfo::Global, | ||
| 0x0, // size |
There was a problem hiding this comment.
I don't know if we should be setting the size here. It might help people trying to debug things, i don't think loaders will look at this information.
| if (relaxation_pass == RELAXATION_CALL) | ||
| initTableJump(); |
There was a problem hiding this comment.
Given that initTableJump() does a full scan, can we make it explicitly a different phase, before RELAXATION_CALL? maybe RELAXATION_TABLE_JUMP?
| } | ||
|
|
||
| static int64_t getCallDisplace(RISCVLDBackend &Backend, const Relocation &R) { | ||
| int64_t S = static_cast<int64_t>(Backend.getSymbolValuePLT(R)); |
There was a problem hiding this comment.
I think we have to jump to the PLT entry if there is one (including e.g. for patching), but it would be good to confirm that this code is right.
The jump table is sort-of like a PLT, but it cannot be lazy and there's no preemption for its addresses.
| uint32_t Jalr = 0; | ||
| R->targetRef()->memcpy(&Jalr, sizeof(Jalr), 4); |
There was a problem hiding this comment.
I don't know why this side uses a local Jalr uint32 variable, when the other side uses Insn. Both could use something local or Insn?
| if (R->type() != llvm::ELF::R_RISCV_JAL && | ||
| R->type() != llvm::ELF::R_RISCV_CALL && | ||
| R->type() != llvm::ELF::R_RISCV_CALL_PLT) |
There was a problem hiding this comment.
What about R_RISCV_QC_E_CALL_PLT? :) A planned follow-up or an oversight?
| llvm::sort(Entries, [](const auto &A, const auto &B) { | ||
| return A.second.Index < B.second.Index; | ||
| }); |
There was a problem hiding this comment.
IIRC, we may want these sorted in a specific order. We should confirm with the architects if there's a performance implication.
2 participants
No description provided.