[PATCH] D100574: [RISCV] Fix missing emergency slots for scalable stack offsets

Hsiangkai Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 19 17:42:43 PDT 2021


HsiangKai added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:858
   // would currently be missed.
   if (!isInt<11>(MFI.estimateStackSize(MF)) || RVVStackSize != 0) {
     int RegScavFI = MFI.CreateStackObject(RegInfo->getSpillSize(*RC),
----------------
frasercrmck wrote:
> HsiangKai wrote:
> > I found a bug in our downstream testing about the condition. We may need to change the condition to
> > 
> > ```
> > const auto &STI = MF.getSubtarget<RISCVSubtarget>();
> > ...
> > if (!isInt<11>(MFI.estimateStackSize(MF)) || STI.hasStdExtV())
> > ```
> > 
> > The reason is we will use VLA instructions for VLS data set. If all the data are VLS types, `RVVStackSize` will be 0. However, we still need one scavenging slot because vector load/store have no immediate offset field in the instructions.
> That makes sense. Are we able to detect that case upfront, though? I'd hope that maybe we'd be able to distinguish a known stack size of 0 vs an "unknown" stack size with VLS types. Something conceptually like an `Optional<int64_t> RVVStackSize`. Then the condition would remain the same since `None != 0`.
> 
> Anyway, maybe that should be a separate patch with its own test case(s)?
It makes sense to me. It should be a separate patch with test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100574



More information about the llvm-commits mailing list