[PATCH] D22373: [GlobalISel] Introduce a simple instruction selector.

Ahmed Bougacha via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 13:50:20 PDT 2016


ab added inline comments.

================
Comment at: lib/CodeGen/MachineRegisterInfo.cpp:60
@@ -59,3 +59,3 @@
     return NewRC;
   if (NewRC->getNumRegs() < MinNumRegs)
     return nullptr;
----------------
qcolombet wrote:
> Actually what I had in mind was to get the size from the type with, like you said, querying the MI for the def size (via getUniqueDef). However, we would still need to access this information from the vreg.
> As having the type on the vreg, no, that does not work. Vregs can change type based on their uses (like and s64, add <2 x s32>).
> 
> But yeah, long term, this code shouldn't be needed. Add a comment about that?
> Actually what I had in mind was to get the size from the type with, like you said, querying the MI for the def size (via getUniqueDef). However, we would still need to access this information from the vreg.
> As having the type on the vreg, no, that does not work. Vregs can change type based on their uses (like and s64, add <2 x s32>).

Right;  I suppose you could see a vreg as having an "initial" type, the one on the def.   The type(s) of an MI is then just the type(s) of its def(s).  I admit it's also somewhat awkward, but at least avoids duplicating the info in a separate, seldom accessed, data structure.  Thoughts?

> But yeah, long term, this code shouldn't be needed. Add a comment about that?

SGTM, will do.  While I'm there, anything else?


https://reviews.llvm.org/D22373





More information about the llvm-commits mailing list