[PATCH][PowerPC] More refactoring prior to real PPC emitPrologue()/emitEpilogue() changes

[Bill Schmidt]

Committed as r188741.  Please consider developing a test case for the
bug you fixed, verifying the correct code generation for the base
pointer restore for PPC32.  I realize it's pretty evident, but we try to
add tests for each bug fixed.

Thanks,
Bill

On Mon, 2013-08-19 at 16:52 -0400, Mark Minich wrote:
> Revised to address comments regarding variable naming....  see http://llvm-reviews.chandlerc.com/D1430
> ...also attached to this email.
> 
> -Mark
> 
> 
> -----Original Message-----
> From: Bill Schmidt [mailto:wschmidt at linux.vnet.ibm.com] 
> Sent: Sunday, August 18, 2013 6:03 PM
> To: Hal Finkel
> Cc: Mark Minich; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH][PowerPC] More refactoring prior to real PPC emitPrologue()/emitEpilogue() changes
> 
> Mark, I'll echo Hal's comment about the variable names.  This is one of
> those naming convention things that we all agree to live by.
> 
> In addition, please change:
> 
>   STW_Inst to StoreInst
>   STWU_Inst to StoreUpdInst
>   STWUX_Inst to StoreUpdIdxInst
>   LWZ_Inst to LoadInst
> 
> or similar.  The point is that STD is never a STW instruction, STDU is
> never a STWU instruction, etc.  Please change the names to avoid bias
> for one subtarget or another.
> 
> Other than that, the patch looks fine.  Please generate a new version
> with the changed variable names and I will regression-test and commit
> for you (provided nobody else has further comments).
> 
> BTW, thanks for doing this!  Factoring this code is long overdue.
> 
> Thanks,
> Bill
> 
> On Fri, 2013-08-16 at 17:03 -0500, Hal Finkel wrote:
> > Mark,
> > 
> > I'll look at this in more detail, but just a quick note: Please remove the underscores from the variable names.
> > 
> > Thanks again,
> > Hal
> > 
> > ----- Original Message -----
> > > 
> > > 
> > > 
> > > 
> > > This is a continuation of the refactorings performed in svn rev
> > > 188573 (see that rev's comments for more detail).
> > > 
> > > 
> > > 
> > > This is my stage 2 refactoring: I combined the emitPrologue() &
> > > emitEpilogue() PPC32 & PPC64 code into a single flow, simplifying a
> > > lot of the code since in essence the PPC32 & PPC64 code generation
> > > logic is the same, only the instruction forms are different (in most
> > > cases). This simplification is necessary because my functional
> > > changes (yet to come) add significant complexity, and without the
> > > simplification of my stage 2 refactoring, the overall complexity of
> > > both emitPrologue() & emitEpilogue() would have become almost
> > > intractable for most mortal programmers (like me).
> > > 
> > > 
> > > 
> > > This submission was intended to be a pure refactoring (no functional
> > > changes whatsoever). However, in the process of combining the PPC32
> > > & PPC64 flows, I spotted a difference that I believe is a bug (see
> > > svn rev 186478 line 863, or svn rev 188573 line 888): This line
> > > appears to be restoring the BP with the original FP content, not the
> > > original BP content. When I merged the 32-bit and 64-bit code, I
> > > used the corresponding code from the 64-bit flow, which I believe
> > > uses the correct offset (BPOffset) for this operation.
> > > 
> > > 
> > > 
> > > http://llvm-reviews.chandlerc.com/D1430
> > > 
> > > 
> > > 
> > > 
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > > 
> > 
>