- reserved x16-x31reg
- RV32E ABI
- calling convention support
- stack align
- add riscv32e target( --target=riscv32e )
Details
- Reviewers
asb
Diff Detail
Event Timeline
lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
42 | I wonder if we can avoid this global variable and instead use Subtarget.isEmbed() where needed. This will require passing a const RISCVSubtarget& to some functions (like CC_RISCV) but given that they are local to the file, I think that would be reasonable. |
lib/Support/Triple.cpp | ||
---|---|---|
46 | This should be on the same line as the case statement, similar to the rest of the table. | |
lib/Target/RISCV/RISCVFrameLowering.cpp | ||
27 | I think this would be more readable as /*StackAlignment=*/ STI.isEmbed() ? 4 : 16, | |
lib/Target/RISCV/RISCVFrameLowering.h | ||
24 | Did you mean to put a 'T' on the end of this variable name? |
Why do we need another triple? Can we not just use the feature flag in the few cases where the code needs to tell the difference? There's a whole lot of boilerplate here just to add the new triple.
rv32e arch and ilp32e ABI is decoupling in GCC, that's mean rv32i with ilp32e is possible, so I would suggest separate two thing.
Very true.
In embedded systems it can often make sense to use the ilp32e ABI even on CPUs with 32 registers because there are significantly fewer argument and temporary registers, making interrupt response time shorter. All the high 16 registers are callee-save, which means you can use them if required but you need to save them first and restore them after.
Hi Daliang and welcome to the LLVM community.
Many thanks for your work on RV32E support. In our discussion on the sw-dev mailing list I suggested splitting this task into the MC layer, calling convention, and codegen support. I'd recommend preparing patches in a way that does this. Each of these elements should come along with tests.
You shouldn't need to have riscv32e as a new triple. Just rely on the SubtargetFeature - this should remove a lot of code from this patch.
Thanks for your advice.
I think decouple the rv32e and ilp32e is great. I have not considered it before. I will do it later.
But, I was not sure what rv32e and ilp32e really meaans. There is still something not clearly. Is the march rv32e only means that the regs only contains x0-x15 ? And the ABI ilp32e means Stack Alignment and calling convention, etc (see in https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#rv32e-calling-convention )?
That's correct.
rv32e is just about hardware. Registers x16-x31 do not exist.
ilp32e is just about software. The function call convention and stack layout, as you say.
The link you point to should have that section renamed.
I will spilt the patch later as you said. It is the first time I have upload my patch. So, maybe I consider too little about how to show my work.I will do better next time.
Just like you and kito-cheng @kito-cheng have said. I had not added the triple:riscv32e in my work until I found clang-side needed to use that triple:riscv32e in the discussion on the sw-dev mailing list. The code in clang is like that
class RISCVABIInfo : public DefaultABIInfo { private: unsigned XLen; // Size of the integer ('x') registers in bits. static const int NumArgGPRs = 8; public: RISCVABIInfo(CodeGen::CodeGenTypes &CGT, unsigned XLen) : DefaultABIInfo(CGT), XLen(XLen) { } };
Because the RISCVABIInfo can't get the SubtargetFeatures, NumArgGPRs will always be 8 int the RISCVABIInfo even if the march=rv32e. So I have no way except for adding the triple:riscv32e. you can also see what I have changed the clang in https://reviews.llvm.org/D53293 about How I changed the NumArgGPRs by using triple:target. I was very confused how to solve this problem.Maybe a better way later.
lib/Support/Triple.cpp | ||
---|---|---|
46 | Because of the first time using this tools to upload my patch, I was not familiar with it. | |
lib/Target/RISCV/RISCVISelLowering.cpp | ||
42 | I was not sure whether passing an argument like const RISCVSubtarget& to the functions (like CC_RISCV) may change too much. |
The RISCVABIInfo constructor is called by RISCVTargetCodeGenInfo's constructor, which in turn is called by CodeGenModule::getTargetCodeGenInfo. CodeGenModule has a getTarget() which should give you access to the feature flags?
Thanks for your advice, I will check the CodeGenModule and RISCVTargetCodeGenInfo's code to solve that problem I have proposed. In that way , we can remove the triple:riscv32e. That will be great!
It was my plan to work on it further, but that won't happen. The place to start is with support for the ilp32e ABI, and then add rv32e support after that.
One thing we don't want, as I understand, is a separate triple to denote rv32e - the use of -march and -mabi should be enough to not need a separate triple.
My patch for ilp32e support is here: D70401 - I've found a little time to go over it this week, and am working on some improvements as we speak. Hopefully it will be ready to review by the end of the week.
One thing we don't want, as I understand, is a separate triple to denote rv32e - the use of -march and -mabi should be enough to not need a separate triple.
This should be on the same line as the case statement, similar to the rest of the table.