llvm dev - Aug 2018 - Condition code in DAGCombiner::visitFADDForFMACombine?

I'm curious why the condition to fuse is this:

// Floating-point multiply-add with intermediate rounding.
  bool HasFMAD = (LegalOperations && TLI.isOperationLegal(ISD::FMAD,
VT));

static bool isContractable(SDNode *N) {
  SDNodeFlags F = N->getFlags();
  return F.hasAllowContract() || F.hasAllowReassociation();
}

bool CanFuse = Options.UnsafeFPMath || isContractable(N);
bool AllowFusionGlobally = (Options.AllowFPOpFusion == FPOpFusion::Fast ||
CanFuse || HasFMAD);
// If the addition is not contractable, do not combine.
if (!AllowFusionGlobally && !isContractable(N))
    return SDValue();

Specifically the AllowFusionGlobally, I would have expected something more
like:

bool AllowFusionGlobally = (Options.AllowFPOpFusion == FPOpFusion::Fast
&&
CanFuse && HasFMAD);

or at the very least:

bool AllowFusionGlobally = ((Options.AllowFPOpFusion == FPOpFusion::Fast ||
CanFuse) && HasFMAD);

It seems that as long as the target can do fmad it does do fmad since
HasFMAD is true.

Thanks.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180820/0c041a6c/attachment-0001.html>

So I have a test case where:

%20 = fmul nnan arcp float %15, %19
%21 = fadd reassoc nnan arcp contract float %20, -1.000000e+00

is being contracted in DAG to fmad. Is this correct since the fmul has no
reassoc or contract fast math flag?

Thanks.

On Mon, Aug 20, 2018 at 12:56 PM Ryan Taylor <ryta1203 at gmail.com>
wrote:
> I'm curious why the condition to fuse is this:
>
> // Floating-point multiply-add with intermediate rounding.
>   bool HasFMAD = (LegalOperations &&
TLI.isOperationLegal(ISD::FMAD, VT));
>
> static bool isContractable(SDNode *N) {
>   SDNodeFlags F = N->getFlags();
>   return F.hasAllowContract() || F.hasAllowReassociation();
> }
>
> bool CanFuse = Options.UnsafeFPMath || isContractable(N);
> bool AllowFusionGlobally = (Options.AllowFPOpFusion == FPOpFusion::Fast ||
> CanFuse || HasFMAD);
> // If the addition is not contractable, do not combine.
> if (!AllowFusionGlobally && !isContractable(N))
>     return SDValue();
>
> Specifically the AllowFusionGlobally, I would have expected something more
> like:
>
> bool AllowFusionGlobally = (Options.AllowFPOpFusion == FPOpFusion::Fast
&&
> CanFuse && HasFMAD);
>
> or at the very least:
>
> bool AllowFusionGlobally = ((Options.AllowFPOpFusion == FPOpFusion::Fast
> || CanFuse) && HasFMAD);
>
> It seems that as long as the target can do fmad it does do fmad since
> HasFMAD is true.
>
> Thanks.
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180821/2b30f2c4/attachment.html>

> On Aug 21, 2018, at 17:08, Ryan Taylor via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> So I have a test case where:
> 
> %20 = fmul nnan arcp float %15, %19
> %21 = fadd reassoc nnan arcp contract float %20, -1.000000e+00
> 
> is being contracted in DAG to fmad. Is this correct since the fmul has no
reassoc or contract fast math flag?
> 
> Thanks.

fmad is defined as the exact same result as the separate fmul + fadd, unlike fma
so this is OK

-Matt

On 21.08.2018 16:08, Ryan Taylor via llvm-dev wrote:
> So I have a test case where:
> 
> %20 = fmul nnan arcp float %15, %19
> %21 = fadd reassoc nnan arcp contract float %20, -1.000000e+00
> 
> is being contracted in DAG to fmad. Is this correct since the fmul has 
> no reassoc or contract fast math flag?
By having the reassoc and contract flags on fadd, the frontend is 
essentially saying "different rounding on the value produced by the fadd 
is okay".

So I'd say contracting this to fma is correct.

Where does this code come from, and why do you think contracting to fma 
is wrong?

Cheers,
Nicolai



> 
> Thanks.
> 
> On Mon, Aug 20, 2018 at 12:56 PM Ryan Taylor <ryta1203 at gmail.com 
> <mailto:ryta1203 at gmail.com>> wrote:
> 
>     I'm curious why the condition to fuse is this:
> 
>     // Floating-point multiply-add with intermediate rounding.
>        bool HasFMAD = (LegalOperations &&
>     TLI.isOperationLegal(ISD::FMAD, VT));
> 
>     static bool isContractable(SDNode *N) {
>        SDNodeFlags F = N->getFlags();
>        return F.hasAllowContract() || F.hasAllowReassociation();
>     }
> 
>     bool CanFuse = Options.UnsafeFPMath || isContractable(N);
>     bool AllowFusionGlobally = (Options.AllowFPOpFusion =>    
FPOpFusion::Fast || CanFuse || HasFMAD);
>     // If the addition is not contractable, do not combine.
>     if (!AllowFusionGlobally && !isContractable(N))
>          return SDValue();
> 
>     Specifically the AllowFusionGlobally, I would have expected
>     something more like:
> 
>     bool AllowFusionGlobally = (Options.AllowFPOpFusion =>    
FPOpFusion::Fast && CanFuse && HasFMAD);
> 
>     or at the very least:
> 
>     bool AllowFusionGlobally = ((Options.AllowFPOpFusion =>    
FPOpFusion::Fast || CanFuse) && HasFMAD);
> 
>     It seems that as long as the target can do fmad it does do fmad
>     since HasFMAD is true.
> 
>     Thanks.
> 
> 
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 

-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.