[PATCH] D64953: GlobalISel: Support physical register inputs in patterns

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:832
 
+  /// A map of anonymouss physical registser operands defined by the matchers
+  /// that may be referenced by the renderers.
----------------
dsanders wrote:
> registser -> register
This typo is still there, I also missed the `anonymouss` typo on the first read


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:2319
 
+class CopyPhysRegRenderer : public OperandRenderer {
+protected:
----------------
dsanders wrote:
> Could you add a description comment to this class?
This still needs a description comment


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:3881-3882
 
+    // FIXME: Is this necessary? Seems to avoid problems with interactions with
+    // special WZR handling.
     if (ChildRec->isSubClassOf("Register")) {
----------------
dsanders wrote:
> I don't think I understand this comment, it doesn't really give enough context to know why the necessity is in doubt or the problems it avoids
This is the only bit that's holding me back from giving a conditional LGTM as I don't understand the comment and therefore I'm not sure if it's an issue. Could you elaborate on what the problems are and why the necessity is in doubt?


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

https://reviews.llvm.org/D64953