[PATCH] D134599: [RISCV] Readjusting the framestack for Zcmp

[Yeting Kuo via Phabricator via llvm-commits]

fakepaper56 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:260
+
+void reallocPushStackFream(MachineFunction &MF) {
+  auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
----------------
KYG wrote:
> VincentWu wrote:
> > KYG wrote:
> > > reallocPushStackFream -> reallocPushStackFrame ?
> > > Is it necessary to setObjectOffset for non-pushed registers? I though their offset has already been set in 
> > > PEI::calculateFrameObjectOffsets (correct me if I'm wrong).
> > IMHO, I think it might be necessary in some cases.
> > Please refer to the `callee_with_irq` in `llvm/test/CodeGen/RISCV/push-pop-popret.ll`.
> > It will generate following code in `RV32I`
> > ```
> > addi    sp, sp, -144
> > sw      ra, 140(sp)                     # 4-byte Folded Spill
> > sw      t0, 136(sp)                     # 4-byte Folded Spill
> > sw      t1, 132(sp)                     # 4-byte Folded Spill
> > sw      t2, 128(sp)                     # 4-byte Folded Spill
> > sw      s0, 124(sp)                     # 4-byte Folded Spill
> > sw      s1, 120(sp)                     # 4-byte Folded Spill
> > sw      a0, 116(sp)                     # 4-byte Folded Spill
> > sw      a1, 112(sp)                     # 4-byte Folded Spill
> > sw      a2, 108(sp)                     # 4-byte Folded Spill
> > sw      a3, 104(sp)                     # 4-byte Folded Spill
> > sw      a4, 100(sp)                     # 4-byte Folded Spill
> > sw      a5, 96(sp)                      # 4-byte Folded Spill
> > sw      a6, 92(sp)                      # 4-byte Folded Spill
> > sw      a7, 88(sp)                      # 4-byte Folded Spill
> > sw      s2, 84(sp)                      # 4-byte Folded Spill
> > sw      s3, 80(sp)                      # 4-byte Folded Spill
> > sw      s4, 76(sp)                      # 4-byte Folded Spill
> > sw      s5, 72(sp)                      # 4-byte Folded Spill
> > sw      s6, 68(sp)                      # 4-byte Folded Spill
> > sw      s7, 64(sp)                      # 4-byte Folded Spill
> > sw      s8, 60(sp)                      # 4-byte Folded Spill
> > sw      s9, 56(sp)                      # 4-byte Folded Spill
> > sw      s10, 52(sp)                     # 4-byte Folded Spill
> > sw      s11, 48(sp)                     # 4-byte Folded Spill
> > sw      t3, 44(sp)                      # 4-byte Folded Spill
> > sw      t4, 40(sp)                      # 4-byte Folded Spill
> > sw      t5, 36(sp)                      # 4-byte Folded Spill
> > sw      t6, 32(sp)
> > ```
> > 
> > if we don't realloc non-pushed registers in `RV32IZcmp`, it will generate code like
> > ```
> > cm.push {ra, s0-s11}, -112
> > addi    sp, sp, -32
> > sw      t0, 136(sp)                     # 4-byte Folded Spill
> > sw      t1, 132(sp)                     # 4-byte Folded Spill
> > sw      t2, 128(sp)                     # 4-byte Folded Spill
> > sw      a0, 116(sp)                     # 4-byte Folded Spill
> > sw      a1, 112(sp)                     # 4-byte Folded Spill
> > sw      a2, 108(sp)                     # 4-byte Folded Spill
> > sw      a3, 104(sp)                     # 4-byte Folded Spill
> > sw      a4, 100(sp)                     # 4-byte Folded Spill
> > sw      a5, 96(sp)                      # 4-byte Folded Spill
> > sw      a6, 92(sp)                      # 4-byte Folded Spill
> > sw      a7, 88(sp)                      # 4-byte Folded Spill
> > sw      t3, 44(sp)                      # 4-byte Folded Spill
> > sw      t4, 40(sp)                      # 4-byte Folded Spill
> > sw      t5, 36(sp)                      # 4-byte Folded Spill
> > sw      t6, 32(sp)                      # 4-byte Folded Spill
> > ```
> > 
> > meanwhile, accronding to Operation of `cm.push` defiend in spec
> > {F27245522}
> > 
> > You may find both `s0` and `t0` will be saved to `sp+136`. It will cause problems.
> > So I think I have to re-alloc non-pushed registers. please correct me if I'm wrong or there is a better solution )
> Ah yes, you're right.
> However, seems like it's only necessary when push/pop change any of the offset, which happens only in irq handler.
> So maybe it's better to `reallocPushStackFrame` only for functions with interrupt attribute.
Is the problem caused by not adjust `RISCVRegisterInfo::hasReservedSpillSlot`? The frame indexes of the wrong asm looks like that it uses the {ra, s0 - s11} frames.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134599