[PATCH] D64338: TableGen: Add address space to matchers

Fri Jul 12 16:55:17 PDT 2019

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM

> I'm also not sure if there's a better way to encode multiple address spaces in the table, rather than putting the number to expect.

It amounts to the same thing but they could be LLT's like the ones defined in llvm/Target/GlobalISel/Target.td. If each target defines it's own pX's then it does at least constrain it to the expected address spaces.

================
Comment at: test/TableGen/address-space-patfrags.td:20
+  let AddressSpaces = [ 999 ];
+  let IsLoad = 1; // FIXME: Can this be inferred?
+  let MemoryVT = i32;
----------------
Regarding the FIXME. The importer currently operates after PatFrag expansion which is too late to infer it. I expect the code that does the expansion could infer it but I haven't dug into that.

================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:245-248
+        if (First)
+          First = false;
+        else
+          OS << ',';
----------------
Other places in this file achieve this with:
```
StringRef Separator = "";
for (...) {
  OS << Separator << ...;
  Separator = ", ";
}
```
I think it ends up a bit neater but I don't mind either way

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

https://reviews.llvm.org/D64338