<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Elena and Robert,<div><br><div><div>On Aug 13, 2014, at 12:01 AM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com">elena.demikhovsky@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><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; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><font face="Calibri" size="2"><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>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>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>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><blockquote type="cite"><div><font face="Calibri" size="2"><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></div></blockquote><blockquote type="cite"><div><font face="Calibri" size="2"><span style="font-size: 11pt;"><div> VR256X, !cast<RegisterClass>("GR"##elsz),</div></span></font></div></blockquote><div><br></div><div>This would be the last line now:</div><br> VR256X, MRC,</div><div><br><blockquote type="cite"><div><font face="Calibri" size="2"><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><div><font face="Calibri" size="2"><span style="font-size: 11pt;"><div><br></div></span></font></div></div></div><div>I haven’t looked at how complicated this would be to add to tablegen.</div><div><br></div><div>Adam</div><div><br></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; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><font face="Calibri" size="2"><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><br></div></body></html>