<div dir="ltr">Hi Adam,<br><div class="gmail_extra"><br><br><div class="gmail_quote">2014-08-15 9:06 GMT+04:00 Adam Nemet <span dir="ltr"><<a href="mailto:anemet@apple.com" target="_blank">anemet@apple.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">Hi Robert,<div><br><div>
<div><div class="h5"><div>On Aug 14, 2014, at 10:02 AM, Robert Khasanov <<a href="mailto:rob.khasanov@gmail.com" target="_blank">rob.khasanov@gmail.com</a>> wrote:</div><br><blockquote type="cite"><div dir="ltr">Hi Elena and Adam,<br>
<div class="gmail_extra"><br><br><div class="gmail_quote">2014-08-13 22:20 GMT+04:00 Adam Nemet <span dir="ltr"><<a href="mailto:anemet@apple.com" target="_blank">anemet@apple.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">Hi Elena and Robert,<div>
<br><div><div><div>On Aug 13, 2014, at 12:01 AM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:</div><br><blockquote type="cite"><div style="font-family:Helvetica;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<font face="Calibri"><span style="font-size:11pt"><div>Hi Nadav, Craig, Adam,</div><div> </div><div>Robert works on SKX and I’m reviewing the code. I have a question about the form of patterns. Let’s assume that everything is correct here. Do you think that it is the suitable form? And shorter is better?</div>
<div>Or on some stage it becomes unreadable?</div></span></font></div></blockquote><div><br></div></div><div>Thanks very much for starting this discussion! I have been struggling to find the balance between generality and readability as well so it’s good to have this meta-discussion to make sure we’re on the same page.</div>
<div><br></div><div>I think that we should try to use tablegen to generate as much as possible. I believe that this is the only way to get the huge number of instruction-variants/intrinsics supported for AVX512 in a scalable/maintainable way. This is where I was going with the AVX512_masking classes and handling intrinsics via lowering to SDNodes.</div>
<div><br></div><div>At the same time we should try to separate the unreadable part (e.g. the !cast<blah>(xxx#eltnr#yyy) from the code that actually captures some important logic.</div><div><br></div><div>One idea I had is to introduce a class to capture all the different information we derive from the vector type. This is essentially used to hide and to centralize the nasty mappings. E.g.:</div>
<div><br></div><div><div>+// Group template arguments that can be derived from the vector type (EltNum x</div><div>+// EltVT). These are things like RegisterClass, the corresponding mask RC,</div><div>+// etc. The idea is to pass one of these as the template argument rather than</div>
<div>+// the individual arguments.</div><div>+class X86VectorVTInfo<int EltNum, ValueType EltVT, RegisterClass rc = VR512,</div><div>+ string suffix = ""> {</div><div>+ RegisterClass RC = rc;</div>
<div>+ RegisterClass KRC = !cast<RegisterClass>("VK"##EltNum##"WM");</div><div>+ RegisterClass MRC = !cast<RegisterClass>("GR"##EltNum);</div><div>+ string Suffix = suffix;</div>
<div>+ ValueType VT = !cast<ValueType>("v"##EltNum##EltVT);</div><div>+}</div><div><br></div><div>I am sure there is bunch more “field” to be added for BW/DQ/VL...</div><div><br></div><div>Then we can have an instance of this class for each vector type:</div>
<div><br></div><div><div>+def v16i32_info : X86VectorVTInfo<16, i32, VR512, "d">;</div><div>+def v8i64_info : X86VectorVTInfo<8, i64, VR512, "q”>;</div><div><br></div><div>And then we would pass these instead of the RC, VT, KRC, MRC, etc, to the classes/multiclasses. So instead of this:</div>
<div><br></div><div><div>-defm VALIGND : avx512_valign<<b>"d", VR512, VK16WM, GR16</b>, i512mem, <b>v16i32</b>, v16f32>,</div><div>- EVEX_V512, EVEX_CD8<32, CD8VF>;</div><div>-defm VALIGNQ : avx512_valign<<b>"q", VR512, VK8WM, GR8</b>, i512mem, <b>v8i64</b>, v8f64>,</div>
<div>- VEX_W, EVEX_V512, EVEX_CD8<64, CD8VF>;</div><div><br></div><div>We’ll have this (we can probably add the other args as well):</div><div><br></div><div><div>+defm VALIGND : avx512_valign1<<b>v16i32_info</b>, i512mem, v16f32>,</div>
<div>+ EVEX_V512, EVEX_CD8<32, CD8VF>;</div><div>+defm VALIGNQ : avx512_valign1<<b>v8i64_info</b>, i512mem, v8f64>,</div><div>+ VEX_W, EVEX_V512, EVEX_CD8<64, CD8VF>;</div>
<div><br></div><div>I didn’t get a chance to look at the VL stuff and see if this helps there or not. Robert, what do you think?</div><div><br></div></div></div></div></div></div></div></div></blockquote><div><br></div>
<div>
I agree that code such !cast<blah>(xxx#eltnr#yyy) becomes unreadable and welcome your idea to centralize common mapping.</div><div><br></div><div>If we choose this way, it could be easy to extend to AVX512VL. We can introduce something like this (assume that VTInfo is parent class for X86VectorVTInfo):</div>
<div><div>class AVX512VLVectorVTInfo<int EltNum, ValueType EltVT, string suffix = ""> {</div><div> VTInfo VTInfo512 = X86VectorVTInfo<EltNum, EltVT, VR512, suffix>;<br></div><div><div> VTInfo VTInfo256 = X86VectorVTInfo<!shr(EltNum,1), EltVT, VR256X, suffix>;</div>
</div><div> VTInfo VTInfo128 = X86VectorVTInfo<!shr(EltNum,2), EltVT, VR128X, suffix>;</div><div>}<br></div></div><div><br></div><div>and pass this class avx512_blah_vl multiclasses through argument.</div></div></div>
</div></blockquote><div><br></div></div></div><div>Excellent. That looks great to me. Why does VTInfo need to be different from X86VectorVTInfo?</div><div><br></div></div></div></div></blockquote><div><br></div><div><span style="font-family:arial,sans-serif;font-size:13px">I checked there is no necessity to make separate VTInfo parent class, ignore this naming.</span><br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>
<div></div><div>Anyhow, I’ll prepare a patch with this and you can take it from there.</div><div class=""><div><br></div></div></div></div></div></blockquote><div><br></div><div>Great! </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div><div class=""><div></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">
<div><div></div><div>Besides this infrastructure (let me know if you think it’s a good idea) we can also use an intermediate level of multiclass to “expand” derived arguments on a case-by-case basis. This multiclass will not do anything other than translate the input args into derived args using the nasty !cast, stringconcats and what not. Then both avx512_int_broadcast_reg_pat and avx512_int_broadcast_reg_pat_vl should be a lot cleaner and hopefully the nasty code is contained in the intermediate class.</div>
<div><br></div></div></div></blockquote><div><br></div><div>As I understand. in this way number of multiclasses in intermediate level equals to number of VL classes,doesn't it?</div></div></div></div></blockquote><div>
<br></div></div><div>I think I understand what you’re asking and the answer is yes. For each VL multiclass there is another multiclass which hides the mapping and is specific to that VL multiclass.</div><div class=""><br>
<blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div>
<div><div></div><div>As an alternative/extension to the previous idea we could also allow “local” definitions in multiclasses (currently you can only have def, defm and let in a multiclass). Then we could write something like:</div>
<div><br></div><div>multiclass blah<string elsz, ….> {</div><div> RegisterClass MRC = !cast<RegisterClass>(“GR” # elsz); // or even better = X86VectorVTInfo<elty, elsz>.MRC</div><div><br></div><div>and then you could use MRC to formulate the logic without the concat/cast getting in the way of clarity:</div>
<div><br></div><div><div><blockquote type="cite"><font face="Calibri"><span style="font-size:11pt"><div> defm : avx512_int_broadcast_reg_pat<InstName##"Z256", X86VBroadcast,</div><div> !cast<RegisterClass>("VK"##vsz256##"WM”),</div>
</span></font></blockquote><blockquote type="cite"><div><font face="Calibri"><span style="font-size:11pt"> VR256X, !cast<RegisterClass>("GR"##elsz),</span></font></div>
</blockquote><div><br></div></div><div>This would be the last line now:</div><br> VR256X, MRC,</div><div><br><blockquote type="cite"><div><font face="Calibri"><span style="font-size:11pt"><div>
!cast<ValueType>("v"##vsz256##"i"##elsz),</div><div> v8i32, !cast<ValueType>("i"##elsz), SubRI>;</div></span></font></div>
</blockquote><div><font face="Calibri"><span style="font-size:11pt"><br></span></font></div></div><div>I haven’t looked at how complicated this would be to add to tablegen.</div></div></div>
</div></blockquote><div><br></div><div>If we will use X86VectorVTInfo with this idea, I think it would be better to include local definitions in multiclasses like avx512_valign, avx512_binop_rm. There we can expand RC, VT and other arg and use them avx512_masking multiclass w/o casts.</div>
</div></div></div></blockquote><div><br></div></div><div>OK. Let’s do this. I’ll post a patch tomorrow with X86VectorVTInfo and a sample application in AVX512_valign. I think that that would be an improvement over what we have today. Then I keep looking at tablegen to implement local definitions.</div>
<div><br></div><div>Thanks for your feedback!</div><span class=""><font color="#888888"><div><br></div><div>Adam</div></font></span><div><div class="h5"><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote">
<div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">
<div><div><div><div><div><div><span><font color="#888888"><div>Adam</div><div><br></div></font></span></div></div></div></div><div><div><br><blockquote type="cite"><div style="font-family:Helvetica;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<font face="Calibri"><span style="font-size:11pt"><div> </div><div>Thank you.</div><div> </div><div>multiclass avx512_int_broadcast_reg_pat<string InstName, SDNode OpNode,</div><div> RegisterClass KRC, RegisterClass DestRC,</div>
<div> RegisterClass SrcRC, ValueType vt,</div><div> ValueType ivt, ValueType svt,</div><div> string SubRI = ""> {</div>
<div> def : Pat<(vt (OpNode (svt SrcRC:$src))),</div><div> (!cast<Instruction>(InstName##"r")</div><div> !if(!eq(SubRI, ""), (svt SrcRC:$src),</div><div> (SUBREG_TO_REG (i32 0), SrcRC:$src, !cast<SubRegIndex>(SubRI))))>;</div>
<div> let AddedComplexity = 30 in {</div><div> def : Pat<(vt (vselect KRC:$mask, (OpNode (svt SrcRC:$src)),</div><div> DestRC:$src0)),</div><div> (!cast<Instruction>(InstName##"rk") DestRC:$src0, KRC:$mask,</div>
<div> !if(!eq(SubRI, ""), (svt SrcRC:$src),</div><div> (SUBREG_TO_REG (i32 0), SrcRC:$src, !cast<SubRegIndex>(SubRI))))>;</div><div> </div><div> def : Pat<(vt (vselect KRC:$mask, (OpNode (svt SrcRC:$src)),</div>
<div> (vt (bitconvert (ivt immAllZerosV))))),</div><div> (!cast<Instruction>(InstName##"rkz") KRC:$mask,</div><div> !if(!eq(SubRI, ""), (svt SrcRC:$src),</div>
<div> (SUBREG_TO_REG (i32 0), SrcRC:$src, !cast<SubRegIndex>(SubRI))))>;</div><div> }</div><div>}</div><div> </div><div>multiclass avx512_int_broadcast_reg_pat_vl<string InstName, string elsz,</div>
<div> string vsz, string vsz256,</div><div> string vsz128, Predicate prd,</div><div> string SubRI = ""> {</div>
<div> let Predicates = [prd] in</div><div> defm : avx512_int_broadcast_reg_pat<InstName##"Z", X86VBroadcast,</div><div> !cast<RegisterClass>("VK"##vsz##"WM"),</div>
<div> VR512, !cast<RegisterClass>("GR"##elsz),</div><div> !cast<ValueType>("v"##vsz##"i"##elsz),</div><div> v16i32, !cast<ValueType>("i"##elsz), SubRI>;</div>
<div> let Predicates = [prd, HasVLX] in {</div><div> defm : avx512_int_broadcast_reg_pat<InstName##"Z256", X86VBroadcast,</div><div> !cast<RegisterClass>("VK"##vsz256##"WM"),</div>
<div> VR256X, !cast<RegisterClass>("GR"##elsz),</div><div> !cast<ValueType>("v"##vsz256##"i"##elsz),</div><div>
v8i32, !cast<ValueType>("i"##elsz), SubRI>;</div><div> defm : avx512_int_broadcast_reg_pat<InstName##"Z128", X86VBroadcast,</div><div> !cast<RegisterClass>("VK"##vsz128##"WM"),</div>
<div> VR128X, !cast<RegisterClass>("GR"##elsz),</div><div> !cast<ValueType>("v"##vsz128##"i"##elsz),</div><div>
v4i32, !cast<ValueType>("i"##elsz), SubRI>;</div><div> }</div><div>}</div><div> </div><div>defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTBr", "8", "64", "32", "16",</div>
<div> HasBWI, "sub_8bit">;</div><div>defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTWr", "16", "32", "16", "8",</div>
<div> HasBWI, "sub_16bit">;</div><div>defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTDr", "32", "16", "8", "4",</div>
<div> HasAVX512>;</div><div>defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTQr", "64", "8", "4", "2",</div><div> HasAVX512>, VEX_W;</div>
<div> </div><ul style="margin:0px;padding-left:36pt"><font face="Times New Roman" size="3" color="#31849B"><span style="font-size:12pt"><li><b><i>Elena</i></b></li></span></font></ul><div> </div><div> </div><div> </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></div></blockquote></div></div></div><br></div></div><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><br>
<br></blockquote></div><br></div></div>
</blockquote></div></div></div><br></div></div></blockquote></div><br></div></div>