[PATCH] D125964: [RISCV] Fix logic for determining RVV stack padding

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

LGTM, but with a requested followup/clarification in inline comment.



================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:948
+  const TargetRegisterInfo *TRI = STI.getRegisterInfo();
+  if (RVVStackSize && (!hasFP(MF) || TRI->hasStackRealignment(MF)) &&
+      Size % 8 != 0) {
----------------
This has a very uncomfortable interlock with getFrameIndexReference's decision making.  Is there a way that we can arrange to *assert* that the decision made here about which register we'll offset from is the same as that code?

One idea: Maybe make RVVPadding an optional?  So that we can distringuish between "no padding needed", and "we didn't think this was used"?

Looking at the frame index compute code, one case that worries me is the realign for non-fixed object with hasBP path.  That seems to not use RVVPadding, and I can't tell if that's correct (e.g. this new code is slightly conservative) or a bug (e.g. this new code is correct, and that needs to be using padding.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125964