This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombine] GetNegatedExpression - add FMA\FMAD support
ClosedPublic

Authored by RKSimon on Jun 11 2019, 7:46 AM.

Details

Summary

If the accumulator and either of the multiply operands are negateable then we can we negate the entire expression.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Jun 11 2019, 7:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2019, 7:46 AM
RKSimon planned changes to this revision.Jun 11 2019, 7:53 AM

Seeing a weird EXPENSIVE_CHECKS failure that I need to get to the bottom of

RKSimon updated this revision to Diff 205021.Jun 17 2019, 4:31 AM

updated to match FMUL/FDIV approach.

LGTM. Does everything still pass if you delete the FMA/FMAD case in AMDGPUTargetLowering::performFNegCombine?

LGTM. Does everything still pass if you delete the FMA/FMAD case in AMDGPUTargetLowering::performFNegCombine?

Thanks for pointing that out - it made me realise there's an issue in that we're not checking for UnsafeMath/hasNoSignedZeros like we do for FADD.

RKSimon planned changes to this revision.Jun 24 2019, 12:34 PM
RKSimon updated this revision to Diff 216134.Aug 20 2019, 6:34 AM

Updated patch to correctly respect NSZ flags. I've also improved the logic to ensure we negate the most profitable multiplication factor, not just the first one.

Once this is in place, the plan is to start work on https://bugs.llvm.org/show_bug.cgi?id=42863 so we can hook this into propagating negation through target opcodes as well.

Should this also remove the AMDGPU version?

Should this also remove the AMDGPU version?

The problem we currently have is that AMDGPUTargetLowering::performFNegCombine allows multiple uses of the FMA/FMAD but the DAGCombine doesn't, so I'm going to suggest we wait until PR42863 to handle that.

arsenm accepted this revision.Aug 22 2019, 10:51 AM

LGTM

This revision is now accepted and ready to land.Aug 22 2019, 10:51 AM
This revision was automatically updated to reflect the committed changes.