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

[Mark Minich]

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
> > 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1430.diff
Type: application/octet-stream
Size: 22568 bytes
Desc: D1430.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130819/097aa984/attachment.obj>