[PATCH] Add a shrink-wrapping pass to improve the placement of prologue and epilogue.

[Kristof Beyls]

> -----Original Message-----
> From: Quentin Colombet [mailto:qcolombet at apple.com]
> Sent: 28 April 2015 17:59
> To: qcolombet at apple.com; grosbach at apple.com
> Cc: kparzysz at codeaurora.org; Kristof Beyls; dberlin at dberlin.org; Amara
> Emerson; justin.holewinski at gmail.com; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Add a shrink-wrapping pass to improve the placement
> of prologue and epilogue.
> 
> ================
> Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:538-543
> @@ -538,5 +537,8 @@
>        MF.getSubtarget().getRegisterInfo());
> -  DebugLoc DL = MBBI->getDebugLoc();
> -  unsigned RetOpcode = MBBI->getOpcode();
> -
> +  DebugLoc DL;
> +  unsigned RetOpcode = 0;
> +  if (MBB.end() != MBBI) {
> +    DL = MBBI->getDebugLoc();
> +    RetOpcode = MBBI->getOpcode();
> +  }
>    int NumBytes = MFI->getStackSize();
> ----------------
> qcolombet wrote:
> > kristof.beyls wrote:
> > > I guess this code sequence will lead to DL sometimes being
> uninitialized.
> > > I'm not sure when "MBB.end() != MBBI", but in that situation, can't
> you get a reasonable debug location from somewhere else?
> > >
> > > If the RetOpcode variable is only used to detect tail call returns,
> > > maybe it's better to replace it with a bool variable called
> "isTailCallReturn" or something similar, and use that later on, instead
> of explicitly checking what the value is?
> > >
> > I guess I can use MBB.findDebugLoc() with some sensible iterator.
> > I will check.
> >
> > Same for RetOpcode, thanks for the feedback.
> I haven’t changed the handling of the debug information. It is indeed
> consistent with what is done
> AArch64FrameLowering::spillCalleeSavedRegisters and
> AArch64FrameLowering::restoreCalleeSavedRegisters.
> Therefore, I think that if we want to fix that, we should do it
> consistently at these three places.
> 
> What do you think?

Good point - I hadn't noticed how this was done in those 2 other methods.
I agree it's best to keep the same style and to fix is consistently at these
three places, if needed - but not as part of this patch.

> 
> http://reviews.llvm.org/D9210
> 
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>