[PATCH] D126278: [RISCV] Replace untested code with assert

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck created this revision.
frasercrmck added reviewers: asb, jrtc27, reames, craig.topper, rogfer01, StephenFan, kito-cheng.
Herald added subscribers: sunshaoce, VincentWu, luke957, vkmr, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya, arichardson.
Herald added a project: All.
frasercrmck requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

We found untested code where negative frame indices were ostensibly
handled despite it being in a block guarded by !MFI.isFixedObjectIndex.

While the implementation of MachineFrameInfo::isFixedObjectIndex
suggests this is possible (i.e., if a frame index was negative than the
number of fixed objects), I couldn't find any test in tree -- for any
target -- where a negative frame index wasn't also a fixed object
offset. I couldn't find a way of creating such a object with the
public MachineFrameInfo creation APIs. Even
MachineFrameInfo::getObjectIndexBegin starts counting at the negative
number of fixed objects, so such frame indices wouldn't be covered by
loops using the provided begin/end methods.

Given all this, an assert that any object encountered in the block is
non-negative seems reasonable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126278

Files:
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp


Index: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -757,8 +757,7 @@
     // objects to the required alignment.
     if (MFI.getStackID(FI) == TargetStackID::Default) {
       Offset += StackOffset::getFixed(MFI.getStackSize());
-      if (FI < 0)
-        Offset += StackOffset::getFixed(RVFI->getLibCallStackSize());
+      assert(FI >= 0 && "Unhandled negative frame index");
     } else if (MFI.getStackID(FI) == TargetStackID::ScalableVector) {
       // Ensure the base of the RVV stack is correctly aligned: add on the
       // alignment padding.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126278.431600.patch
Type: text/x-patch
Size: 724 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220524/c4551f0f/attachment.bin>