[PATCH] [LLVMdev] More FMA folding opportunities

Thu Jan 8 16:06:44 PST 2015

----- Original Message -----
> From: "Aditya Nandakumar" <aditya_nandakumar at apple.com>
> To: "Olivier H Sallenave" <ohsallen at us.ibm.com>
> Cc: "Hal Finkel" <hfinkel at anl.gov>, llvm-commits at cs.uiuc.edu
> Sent: Thursday, January 8, 2015 5:53:47 PM
> Subject: Re: [PATCH] [LLVMdev] More FMA folding opportunities
> 
> Hi Olivier, Hal
> 
> 
> I have a question about this patch.
> fadd( fp_ext (fmul a,b ) c)-> fma(a,b,c) just throws away type
> information which prevents targets from optimizing for
> precision/power.
> Shouldn’t this be target specific or at the least inside
> enableAggressiveFMAFusion?

If you think it should be because it might not be universally profitable, then it probably should be (or under some other similar flag).

> 
> 
> Also, won’t the FMA node be illegal ? - It will have two F16 operands
> and 1 F32 operand.

Olivier, Aditya is right. You need to add the FP_EXTEND to the operands of the resulting FMA as appropriate. Can you please fix that? We can also move those under the "aggressive" flag for now.

 -Hal

> 
> 
> Thanks
> Aditya
> 
> 
> 
> 
> On Jan 7, 2015, at 1:01 PM, Olivier H Sallenave < ohsallen at us.ibm.com
> > wrote:
> 
> 
> 
> 
> Just got access, commited as r225380.
> 
> Thanks for reviewing,
> Olivier
> 
> <graycol.gif> Hal Finkel ---01/06/2015 10:26:02 PM-------- Original
> Message ----- > From: "Olivier H Sallenave" < ohsallen at us.ibm.com >
> 
> From: Hal Finkel < hfinkel at anl.gov >
> To: Olivier H Sallenave/Watson/IBM at IBMUS
> Cc: < llvm-commits at cs.uiuc.edu >
> Date: 01/06/2015 10:26 PM
> Subject: Re: [PATCH] Fw: [LLVMdev] More FMA folding opportunities
> 
> 
> 
> 
> ----- Original Message -----
> > From: "Olivier H Sallenave" < ohsallen at us.ibm.com >
> > To: "Hal Finkel" < hfinkel at anl.gov >
> > Cc: llvm-commits at cs.uiuc.edu
> > Sent: Tuesday, January 6, 2015 8:03:51 PM
> > Subject: Re: [PATCH] Fw: [LLVMdev] More FMA folding opportunities
> > 
> > 
> > 
> > Hal,
> > 
> > You're right, I moved those heuristics in the target-independent
> > combiner.
> 
> LGTM. Do you have commit access?
> 
> -Hal
> 
> > 
> > Thanks,
> > Olivier
> > 
> > (See attached file: patch-v3.diff)
> > 
> > Inactive hide details for Hal Finkel ---01/06/2015 06:52:06
> > PM-------- Original Message ----- > From: "Olivier H Sallenave"
> > <ohHal Finkel ---01/06/2015 06:52:06 PM-------- Original Message
> > ----- > From: "Olivier H Sallenave" < ohsallen at us.ibm.com >
> > 
> > From: Hal Finkel < hfinkel at anl.gov >
> > To: Olivier H Sallenave/Watson/IBM at IBMUS
> > Cc: < llvm-commits at cs.uiuc.edu >
> > Date: 01/06/2015 06:52 PM
> > Subject: Re: [PATCH] Fw: [LLVMdev] More FMA folding opportunities
> > 
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Olivier H Sallenave" < ohsallen at us.ibm.com >
> > > To: llvm-commits at cs.uiuc.edu
> > > Cc: "Hal Finkel" < hfinkel at anl.gov >
> > > Sent: Monday, January 5, 2015 9:51:22 AM
> > > Subject: Fw: [PATCH] Fw: [LLVMdev] More FMA folding opportunities
> > > 
> > > 
> > > 
> > > Hi,
> > > 
> > > Here's a more comprehensive patch also handling the fact that
> > > FP_EXTEND will later be removed for PPC. Accordingly, new
> > > patterns
> > > are implemented in the PPC-specific combiner:
> > 
> > Given that these are predicated on (TM.Options.UnsafeFPMath ||
> > TM.Options.AllowFPOpFusion == FPOpFusion::Fast), what makes them
> > PPC
> > specific? Either of those two options allow us to compute the
> > relevant operations with extra precision, right?
> > 
> > -Hal
> > 
> > > 
> > > > Finally, specifically for the PPC target, we could ignore
> > > > FP_EXTEND
> > > > in the patterns above as it will be removed by the Machine
> > > > Common
> > > > Subexpression Elimination pass. For instance:
> > > > 
> > > > fold (fadd (fpext (fmul x, y)), z) -> (fma x, y, z)
> > > > fold (fadd (fpext (fma x, y, (fmul u, v))), z) -> (fma x, y
> > > > (fma
> > > > u,
> > > > v, z))
> > > 
> > > I also moved the tests in new files (fma-ext.ll, fma-assoc.ll)
> > > for
> > > more clarity.
> > > 
> > > Thanks,
> > > Olivier
> > > 
> > > (See attached file: patch-v2.diff)
> > > 
> > > 
> > > 
> > > ----- Forwarded by Olivier H Sallenave/Watson/IBM on 01/05/2015
> > > 10:44
> > > AM -----
> > > 
> > > From: Olivier H Sallenave/Watson/IBM
> > > To: llvm-commits at cs.uiuc.edu
> > > Cc: "Hal Finkel" < hfinkel at anl.gov >
> > > Date: 12/29/2014 05:09 PM
> > > Subject: [PATCH] Fw: [LLVMdev] More FMA folding opportunities
> > > 
> > > 
> > > 
> > > Hi,
> > > 
> > > Attached is a patch to support more FMA folding opportunities
> > > (especially for PPC) as discussed below.
> > > 
> > > Thanks,
> > > Olivier
> > > 
> > > (See attached file: patch.diff)
> > > 
> > > 
> > > 
> > > ----- Forwarded by Olivier H Sallenave/Watson/IBM on 12/29/2014
> > > 04:56
> > > PM -----
> > > 
> > > From: Hal Finkel < hfinkel at anl.gov >
> > > To: Olivier H Sallenave/Watson/IBM at IBMUS
> > > Cc: < llvmdev at cs.uiuc.edu >
> > > Date: 09/30/2014 08:08 PM
> > > Subject: Re: [LLVMdev] More FMA folding opportunities
> > > 
> > > 
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Olivier H Sallenave" < ohsallen at us.ibm.com >
> > > > To: llvmdev at cs.uiuc.edu
> > > > Sent: Monday, September 29, 2014 3:34:51 PM
> > > > Subject: [LLVMdev] More FMA folding opportunities
> > > > 
> > > > Hi,
> > > > 
> > > > I think more opportunities might be added for FMA in the DAG
> > > > combiner, please tell me what you think. Right now, those cases
> > > > are
> > > > implemented:
> > > > 
> > > > fold (fadd (fmul x, y), z) -> (fma x, y, z)
> > > > fold (fadd x, (fmul y, z)) -> (fma y, z, x)
> > > > 
> > > > When the TLI callback "enableAggressiveFMAFusion" returns true,
> > > > we
> > > > might also support:
> > > > 
> > > > fold (fadd (fma x, y, (fmul u, v)), z) -> (fma x, y (fma u, v,
> > > > z))
> > > > fold (fadd x, (fma y, z, (fmul u, v)) -> (fma y, z (fma u, v,
> > > > x))
> > > > 
> > > > This kind of reassociation generates two FMA for (x^2 + y^2 +
> > > > z).
> > > 
> > > Yes, this all sounds reasonable.
> > > 
> > > Thanks again,
> > > Hal
> > 
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > 
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 

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