Optimize emitSPAdjustment function to generate as small as possible
instructions to adjust SP.
Details
- Reviewers
simoll k-ishizaka - Commits
- rG3bd78b7cc00d: [VE] Optimize emitSPAdjustment function
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/VE/VEFrameLowering.cpp | ||
---|---|---|
232–243 | So, you could name it emitAddImm(Reg, Imm). The reason i was asking you to consider this, is that putting this into its own function with a telling name would document what you are doing here and make the functionality reuseable. I imagine there may be other places where we need to emit code to increment a register by a constant. |
llvm/lib/Target/VE/VEFrameLowering.cpp | ||
---|---|---|
232–243 | As I said, it requires clobber register like S13 in this case. I simply don't understand what you really want. I already named like emitSPAdjustment. Should I change this name to emitAddSPImm or something? Or should I make a new function to handle only isInt<7> and isInt<32> cases? Or should I make a new function to handle all cases? If this function generates really generic instructions, I agree with you. However, this isn't IMHO. |
It's about making the code more reusable - this is not a stopper for this patch, however.
llvm/lib/Target/VE/VEFrameLowering.cpp | ||
---|---|---|
232–243 | You could put all of the cases in a function called: bool emitAddImm(Register TheReg, Imm TheValue, Register ClobberReg) That function would fail if the ClobberReg is invalid but required. It returns true if the ClobberReg is valid and had to be used to load the immediate. Else, it returns false and the ClobberReg was not necessary to load the immediate. This would be a reusable function. |
llvm/lib/Target/VE/VEFrameLowering.cpp | ||
---|---|---|
232–243 | I think that way, creating such a function, also. However, isn't it more generic and resuable that a function uses createVirtualRegister inside? I have no idea who want to use a function with clobber register this time. Anyway, I'll make such a function if I find other use of emitAddImmWithClobber. |
typo: Nothing