[PATCH] D92174: [VE] Optimize emitSPAdjustment function

Kazushi Marukawa via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 27 05:15:25 PST 2020

kaz7 added inline comments.

Comment at: llvm/lib/Target/VE/VEFrameLowering.cpp:232-243
+  } else if (isInt<7>(NumBytes)) {
+    // adds.l %s11, NumBytes at lo, %s11
     BuildMI(MBB, MBBI, DL, TII.get(VE::ADDSLri), VE::SX11)
-    return;
+  } else if (isInt<32>(NumBytes)) {
+    // lea %s11, NumBytes at lo(, %s11)
simoll wrote:
> kaz7 wrote:
> > simoll wrote:
> > > Consider factoring this into a separate "`LoadImm`" function.
> > This is "addImmediate" to SP and need to specify clobber register also.  I think it is not general instructions as you expected.
> 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.
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.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list