[PATCH] D74796: [tablegen] Add !iscomplete(Value)

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: llvm/test/TableGen/isa-consistency.td:33
+
+// So far so good, but it doesn't work without the field assignment.
+// CHECK: def Z4 {
----------------
nhaehnle wrote:
> That comment is out of place now, isn't it?
Z4's ret still fails to resolve like it did before this change. The check is what I'd expect from it but we actually get:
```
llvm-project/llvm/test/TableGen/isa-consistency.td:37:1: error: Initializer of 'ret' in 'Z4' could not be fully resolved: !isa<int>(?)
```
I'm not sure what the cause of the inconsistency is but I believe the type of the field `A` is being accounted for in a way that `?` and `!cast<int>(?)` somehow don't.

Overall, this file is documenting the strange behaviour of isa<> that I ran into while trying to distinguish `?` from `0`. I haven't changed that behaviour at all. I've added a comment to the top of the test to explain that


================
Comment at: llvm/test/TableGen/isa-consistency.td:50
+
+// Yet it does work on bits<>, even though it didn't work on bit.
+// CHECK: def Z6 {
----------------
nhaehnle wrote:
> Same.
It's still the case that the `!cast<bit>(?)` in Z5 doesn't work but the `{?, ?}` in Z6 does.
We get the same error as Z4 but with a different expression


================
Comment at: llvm/test/TableGen/isa-consistency.td:59-60
+
+// Stranger still, if you extract an unset bit from a bits<> it still works even
+// though bit didn't.
+// CHECK: def Z7 {
----------------
nhaehnle wrote:
> Same.
It's still the case that the `!cast<int>(?)` in Z5 doesn't work but the `{?, ?}{0}` in Z7 does. This one appears to be due to it having the type `bits<1>` instead of `bit` which is a bit surprising but does make some sense.


================
Comment at: llvm/test/TableGen/iscomplete.td:159-161
+  // This one however, caused an assertion because BitC's value an UnsetInit
+  // and !eq() can only accept TypedInit's.
+  // bit D = !if(!eq(Inputs.BitC, 0), 1, 0);
----------------
nhaehnle wrote:
> It doesn't assert, does it? Rather it produces an error message.
This bit is talking about the original bug where it was asserting. I've added a bit mentioning that it now errors out properly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74796