TableGen: Introduce `!range` operator for half-opened interval

[chapuni]

Summary

!range(a, b) generates a list [a,b). a is optional and 0 by default.

• !range(-1, 4) generates [-1, 0, 1, 2, 3]
• !range(4) generates [0, 1, 2, 3]
• !range(2, 2) generates []<list<int>>

!range(list) is equivalent to !range(0, !size(list)).

[chapuni]

• Fixup

[chapuni]

• Add a test case

[Paul-C-Anagnostopoulos]

I'm tempted to suggest that this bang operator should be called !rangelist. The name !range suggests a pair of integers or a sequence of integers not encapsulated in a list. Add a couple of other reviewers so we can get a second opinion.

Add some simpler initial tests to range-op.td, such as the examples you gave in the summary.

[chapuni]

Thanks for your comment.

!range(m, n) and !range(n) come from Python.
!range(list) is just a syntax sugar.

I will accept even if a minimal functionality (m, n) would be landed. Do you think I should drop other syntax sugars?

Add a couple of other reviewers so we can get a second opinion.

I will happy if you could nominate anyone, since you ask a second opinion.

Add some simpler initial tests to range-op.td, such as the examples you gave in the summary.

I didn't think I could add simpler examples to the test. I will do if it would be better.

[Paul-C-Anagnostopoulos]

I've added two reviewers. Could I get a second opinion on the name of this bang operator?

I think a few simpler tests first is a good idea. In particular, it helps a future developer understand what is going on.

[chapuni]

• Update a test more descriptive

[Paul-C-Anagnostopoulos]

Nice new tests. Thanks!

[nhaehnle]

!range makes sense to me because I also see the analogy to Python's range.

I have some minor notes inline, apart from that the code looks good to me.

llvm/lib/TableGen/TGParser.cpp

1509

You're relying here on the cast always being successful (which makes sense), so you should use cast instead of dyn_cast. (This has the benefit of raising an assertion if the cast fails.)

1510

Using a switch for this instead of an if/else chain is a bit unorthodox, but I suppose we can live with it.

1526

Also here: use cast instead of dyn_cast

[Paul-C-Anagnostopoulos]

I see the analogy to Python's range(), except that it returns a sequence of numbers, not a list. But I'm not really too worried about this.

[chapuni]

• Use cast<ty> rather than dyn_cast<ty> when <ty> is assumed.
• Add a testcase.
• Other cleanups

[chapuni]

llvm/lib/TableGen/TGParser.cpp

1509

I have misunderstood cast<ty> as it would crash with incompatible type.
Thanks for pointing out.

1510

I avoided duplicate usages of InitList.size(), even if they were optimized out.

[arsenm]

llvm/lib/TableGen/TGParser.cpp

1527

I still see separate isa and cast?

llvm/test/TableGen/range-op-fail.td

27

Test with mixed integer and not-integer

[chapuni]

• Make Arg1 consistent

[chapuni]

@arsenm Thanks for your reviews!

llvm/lib/TableGen/TGParser.cpp

1527

The inner cast was for TypedInit::getType(). Anyways, I have rewritten more consistent.

llvm/test/TableGen/range-op-fail.td

27

This detects that 1st arg is not integer.
OTOH, I suppose ERR1 would cover if 1st arg was integer.

[arsenm]

llvm/lib/TableGen/Record.cpp

1206

Can early break and reduce indentation

llvm/test/TableGen/range-op-fail.td

27

Right but you aren't testing the error is produced for each position individually

[chapuni]

• Use early break
• range-op-fail.td: Let unnumbered
• Add ERR_UNEXPECTED_TYPE(s)

[chapuni]

llvm/lib/TableGen/Record.cpp

1206

I followed old code style around. Updated.
(Frankly said, I started from copypaste)

llvm/test/TableGen/range-op-fail.td

27

Put string for each position.
(and rename cases as unnumbered)

[chapuni]

Ping. Any other issues?

[Paul-C-Anagnostopoulos]

I don't see the documentation for this new operator. Did it go in already?

[chapuni]

@Paul-C-Anagnostopoulos I suppose ProgRef.rst would satisfy. Would I miss anything?

[Paul-C-Anagnostopoulos]

Yes, that's the file. You need to add the name to the list in section 1.3.3 and the description to section 1.10.

[chapuni]

• Rebase
• ProgRef.rst: Update bang ops index

[chapuni]

@Paul-C-Anagnostopoulos Updated. Okay?

[Paul-C-Anagnostopoulos]

I can't figure out what is going on here. It doesn't look like you added a description of !range.

[chapuni]

llvm/docs/TableGen/ProgRef.rst

1780–1787

@Paul-C-Anagnostopoulos Is it insufficient or improper?

[Paul-C-Anagnostopoulos]

llvm/docs/TableGen/ProgRef.rst

1780–1787

Say: The result is ...
If *a* `>=` *b*, then the result is ...

[chapuni]

llvm/docs/TableGen/ProgRef.rst

1780–1787

How about them?

• Omit the sentence "Its result is...", since it is obvious.
• Revise the last sentence as your suggestion.

[Paul-C-Anagnostopoulos]

llvm/docs/TableGen/ProgRef.rst

1780–1787

I would keep both sentences about the results.

[chapuni]

• ProgRef.rst: Revise the description

[foad]

Late naming suggestion: !iota

But I am also fine with !range