[PATCH] D145872: TableGen: Let expressions available to list subscripts and list slices

Sat Apr 1 23:53:37 PDT 2023

chapuni added a comment.

In D145872#4235592 <https://reviews.llvm.org/D145872#4235592>, @barannikov88 wrote:

> I like the idea, but the implementation looks more like a PoC than a complete solution.
> There is clearly an overlap between RangeList and SliceElements and I don't see why they should be distinct.

In fact I started this as PoC at first but I don't suppose this is still PoC..
When I dropped `VarListElementInit`, I thought I achieved a step of migrations.
I understand you feel this as PoC, though.

I think the new semantics could be applied also to list initializer.
I haven't done since it didn't prevent my other changes, for list manipulations.
OTOH, I didn't change `BitSlice`, because I thought it is satisfied with fixed length array.

I am still dubious I could reuse existing codes. They assume each value could be resolved in-place.

I propose this for the series of D145937 <https://reviews.llvm.org/D145937>. I supposed this was essential to them at first,
but I have noticed list indexing could be hacked with `!listconcat` `!foldl`, etc.
I could detach this out of D145937 <https://reviews.llvm.org/D145937>, if this would be supposed not ready for landing.

================
Comment at: llvm/include/llvm/TableGen/Record.h:852
+    LISTSLICE,
+    RANGEC,
     STRCONCAT,
----------------
barannikov88 wrote:
> chapuni wrote:
> > barannikov88 wrote:
> > > What is C in RANGEC? Please give a descriptive name.
> > It is a retronym "RANGE (close interval)" to D145871 (`RANGE` as half-open interval).
> > Could I revise both names?
> RANGE is fine in D145871 because it directly corresponds to a bang operator.
> There is no operator in the language that corresponds to any of LISTELEM/LISTSLICE/RANGEC.
> 
> I wish I new TableGen better to suggest a way of properly handling this...
> It feels like it would be better to update the deleted code instead of deleting it.
> 
I have taken `BinOpInit` for them rather than a new `TypedInit` derivative, since they can be treated as operators with two values except that they are not bang operators.
I suppose it would make sense if `OpInit` tree might be handled as the expr tree (like other languages).

I could rewrite them as derivatives of `TypeInit` if `OpInit` should be bang operator as principle. Then I can guess they would be code duplication. I think it would not be smart.

@Paul-C-Anagnostopoulos @nhaehnle Thoughts? Could we expand `OpInit` for non-bang-ops?

================
Comment at: llvm/lib/TableGen/TGParser.cpp:746
+  }
+  case tgtok::IntVal: { // Deprecated "-num"
+    auto i = -Lex.getCurIntVal();
----------------
barannikov88 wrote:
> chapuni wrote:
> > barannikov88 wrote:
> > > I don't get it how IntVal implies `-`. Could you explain?
> > See L754 in `TGParser::ParseRangePiece()`.
> > Could I update this as `// Expects "-num" (deprecated)` ?
> I think it is because `1 - -2` parses the same as `1 2` because `--2 == 2`.
> 
I suppose it to interpret `0-7`. Tokenized as `0` and `-7`.
See also;
https://llvm.org/docs/TableGen/ProgRef.html#id10
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/ARM/ARMScheduleA9.td#L2109

`1 - -2` is tokenized as `num:1` `minus` `num:-2`.
`--2` as `minus` `num:-2`.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145872/new/

https://reviews.llvm.org/D145872