[PATCH] D37445: [globalisel][tablegen] Map ld and st to G_LOAD and G_STORE. NFC

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

The added predicate part looks fine, but I didn't get why we need to change CodeGenInstruction into record just yet.



================
Comment at: include/llvm/Target/GlobalISel/SelectionDAGCompat.td:83
+// isSignExtLoad require that this is not a perfect 1:1 mapping since a
+// sign-extending load is (G_SEXT (G_LOAD x)) in GlobalISel.
+def : GINodeEquiv<G_LOAD, ld> { let CheckMMOIsNonAtomic = 1; }
----------------
Although I get the comment, I don't see how this affect the following definition.
I.e., what in the following definition changed compared to the surrounding ones to take this into account?


================
Comment at: include/llvm/Target/GlobalISel/SelectionDAGCompat.td:88
+// isTruncStore require that this is not a perfect 1:1 mapping since a
+// truncating store is (G_TRUNCATE (G_LOAD x)) in GlobalISel.
+def : GINodeEquiv<G_STORE, st> { let CheckMMOIsNonAtomic = 1; }
----------------
Typo (G_STORE (G_TRUNCATE x))


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1996
   /// This is defined using 'GINodeEquiv' in the target description.
-  DenseMap<Record *, const CodeGenInstruction *> NodeEquivs;
+  DenseMap<Record *, Record *> NodeEquivs;
 
----------------
I don't get why this change is required yet. G_LOAD and G_STORE records are still CodeGenInstruction, aren't they? (I haven't checked)


https://reviews.llvm.org/D37445