[PATCH] D94465: [RISCV] Frame handling for RISC-V V extension. (2nd. version)

[Hsiangkai Wang via Phabricator via llvm-commits]

HsiangKai added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:162
+  DebugLoc DL = II->getDebugLoc();
+  int64_t NumOfVReg = Amount / 8;
+
----------------
craig.topper wrote:
> Is Amount expected to be a multiple of 8 or does the division need to take the ceiling?
Amount is expected to be a multiple of 8. If the object size is less than 8, it will be set to 8 at assignRVVStackObjectOffsets() in RISCVFrameLowering.cpp.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:167
+  Register FactorRegister = MRI.createVirtualRegister(&RISCV::GPRRegClass);
+  if (isPowerOf2_32(NumOfVReg)) {
+    uint32_t ShiftAmount = Log2_32(NumOfVReg);
----------------
craig.topper wrote:
> Is Amount and NumOfVReg always positive? Should we check or assert that NumOfVReg fits in 32 bits before using isPowerOf2_32?
Amount and NumOfVReg are always positive. I will add assertions for the condition.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94465/new/

https://reviews.llvm.org/D94465