[PATCH] D62190: [RISCV] Allow shrink wrapping for RISC-V

[Lewis Revill via Phabricator via llvm-commits]

lewis-revill added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:230
+  MachineBasicBlock::iterator MBBI = MBB.getFirstTerminator();
+  if (MBBI == MBB.end())
+    MBBI = MBB.getLastNonDebugInstr();
----------------
shiva0217 wrote:
> apazos wrote:
> > I have been verifying the pending patches at Oz, Os, and also O2. This is helping with uncovering issues. 
> > You can run it with 'llc test.ll  -enable-shrink-wrap
> > 
> > With the latest standalone patch, it seems we have a few crashes left. Below is a bugpoint reduced test:
> > 
> > define dso_local void @test() local_unnamed_addr {
> > entry:
> >   br i1 undef, label %T.exit, label %for.body.i
> > 
> > for.body.i:                                       ; preds = %for.body.i, %entry
> >   store i32 0, i32* undef
> >   %incdec.ptr.i.i = getelementptr inbounds i32, i32* null, i32 1
> >   %cmp.i.i = icmp eq i32* undef, undef
> >   br i1 %cmp.i.i, label %T.exit.loopexit, label %for.body.i
> > 
> > T.exit.loopexit: ; preds = %for.body.i
> >   %0 = ptrtoint i32* %incdec.ptr.i.i to i32
> >   br label %T.exit
> > 
> > T.exit: ; preds = %T.exit.loopexit, %entry
> >   ret void
> > }
> > 
> > 
> It seems that shrink wrapping may choose an empty basic block to insert epilogue. We might need to add empty block detection for DL initialization and MBBI advance. Something like: 
>   DebugLoc DL = !MBB.empty() ? MBBI->getDebugLoc() : DebugLoc();
>   if (!MBB.empty() && !MBBI->isTerminator())
>      MBBI = std::next(MBBI);
Thanks Ana, it looks like this is a case of the shrink wrapping pass choosing a basic block which is empty as the place to insert the prologue. I didn't realise it was possible, but I'll push an updated patch that takes this into account.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62190