Page MenuHomePhabricator

[TableGen] Introduce !listsplat 'binary' operator
ClosedPublic

Authored by lebedev.ri on Apr 6 2019, 1:29 PM.

Details

Summary
``!listsplat(a, size)``
    A list value that contains the value ``a`` ``size`` times.
    Example: ``!listsplat(0, 2)`` results in ``[0, 0]``.

The X86ScheduleBdVer2.td change highlights the use-case.

This is a little bit controversial because unlike every other binary operator
the types aren't identical.

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri created this revision.Apr 6 2019, 1:29 PM

Not objecting to this feature. Just asking a question.

lib/Target/X86/X86ScheduleBdVer2.td
214 ↗(On Diff #194029)

Doesn’t WriteRes already treat an empty list as 1 cycle per port?

lebedev.ri marked 2 inline comments as done.Apr 6 2019, 1:45 PM
lebedev.ri added inline comments.
lib/Target/X86/X86ScheduleBdVer2.td
214 ↗(On Diff #194029)

The problem is that you can't concatenate an empty list (implied all-ones) with non-empty list here.
The result will be the non-empty list, and it won't match the length of the ExePorts list.

lebedev.ri updated this revision to Diff 194032.Apr 6 2019, 1:48 PM
lebedev.ri marked an inline comment as done.

Streamline

lebedev.ri marked an inline comment as done.Apr 6 2019, 2:07 PM
lebedev.ri added inline comments.
lib/Target/X86/X86ScheduleBdVer2.td
214 ↗(On Diff #194029)

Or, to actually answer your question, the problems begin when LoadRes != 1 here,
which is the case in PdWriteResYMMPair, and more importantly i think it will be the case for PdWriteResExPair

I wonder if !listfill might be a better name? I feel like splat has a meaning to those of us who have worked on vectors within LLVM IR, but may not be meaningful right away to other people?

lib/Target/X86/X86ScheduleBdVer2.td
214 ↗(On Diff #194029)

Ok that makes sense.

I wonder if !listfill might be a better name? I feel like splat has a meaning to those of us who have worked on vectors within LLVM IR, but may not be meaningful right away to other people?

Normally std::fill() sets all the specified elements to the specified value.
To me, this isn't the case here, while we do fill the list, we don't 'fill the list', since we don't get any list..
That is at least how i see this. I totall can change it if other reviewers also agree that it would be better.

lebedev.ri marked an inline comment as done.Apr 6 2019, 11:37 PM

Thanks for this. Its good you show a use-case but probably the use case can be a separate commit.

lib/TableGen/TGParser.cpp
1154 ↗(On Diff #194032)

'output type' ? Would that be a bit confusing to user. I can't think of a better option though since the context (assignment?) won't be known.

lebedev.ri marked an inline comment as done.Apr 9 2019, 2:39 PM

Thanks for this. Its good you show a use-case but probably the use case can be a separate commit.

I absolutely can split that up during commit.

lib/TableGen/TGParser.cpp
1154 ↗(On Diff #194032)

IIRC this is checking for

class X<int a, int b> {
  str x = !listsplat(a, b);
}
def DX00 : X<0, 0>;

We know the type !listsplat is supposed to return since we know the LHS of the assignment.
I'm not sure how to spell this out best.

Thanks for this.

Thank you for looking at this!

javed.absar accepted this revision.Apr 10 2019, 10:46 AM
This revision is now accepted and ready to land.Apr 10 2019, 10:46 AM

Alright, thank you for the review!

Closed by commit rL358117: [TableGen] Introduce !listsplat 'binary' operator (authored by lebedevri, committed by ). · Explain WhyApr 10 2019, 11:25 AM
This revision was automatically updated to reflect the committed changes.