Complex patterns in .td file - a question
Robert Khasanov
rob.khasanov at gmail.com
Thu Aug 14 10:02:01 PDT 2014
Hi Elena and Adam,
2014-08-13 22:20 GMT+04:00 Adam Nemet <anemet at apple.com>:
> Hi Elena and Robert,
>
> On Aug 13, 2014, at 12:01 AM, Demikhovsky, Elena <
> elena.demikhovsky at intel.com> wrote:
>
> Hi Nadav, Craig, Adam,
>
> 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?
> Or on some stage it becomes unreadable?
>
>
> 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.
>
> 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.
>
> 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.
>
> 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.:
>
> +// Group template arguments that can be derived from the vector type
> (EltNum x
> +// EltVT). These are things like RegisterClass, the corresponding mask
> RC,
> +// etc. The idea is to pass one of these as the template argument rather
> than
> +// the individual arguments.
> +class X86VectorVTInfo<int EltNum, ValueType EltVT, RegisterClass rc =
> VR512,
> + string suffix = ""> {
> + RegisterClass RC = rc;
> + RegisterClass KRC = !cast<RegisterClass>("VK"##EltNum##"WM");
> + RegisterClass MRC = !cast<RegisterClass>("GR"##EltNum);
> + string Suffix = suffix;
> + ValueType VT = !cast<ValueType>("v"##EltNum##EltVT);
> +}
>
> I am sure there is bunch more “field” to be added for BW/DQ/VL...
>
> Then we can have an instance of this class for each vector type:
>
> +def v16i32_info : X86VectorVTInfo<16, i32, VR512, "d">;
> +def v8i64_info : X86VectorVTInfo<8, i64, VR512, "q”>;
>
> And then we would pass these instead of the RC, VT, KRC, MRC, etc, to the
> classes/multiclasses. So instead of this:
>
> -defm VALIGND : avx512_valign<*"d", VR512, VK16WM, GR16*, i512mem,
> *v16i32*, v16f32>,
> - EVEX_V512, EVEX_CD8<32, CD8VF>;
> -defm VALIGNQ : avx512_valign<*"q", VR512, VK8WM, GR8*, i512mem, *v8i64*,
> v8f64>,
> - VEX_W, EVEX_V512, EVEX_CD8<64, CD8VF>;
>
> We’ll have this (we can probably add the other args as well):
>
> +defm VALIGND : avx512_valign1<*v16i32_info*, i512mem, v16f32>,
> + EVEX_V512, EVEX_CD8<32, CD8VF>;
> +defm VALIGNQ : avx512_valign1<*v8i64_info*, i512mem, v8f64>,
> + VEX_W, EVEX_V512, EVEX_CD8<64, CD8VF>;
>
> 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?
>
>
I agree that code such !cast<blah>(xxx#eltnr#yyy) becomes unreadable and
welcome your idea to centralize common mapping.
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):
class AVX512VLVectorVTInfo<int EltNum, ValueType EltVT, string suffix = "">
{
VTInfo VTInfo512 = X86VectorVTInfo<EltNum, EltVT, VR512, suffix>;
VTInfo VTInfo256 = X86VectorVTInfo<!shr(EltNum,1), EltVT, VR256X, suffix>;
VTInfo VTInfo128 = X86VectorVTInfo<!shr(EltNum,2), EltVT, VR128X, suffix>;
}
and pass this class avx512_blah_vl multiclasses through argument.
> 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.
>
>
As I understand. in this way number of multiclasses in intermediate level
equals to number of VL classes,doesn't it?
> 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:
>
> multiclass blah<string elsz, ….> {
> RegisterClass MRC = !cast<RegisterClass>(“GR” # elsz); // or even
> better = X86VectorVTInfo<elty, elsz>.MRC
>
> and then you could use MRC to formulate the logic without the concat/cast
> getting in the way of clarity:
>
> defm : avx512_int_broadcast_reg_pat<InstName##"Z256", X86VBroadcast,
>
> !cast<RegisterClass>("VK"##vsz256##"WM”),
>
> VR256X,
> !cast<RegisterClass>("GR"##elsz),
>
>
> This would be the last line now:
>
> VR256X, MRC,
>
>
> !cast<ValueType>("v"##vsz256##"i"##elsz),
> v8i32, !cast<ValueType>("i"##elsz),
> SubRI>;
>
>
> I haven’t looked at how complicated this would be to add to tablegen.
>
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.
> Adam
>
>
>
> Thank you.
>
> multiclass avx512_int_broadcast_reg_pat<string InstName, SDNode OpNode,
> RegisterClass KRC, RegisterClass
> DestRC,
> RegisterClass SrcRC, ValueType vt,
> ValueType ivt, ValueType svt,
> string SubRI = ""> {
> def : Pat<(vt (OpNode (svt SrcRC:$src))),
> (!cast<Instruction>(InstName##"r")
> !if(!eq(SubRI, ""), (svt SrcRC:$src),
> (SUBREG_TO_REG (i32 0), SrcRC:$src,
> !cast<SubRegIndex>(SubRI))))>;
> let AddedComplexity = 30 in {
> def : Pat<(vt (vselect KRC:$mask, (OpNode (svt SrcRC:$src)),
> DestRC:$src0)),
> (!cast<Instruction>(InstName##"rk") DestRC:$src0, KRC:$mask,
> !if(!eq(SubRI, ""), (svt SrcRC:$src),
> (SUBREG_TO_REG (i32 0), SrcRC:$src,
> !cast<SubRegIndex>(SubRI))))>;
>
> def : Pat<(vt (vselect KRC:$mask, (OpNode (svt SrcRC:$src)),
> (vt (bitconvert (ivt
> immAllZerosV))))),
> (!cast<Instruction>(InstName##"rkz") KRC:$mask,
> !if(!eq(SubRI, ""), (svt SrcRC:$src),
> (SUBREG_TO_REG (i32 0), SrcRC:$src,
> !cast<SubRegIndex>(SubRI))))>;
> }
> }
>
> multiclass avx512_int_broadcast_reg_pat_vl<string InstName, string elsz,
> string vsz, string vsz256,
> string vsz128, Predicate prd,
> string SubRI = ""> {
> let Predicates = [prd] in
> defm : avx512_int_broadcast_reg_pat<InstName##"Z", X86VBroadcast,
> !cast<RegisterClass>("VK"##vsz##"WM"),
> VR512, !cast<RegisterClass>("GR"##elsz),
> !cast<ValueType>("v"##vsz##"i"##elsz),
> v16i32, !cast<ValueType>("i"##elsz),
> SubRI>;
> let Predicates = [prd, HasVLX] in {
> defm : avx512_int_broadcast_reg_pat<InstName##"Z256", X86VBroadcast,
>
> !cast<RegisterClass>("VK"##vsz256##"WM"),
> VR256X,
> !cast<RegisterClass>("GR"##elsz),
>
> !cast<ValueType>("v"##vsz256##"i"##elsz),
> v8i32, !cast<ValueType>("i"##elsz),
> SubRI>;
> defm : avx512_int_broadcast_reg_pat<InstName##"Z128", X86VBroadcast,
>
> !cast<RegisterClass>("VK"##vsz128##"WM"),
> VR128X,
> !cast<RegisterClass>("GR"##elsz),
>
> !cast<ValueType>("v"##vsz128##"i"##elsz),
> v4i32, !cast<ValueType>("i"##elsz),
> SubRI>;
> }
> }
>
> defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTBr", "8", "64", "32",
> "16",
> HasBWI, "sub_8bit">;
> defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTWr", "16", "32", "16",
> "8",
> HasBWI, "sub_16bit">;
> defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTDr", "32", "16", "8",
> "4",
> HasAVX512>;
> defm : avx512_int_broadcast_reg_pat_vl<"VPBROADCASTQr", "64", "8", "4",
> "2",
> HasAVX512>, VEX_W;
>
>
> - *Elena*
>
>
>
>
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140814/bb7ff2cd/attachment.html>
More information about the llvm-commits
mailing list