<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from rtf -->
<style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<font face="Times New Roman" size="3"><span style="font-size:12pt;"><a name="_MailEndCompose"></a>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">- Elena</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">-----Original Message-----<br>
From: Mitch Bodart [<a href="mailto:mitch.l.bodart@intel.com">mailto:mitch.l.bodart@intel.com</a>]
<br>
Sent: Tuesday, January 19, 2016 19:04<br>
To: Breger, Igor <igor.breger@intel.com>; Demikhovsky, Elena <elena.demikhovsky@intel.com>; Badouh, Asaf <asaf.badouh@intel.com>; Kreitzer, David L <david.l.kreitzer@intel.com>; Bodart, Mitch L <mitch.l.bodart@intel.com><br>
Cc: llvm-commits@lists.llvm.org<br>
Subject: Re: [PATCH] D16137: AVX512: VMOVDQU8/16/32/64 (load) intrinsic implementation.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">mbodart added a comment.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">Hi Asaf,</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">Thanks for the answers.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">Note that SelectionDAGLegalize::LegalizeOp, in the case of custom lowering, already handles the case of multiple results, albeit with this FIXME comment:</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> // FIXME: The handling for custom lowering with multiple results is</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> // a complete mess.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">So it is not clear to me why the new masked load intrinsics require additional changes, especially since we have existing masked load intrinsics with a chain result.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">I'm guessing it's because legalization of the I64 mask operand on IA32 does not go through this SelectionDAGLegalize::LegalizeOp path. Is that correct?</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"><b><i>[Demikhovsky, Elena] </i></b><b><i>Yes, right. The existing</i></b><b><i> masked_load just does not have i64 a</i></b><b><i>rgument that requires lega</i></b><b><i>lization</i></b><b><i>.</i></b></span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">Assuming we do need an X86 override of LowerOperationWrapper, then my next thought would be to try and fix the FILD issue in X86TargetLowering::FP_TO_INTHelper. But I don't know how to express
there, in Selection Dag representation, that only one result is needed.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">If you know how to do that, then that would be a preferable solution.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"><b><i>[Demikhovsky, Elena] </i></b><b><i>Igor tried. </i></b><b><i>The created node </i></b><b><i>(load or memory intrinsic) has 2 results </i></b><b><i>– value </i></b><b><i>and chain.</i></b><b><i>
</i></b><b><i>But this is Ok. Look</i></b><b><i> at </i></b><b><i>ReplaceAllUsesWith</i></b><b><i>.</i></b></span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"><b><i>This function is used on all stages of DAG modification.</i></b><b><i> If </i></b><b><i>“</i></b><b><i>From</i></b><b><i>”</i></b><b><i> has 1 values and </i></b><b><i>“</i></b><b><i>To</i></b><b><i>”</i></b><b><i>
has 2 values</i></b><b><i> then the second value of </i></b><b><i>“</i></b><b><i>To</i></b><b><i>”</i></b><b><i> is dropped.</i></b></span></font></div>
<div><font face="Consolas" size="2" color="blue"><span style="font-size:11pt;">void<font color="black"> </font><font color="#2B91AF">SelectionDAG</font><font color="black">::ReplaceAllUsesWith(</font><font color="#2B91AF">SDNode</font><font color="black"> *</font><font color="gray">From</font><font color="black">,
</font>const<font color="black"> </font><font color="#2B91AF">SDValue</font><font color="black"> *</font><font color="gray">To</font><font color="black">) {</font></span></font></div>
<div><font face="Consolas" size="2"><span style="font-size:11pt;"> <font color="blue">if</font> (<font color="gray">From</font>->getNumValues() == 1) <font color="green">// Handle the simple case efficiently.</font></span></font></div>
<div><font face="Consolas" size="2"><span style="font-size:11pt;"> <font color="blue">return</font> ReplaceAllUsesWith(<font color="#2B91AF">SDValue</font>(<font color="gray">From</font>, 0), <font color="gray">To</font>[0]);</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">If we really need LowerOperationWrapper to discard the FILD's Chain operand, then I think we need to make it clear in that routine that it should only be happening for the FILD case, as in general
it is not safe to simply drop a result. So a source comment there describing the situation is needed. And when we do need to drop a result, I think there should be an assertion checking exactly for the FILD case. That is, we should only ever drop one result,
and it should be a Chain, and if reasonable, we should check that the retained result is an FILD. This will make it clear to developers why this code is needed, and thus make it easier to modify in the future.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"><b><i>[Demikhovsky, Elena] </i></b><b><i>There is a special interface that allows to</i></b><b><i> “drop” Res1. It goes to </i></b><b><i> </i></b><b><i>ReplaceAllUsesWith</i></b><b><i>() I pointed
above.</i></b></span></font></div>
<div><font face="Consolas" size="2" color="#2B91AF"><span style="font-size:11pt;">SDValue<font color="black"> CombineTo(</font>SDNode<font color="black"> *</font><font color="gray">N</font><font color="black">, </font>SDValue<font color="black"> </font><font color="gray">Res0</font><font color="black">,
</font>SDValue<font color="black"> </font><font color="gray">Res1</font><font color="black">,</font></span></font></div>
<div><font face="Consolas" size="2"><span style="font-size:11pt;"> <font color="blue">bool</font> <font color="gray">AddTo</font> = <font color="blue">true</font>) {</span></font></div>
<div><font face="Consolas" size="2"><span style="font-size:11pt;"> <font color="#2B91AF">SDValue</font> To[] = { <font color="gray">Res0</font>, <font color="gray">Res1</font> };</span></font></div>
<div><font face="Consolas" size="2"><span style="font-size:11pt;"> <font color="blue">return</font> CombineTo(<font color="gray">N</font>, To, 2, <font color="gray">AddTo</font>);</span></font></div>
<div><font face="Consolas" size="2"><span style="font-size:11pt;"> }</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">Thanks for updating the test functions!</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">One other thing I noticed about the test files is that most of them do not test for a 32-bit target.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">That doesn't need to be added for this change set. But as I64 masking has interesting behavior on IA32, it would be good to beef up testing there.</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">regards,</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">- mitch</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;">Repository:</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> rL LLVM</span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"><a href="http://reviews.llvm.org/D16137">http://reviews.llvm.org/D16137</a></span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
<div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div>
</span></font>
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited</p>
<p>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.</p></body>
</html>