<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 7, 2015, at 11:42 AM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" class="">elena.demikhovsky@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">><span class="Apple-converted-space"> </span></span>Well, VSELECT can be translated to an instruction, so I think that’s different. If the VSELECT and the op get separated we generate less efficient code so that’s always OK.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">It may cause FP exception.</div></div></div></blockquote><div><br class=""></div><div>In those cases I am not convinced that we are correct in generating a vselect.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">><span class="Apple-converted-space"> </span></span>In this case I am not convinced that we can’t generate the wrong result.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">If ROUNDMODE and instruction will be separated somehow, the compilation will fail.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">I say “somehow” because we are talking about intrinsic that mapped directly to instruction - everything is legal inside.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Can you give an example?<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">The alternative is to invent one more node per instruction that needs rounding mode.</div></div></div></blockquote><div><br class=""></div><div>Sounds like we may want to create AVX512-specific nodes for all FP ops. Then we could have both the mask register and the rounding mode as operands.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">> It’s an important enough design decision for the AVX512 SDAG IR to warrant some scrutiny.<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">> I suggest to properly comment this node and provide explanation in the review itself and CC a few more people (Nadav, Chandler, Hal, Jim, etc.)<o:p class=""></o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">I see this as a small patch which resolves FP intrinsics rounding mode, but no problem, we can widen the list of reviewers.</span></div></div></div></blockquote><div><br class=""></div><div>It’s usually not the size of patch that matters but whether it’s changing the design. In this case, I think that we’re breaking the notion that the SDAG (A (B)) is legal if both A and B have ISA counterparts (whether (A (B)) together has a match is secondary). As you say we’re late in the backend so this could made an exception but this still feels like an extension of the design.</div><div><br class=""></div><div>Adam</div><br class=""><blockquote type="cite" class=""><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""> </span></div><div class=""><div style="margin: 0cm 0cm 0.0001pt 36pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -18pt;" class=""><span style="font-family: Calibri, sans-serif; color: rgb(49, 132, 155);" class=""><span class="">-<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';" class=""> <span class="Apple-converted-space"> </span></span></span></span><span dir="LTR" class=""></span><b class=""><i class=""><span style="color: rgb(49, 132, 155);" class=""> Elena<o:p class=""></o:p></span></i></b></div></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""> </span></div><div class=""><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0cm 0cm;" class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><b class=""><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class="">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class=""><span class="Apple-converted-space"> </span>Adam Nemet [<a href="mailto:anemet@apple.com" style="color: purple; text-decoration: underline;" class="">mailto:anemet@apple.com</a>]<span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Wednesday, January 07, 2015 20:09<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Demikhovsky, Elena<br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:reviews+D6835+public+db07cccdafa4527c@reviews.llvm.org" style="color: purple; text-decoration: underline;" class="">reviews+D6835+public+db07cccdafa4527c@reviews.llvm.org</a>; Badouh, Asaf;<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits@cs.uiuc.edu</a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [PATCH] [AVX-512] - Add FMA instruction with Rounding mode<o:p class=""></o:p></span></div></div></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">On Jan 6, 2015, at 11:12 PM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" style="color: purple; text-decoration: underline;" class="">elena.demikhovsky@intel.com</a>> wrote:<o:p class=""></o:p></div></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class=""><br class=""><br class="">- Elena<br class=""><br class="">-----Original Message-----<br class="">From: Adam Nemet [</span><a href="mailto:anemet@apple.com" style="color: purple; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">mailto:anemet@apple.com</span></a><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">]<span class="apple-converted-space"> </span><br class="">Sent: Wednesday, January 07, 2015 03:04<br class="">To: Badouh, Asaf; Demikhovsky, Elena;<span class="apple-converted-space"> </span></span><a href="mailto:anemet@apple.com" style="color: purple; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">anemet@apple.com</span></a><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class=""><br class="">Cc:<span class="apple-converted-space"> </span></span><a href="mailto:cameron.mcinally@nyu.edu" style="color: purple; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">cameron.mcinally@nyu.edu</span></a><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">;<span class="apple-converted-space"> </span></span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">llvm-commits@cs.uiuc.edu</span></a><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class=""><br class="">Subject: Re: [PATCH] [AVX-512] - Add FMA instruction with Rounding mode<br class=""><br class="">REPOSITORY<br class=""> rL LLVM<br class=""><br class="">================<br class="">Comment at: lib/Target/X86/X86ISelLowering.h:374<br class="">@@ -373,1 +373,3 @@<br class=""><br class="">+ ROUNDMODE,<br class="">+<br class="">----------------<br class="">Hmm, has this been completely thought through? I may have missed the discussion...<span class="apple-converted-space"> </span><br class=""><br class="">Looks like you're introducing a new node that if it surrounds an FMA op it changes its rounding mode?<br class=""><br class="">What happens if the two nodes get separated by some transformation?<br class="">[Demikhovsky, Elena] We surround with VSELECT to set mask for intrinsic. The idea here is the same, even less risky. The stage is late, all transformations are behind.<br class="">We plan to use this node for all FP arithmetic and conversion intrinsics.</span><o:p class=""></o:p></div></div></blockquote><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Well, VSELECT can be translated to an instruction, so I think that’s different. If the VSELECT and the op get separated we generate less efficient code so that’s always OK. In this case I am not convinced that we can’t generate the wrong result.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">My main point is that unless this was discussed somewhere else, it shouldn’t be hidden in a patch like this. It’s an important enough design decision for the AVX512 SDAG IR to warrant some scrutiny.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">I suggest to properly comment this node and provide explanation in the review itself and CC a few more people (Nadav, Chandler, Hal, Jim, etc.)<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Thanks,<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Adam<o:p class=""></o:p></div></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><br class=""><br class=""><o:p class=""></o:p></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class=""><br class="">================<br class="">Comment at: lib/Target/X86/X86InstrAVX512.td:3541-3542<br class="">@@ +3540,4 @@<br class="">+ X86VectorVTInfo VTI, SDPatternOperator<span class="apple-converted-space"> </span><br class="">+ OpNode> { defm v213r : avx512_fma3_round_rrb<opc213, !strconcat(OpcodeStr, "213", VTI.Suffix),<br class="">+ VTI, OpNode>, EVEX_CD8<VTI.EltSize,<span class="apple-converted-space"> </span><br class="">+ CD8VF>;<br class="">+<br class="">----------------<br class="">I think that the EVEX_CD8 thing can be written as VTI.CD8TupleForm.<br class=""><br class="">Same later.<br class=""><br class=""><a href="http://reviews.llvm.org/D6835" style="color: purple; text-decoration: underline;" class="">http://reviews.llvm.org/D6835</a><br class=""><br class="">EMAIL PREFERENCES<br class=""> <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" style="color: purple; text-decoration: underline;" class="">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br class=""><br class=""><br class="">---------------------------------------------------------------------<br class="">Intel Israel (74) Limited<br class=""><br class="">This e-mail and any attachments may contain confidential material for<br class="">the sole use of the intended recipient(s). Any review or distribution<br class="">by others is strictly prohibited. If you are not the intended<br class="">recipient, please contact the sender and delete all copies.<br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""></span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">llvm-commits@cs.uiuc.edu</span></a><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class=""><br class=""></span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="color: purple; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica, sans-serif;" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</span></a><o:p class=""></o:p></div></div></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><p style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">---------------------------------------------------------------------<br class="">Intel Israel (74) Limited</p><p style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">This e-mail and any attachments may contain confidential material for<br class="">the sole use of the intended recipient(s). Any review or distribution<br class="">by others is strictly prohibited. If you are not the intended<br class="">recipient, please contact the sender and delete all copies.</p></div></blockquote></div><br class=""></body></html>