Please review the patch for tab characters and trailing whitespace.<br><br>   defm VFMADDPD    : fma3p_forms<0x98, 0xA8, 0xB8, "vfmadd", "pd", memopv2f64,<br>-    memopv4f64, int_x86_fma_vfmadd_pd, int_x86_fma_vfmadd_pd_256>, VEX_W;<br>

+    memopv4f64, int_x86_fma_vfmadd_pd, int_x86_fma_vfmadd_pd_256, X86Fmadd, v2f64, v4f64>, VEX_W;<br>   defm VFMSUBPD    : fma3p_forms<0x9A, 0xAA, 0xBA, "vfmsub", "pd", memopv2f64,<br>-    memopv4f64, int_x86_fma_vfmsub_pd, int_x86_fma_vfmsub_pd_256>, VEX_W;<br>

+    memopv4f64, int_x86_fma_vfmsub_pd, int_x86_fma_vfmsub_pd_256, X86Fmsub, v2f64, v4f64>, VEX_W;<br>   defm VFMADDSUBPD : fma3p_forms<0x96, 0xA6, 0xB6, "vfmaddsub", "pd", memopv2f64,<br>-    memopv4f64, int_x86_fma_vfmaddsub_pd, int_x86_fma_vfmaddsub_pd_256>, VEX_W;<br>

+    memopv4f64, int_x86_fma_vfmaddsub_pd, int_x86_fma_vfmaddsub_pd_256, X86Fmaddsub, v2f64, v4f64>, VEX_W;<br>   defm VFMSUBADDPD : fma3p_forms<0x97, 0xA7, 0xB7, "vfmsubadd", "pd", memopv2f64,<br>

-    memopv4f64, int_x86_fma_vfmsubadd_pd, int_x86_fma_vfmsubadd_pd_256>, VEX_W;<br>+    memopv4f64, int_x86_fma_vfmsubadd_pd, int_x86_fma_vfmsubadd_pd_256, X86Fmsubadd, v2f64, v4f64>, VEX_W;<br><br>Please keep lines below 80 columns. This is just an example. There are others.<br>

<br>+  if (ScalarVT != MVT::f32 && ScalarVT != MVT::f64 || !Subtarget->hasFMA())<br>+    return SDValue();<br><br>This code fails -Wlogical-op-parentheses. Please put parentheses around the && part.<br>

<br>      setOperationAction(ISD::FMA,             MVT::v8f32, Custom);<br>      setOperationAction(ISD::FMA,             MVT::v4f64, Custom);<br>      setOperationAction(ISD::FMA,             MVT::v4f32, Custom);<br>      setOperationAction(ISD::FMA,             MVT::v2f64, Custom);<br>

      setOperationAction(ISD::FMA,             MVT::f32, Custom);<br>      setOperationAction(ISD::FMA,             MVT::f64, Custom);<br><br>Why are these under hasAVX2() instead of hasFMA()?<br><br>+static SDValue PerformFMACombine(SDNode *N, SelectionDAG &DAG, <br>

+                                 const X86Subtarget* Subtarget) {<br>+  DebugLoc dl = N->getDebugLoc();<br>+  SDValue N0 = N->getOperand(0);<br>+  EVT VT = N->getValueType(0);<br><br>The N0 variable here is unused.<br>

<br>Other than those thing it looks good to me.<br><br><br><div class="gmail_quote">On Thu, Jul 26, 2012 at 12:52 AM, Demikhovsky, Elena <span dir="ltr"><<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">





<div link="blue" vlink="purple" lang="EN-US">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thank you, Lang.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Craig did not answer yet. I’ll wait a few days.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><i><span style="color:teal">- Elena</span></i></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Lang Hames [mailto:<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>]
<br>
<b>Sent:</b> Wednesday, July 25, 2012 21:45<br>
<b>To:</b> Demikhovsky, Elena<br>
<b>Cc:</b> Craig Topper; Commit Messages and Patches for LLVM</span></p><div><div><br>
<b>Subject:</b> Re: [llvm-commits] Please review: FMA implementation for X86 architecture<u></u><u></u></div></div><p></p>
</div><div><div>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Hi Elena,<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">One minor style note:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">+  // Negative multiplication when NegA xor NegB <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+  bool NegMul = (NegA || NegB) && !(NegA && NegB); <u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">This could be expressed as <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">bool NegMul = NegA ^ NegB;<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">or<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">bool NegMul = NegA != NegB;<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Otherwise it looks good to me.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Cheers,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Lang.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Wed, Jul 25, 2012 at 2:36 AM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I’d like to commit this patch.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Craig,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Any objections?</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><i><span style="color:teal">- Elena</span></i></b><u></u><u></u></p>
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Demikhovsky, Elena
<br>
<b>Sent:</b> Tuesday, July 24, 2012 09:51<br>
<b>To:</b> 'Lang Hames'<br>
<b>Subject:</b> RE: [llvm-commits] Please review: FMA implementation for X86 architecture</span><u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Lang,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">You are right. The expression x--(y*z) was converted to x+(y*z) before it reaches Combine().</span><u></u><u></u></p>


<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I removed the code bellow.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">May I commit?
</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><i><span style="color:teal">- Elena</span></i></b><u></u><u></u></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Lang Hames [<a href="mailto:lhames@gmail.com" target="_blank">mailto:lhames@gmail.com</a>]
<br>
<b>Sent:</b> Tuesday, July 24, 2012 00:19<br>
<b>To:</b> Demikhovsky, Elena<br>
<b>Cc:</b> Commit Messages and Patches for LLVM<br>
<b>Subject:</b> Re: [llvm-commits] Please review: FMA implementation for X86 architecture</span><u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<p class="MsoNormal">Hi Elena,<u></u><u></u></p>
<div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">The second DAG combine looks a bit off:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">+    // fold (fsub x, -(fmul, y, z)) -> (fma (fneg y), z, x)<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">That implies:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">x - -(y * z) ==> -y * z + a<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Which isn't a valid transformation. You probably don't want the negate on 'y'.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+    if (N1.getOpcode() == ISD::FNEG && <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+        N1.getOperand(0).getOpcode() == ISD::FMUL &&<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+        N1->hasOneUse() && N1.getOperand(0).hasOneUse()) {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+      SDValue N10 = N1.getOperand(0).getOperand(0);<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+      SDValue N11 = N1.getOperand(0).getOperand(1);<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+      return DAG.getNode(ISD::FMA, dl, VT,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+                         DAG.getNode(ISD::FNEG, N1->getDebugLoc(), VT, N10),<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+                         N11, N1);<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Otherwise it looks good to me.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Cheers,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Lang.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Mon, Jul 23, 2012 at 8:43 AM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
I added code generation for FMA patterns on AVX2 architecture.<br>
<br>
- Elena<br>
<br>
<br>
---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.</span><u></u><u></u></p>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div></div></div><div><div>
<font face="monospace">---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.</font></div></div></div>

</blockquote></div><br><br clear="all"><br>-- <br>~Craig<br>