[llvm-commits] Please review: FMA implementation for X86 architecture

[Lang Hames]

Hi Elena,

You might want to check with Craig about the X86 specifics, since I'm not
very familiar with the X86 FMA work, but the DAG combines look good.

One minor style note:

+  // Negative multiplication when NegA xor NegB
+  bool NegMul = (NegA || NegB) && !(NegA && NegB);

This could be expressed as

bool NegMul = NegA ^ NegB;
or
bool NegMul = NegA != NegB;

Otherwise it looks good to me.

Cheers,
Lang.

On Wed, Jul 25, 2012 at 2:36 AM, Demikhovsky, Elena <
elena.demikhovsky at intel.com> wrote:

>  I’d like to commit this patch.****
>
> Hi Craig,****
>
> ** **
>
> Any objections?****
>
> ** **
>
> Thanks****
>
> ** **
>
> *- Elena*****
>
> *From:* Demikhovsky, Elena
> *Sent:* Tuesday, July 24, 2012 09:51
> *To:* 'Lang Hames'
> *Subject:* RE: [llvm-commits] Please review: FMA implementation for X86
> architecture****
>
> ** **
>
> Hi Lang,****
>
> ** **
>
> You are right. The expression x--(y*z) was converted to x+(y*z) before it
> reaches Combine().****
>
> I removed the code bellow.****
>
> ** **
>
> May I commit? ****
>
> ** **
>
> Thanks****
>
> ** **
>
> *- Elena*****
>
> *From:* Lang Hames [mailto:lhames at gmail.com <lhames at gmail.com>]
> *Sent:* Tuesday, July 24, 2012 00:19
> *To:* Demikhovsky, Elena
> *Cc:* Commit Messages and Patches for LLVM
> *Subject:* Re: [llvm-commits] Please review: FMA implementation for X86
> architecture****
>
> ** **
>
> Hi Elena,****
>
> ** **
>
> The second DAG combine looks a bit off:****
>
> ** **
>
> +    // fold (fsub x, -(fmul, y, z)) -> (fma (fneg y), z, x)****
>
> ** **
>
> That implies:****
>
> ** **
>
> x - -(y * z) ==> -y * z + a****
>
> ** **
>
> Which isn't a valid transformation. You probably don't want the negate on
> 'y'.****
>
> ** **
>
> +    if (N1.getOpcode() == ISD::FNEG && ****
>
> +        N1.getOperand(0).getOpcode() == ISD::FMUL &&****
>
> +        N1->hasOneUse() && N1.getOperand(0).hasOneUse()) {****
>
> +      SDValue N10 = N1.getOperand(0).getOperand(0);****
>
> +      SDValue N11 = N1.getOperand(0).getOperand(1);****
>
> +      return DAG.getNode(ISD::FMA, dl, VT,****
>
> +                         DAG.getNode(ISD::FNEG, N1->getDebugLoc(), VT,
> N10),****
>
> +                         N11, N1);****
>
> ** **
>
> Given the observation above, I think the ISD::FNEG should go away here.
> Also N1 is being used as the RHS addend. I think that should be N0?****
>
> ** **
>
> Otherwise it looks good to me.****
>
> ** **
>
> Cheers,****
>
> Lang.****
>
> ** **
>
> ** **
>
> On Mon, Jul 23, 2012 at 8:43 AM, Demikhovsky, Elena <
> elena.demikhovsky at intel.com> wrote:****
>
>
> I added code generation for FMA patterns on AVX2 architecture.
>
> - Elena
>
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits****
>
> ** **
>  ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120725/f87dba16/attachment.html>