[LLVMdev] Enhancing TableGen

[David A. Greene]

Jakob Stoklund Olesen <jolesen at apple.com> writes:

> I think the for loops have merit, but not the way you want to use them.
>
> Some target descriptions have many sequential definitions, for example PowerPC/PPCRegisterInfo.td:
>
> // Vector registers
> def V0  : VR< 0,  "v0">, DwarfRegNum<[77, 77]>;
> def V1  : VR< 1,  "v1">, DwarfRegNum<[78, 78]>;
> ...
> def V30 : VR<30, "v30">, DwarfRegNum<[107, 107]>;
> def V31 : VR<31, "v31">, DwarfRegNum<[108, 108]>;
>
> Such 'embarrassingly sequential' definitions have no value, and I
> would welcome a for-loop construct that can create them. I think this
> was the primary motivation behind Che-Liang's proposal.

Ok.  We agree here.

> I do NOT want to factor out redundancy from instruction definitions like this:
>
> multiclass PTX_FLOAT_3OP<string opcstr> {
>   def rr32 : InstPTX<(outs RegF32:$d),
>                      (ins RndMode:$r, RegF32:$a, RegF32:$b),
>                      !strconcat(opcstr, "$r.f32\t$d, $a, $b"), []>;
>   def ri32 : InstPTX<(outs RegF32:$d),
>                      (ins RndMode:$r, RegF32:$a, f32imm:$b),
>                      !strconcat(opcstr, "$r.f32\t$d, $a, $b"), []>;
>   def rr64 : InstPTX<(outs RegF64:$d),
>                      (ins RndMode:$r, RegF64:$a, RegF64:$b),
>                      !strconcat(opcstr, "$r.f64\t$d, $a, $b"), []>;
>   def ri64 : InstPTX<(outs RegF64:$d),
>                      (ins RndMode:$r, RegF64:$a, f64imm:$b),
>                      !strconcat(opcstr, "$r.f64\t$d, $a, $b"), []>;
> }
>
> As repeated many times on this thread, the most common operation that
> a .td file must support is looking up an instruction and figuring out
> what its properties are and where they came from.

Ok.  What properties are most important to look up?  I have found it
really easy to just run "tblgen X86.td" and look at the output.  That's
really the canonical definition, right?  Yes, I know it's an extra step
but if one really can't puzzle out where an instruction came from, it's
an easy one.

To me, this:

          !strconcat(opcstr, "$r.f32\t$d, $a, $b"), []>;
          !strconcat(opcstr, "$r.f32\t$d, $a, $b"), []>;
          !strconcat(opcstr, "$r.f64\t$d, $a, $b"), []>;
          !strconcat(opcstr, "$r.f64\t$d, $a, $b"), []>;

is dangerously redundant.  It's the patterns that are hardest to write
so I'd rather get them right once or a few times rather than get them
wrong in lots of places and have to go hunt and fix them.

This could be mitigated somewhat by doing something like this:

class binary_pattern<string opcstr, string type> {
  string pattern = !strconcat(opcstr, "$r."#type#"\t$d, $a, $b");
}

multiclass PTX_FLOAT_3OP<string opcstr> {
  def rr32 : InstPTX<(outs RegF32:$d),
                     (ins RndMode:$r, RegF32:$a, RegF32:$b),
                     binary_pattern<opcstrm "f32">.pattern, []>;
  def ri32 : InstPTX<(outs RegF32:$d),
                     (ins RndMode:$r, RegF32:$a, f32imm:$b),
                     binary_pattern<opcstrm "f32">.pattern, []>;
  def rr64 : InstPTX<(outs RegF64:$d),
                     (ins RndMode:$r, RegF64:$a, RegF64:$b),
                     binary_pattern<opcstrm "f64">.pattern, []>;
  def ri64 : InstPTX<(outs RegF64:$d),
                     (ins RndMode:$r, RegF64:$a, f64imm:$b),
                     binary_pattern<opcstrm "f64">.pattern, []>;
}

Would something like that be acceptable?

This is even more important for many of the Pat<> patterns which tend to
be quite complex.  I have found several cases where Pat<> patterns
existed for some types of, say, add operation but not others, for no
apparent reason other than someone added one and forgot to add the
others.

Pat<> patterns is the whole reason multidefs got developed in the first
place.  I wanted to define Pat<> patterns right in the same place the
"ordinary" patterns got defined.  There's much less chance of missing
something or writing inconsistent patterns that way.

For x86, I also find the encoding details at the top level very
distracting.  I was confused for months about what they meant and I
didn't care.  I just wanted to generate assembly code.  SSE/AVX has a
very regular encoding and I think it would be valuable to express that a
bit more concisely.  Some of that is done now but I think it could be
better.  I have a less well-defined idea of what that looks like as it's
one of the things I don't particularly like about what I developed here.

                        -Dave