[PATCH] D37457: [globalisel][tablegen] Implement unindexed load, non-extending load, and MemVT checks

Daniel Sanders via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 12 14:21:26 PDT 2017


dsanders added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/InstructionSelector.h:124
+  /// - SizeInBits - The size of the pointer value in bits.
+  GIM_CheckPointerToAny,
   /// Check the register bank for the specified operand
----------------
qcolombet wrote:
> Doesn't the pointer size depend on the address space?
> Thus, given we specific the size this is not really any address space, isn't it?
> 
> Basically what I am saying is either don't put the size or change the name of the operator.
> BTW, shouldn't we already check the size when checking the type?
Hmm, I agree that it makes sense for pointer sizes to vary between address spaces. However, iPTR doesn't do anything about address spaces so I don't think SelectionDAG rules account for it. I'll have another look and see what I can find out.

> BTW, shouldn't we already check the size when checking the type?

This opcode is used instead of GIM_CheckType for pointers. Pointers are a bit tricky since we have to call a target hook to find out the size of a pointer so we don't know the type until run-time.


================
Comment at: include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h:247
     }
-
+    case GIM_CheckPointerToAny: {
+      int64_t InsnID = MatchTable[CurrentIdx++];
----------------
qcolombet wrote:
> Could you add a tablegen test case for this operator?
Ok


https://reviews.llvm.org/D37457





More information about the llvm-commits mailing list