[LLVMdev] [PATCH]: Allow PRE to insert no-cost phi nodes

[Hal Finkel]

----- Original Message -----
> From: "Daniel Berlin" <dberlin at dberlin.org>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Tuesday, February 3, 2015 11:32:46 AM
> Subject: Re: [LLVMdev] [PATCH]: Allow PRE to insert no-cost phi nodes
> 
> 
> Updated patch attached

LGTM, thanks!

 -Hal

> 
> 
> On Fri Jan 30 2015 at 1:58:04 PM Daniel Berlin < dberlin at dberlin.org
> > wrote:
> 
> 
> 
> 
> On Wed Jan 28 2015 at 9:51:02 PM Hal Finkel < hfinkel at anl.gov >
> wrote:
> 
> 
> [moving to llvm-commits for patch review]
> 
> Hi Danny,
> 
> This seems reasonable to me.
> 
> You're patch has tabs in it, which you'll need to remove ;)
> 
> 
> 
> 
> 
> Sorry, i haven't had emacs set up for llvm in a while :)
> I'll make sure it happens
> 
> 
> 
> 
> 
> +; <label>:6 ; preds = %4, %2
> +; CHECK: pre-phi
> +; CHECK-NEXT: ret i32 %.pre-phi
> + %7 = add nsw i32 %a, %b
> + ret i32 %7
> 
> I'd like to see this test case be more explicit about what it is
> expecting. Can you please check for the new phi you're expecting
> with its value, and then check that it is returned?
> 
> 
> 
> 
> 
> Will do.
> 
> 
> 
> 
> 
> Thanks again,
> Hal
> 
> ----- Original Message -----
> > From: "Daniel Berlin" < dberlin at dberlin.org >
> > To: "LLVM Developers Mailing List" < llvmdev at cs.uiuc.edu >
> > Sent: Wednesday, January 28, 2015 5:15:20 PM
> > Subject: [LLVMdev] [PATCH]: Allow PRE to insert no-cost phi nodes
> > 
> > 
> > 
> > Current PRE tries to not increase code size.
> > It does this with a check whether or not the number of places it
> > would have to insert is 1 or not.
> > 
> > 
> > The problem with this is that the answer could also be "we have to
> > insert in zero places" in the case it's available in all
> > predecessors.
> > :)
> > 
> > 
> > Normal dominator based elimination in GVN will not catch these
> > cases
> > because the branches do not dominate the merge point.
> > 
> > 
> > A simple example
> > int c;
> > 
> > int d;
> > int pre(int foo, int a, int b)
> > {
> > int g;
> > if (foo) {
> > c = a+b;
> > } else {
> > d = a+ b;
> > }
> > g = a+b;
> > return g;
> > }
> > 
> > 
> > Without this patch, PRE (and GVN) will not eliminate g = a+b.
> > With this patch, PRE will create phi(c, d) and eliminate g = a + b.
> > 
> > 
> > (It is tremendously more difficult to make current GVN understand
> > the
> > problem, and GCC solves this the same way i'm about to).
> > 
> > 
> > The patch looks large because of code movement, but it basically
> > all
> > boils down to changing this check:
> > 
> > 
> > if (NumWithout != 1 || NumWith == 0)
> > 
> > 
> > to
> > if (NumWithout > 1 || NumWith == 0)
> > 
> > 
> > and skipping insertion when NumWithout == 0
> > 
> > 
> > testcase included.
> > 
> > 
> > ______________________________ _________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/ mailm an/listinfo/llvmdev
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory