[PATCH] D22008: GlobalISel: implement low-level type suitable for MachineInstr selection

Tim Northover via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 5 14:03:30 PDT 2016

t.p.northover marked 2 inline comments as done.
t.p.northover added a comment.

> I'd like to keep (at least for now), the type recording capabilities in register bank info though.  I believe making a second constructor for LLT that takes SVT is going to be useful anyway.

I removed that reasonably late in development. Going from an SVT to an LLT is fine, the problem was that the only query of the information was based on a MachineInstr's (LLT) type which no longer has the information needed to choose a sane SVT. I suppose we could add some concept of a generic MachineInstr's "realm" (int/float) to provide the dropped info?


Comment at: lib/CodeGen/LowLevelType.cpp:24
@@ +23,3 @@
+    ScalarSize = VTy->getElementType()->getPrimitiveSizeInBits();
+    NumElements = VTy->getNumElements();
+  } else if (Ty->isSized()) {
qcolombet wrote:
> Don't you say we should allow <1 x Ty> kind of vector?
> I do not see that here.
Yep, discovered that myself while integrating this patch with my legalize things. I'll fix it in the updated version.

Comment at: test/CodeGen/AArch64/GlobalISel/arm64-regbankselect.mir:72
@@ -73,1 +71,3 @@
+    ; CHECK:      %0(32) = G_ADD s32 %w0
+    %0(32) = G_ADD s32 %w0, %w0
qcolombet wrote:
> Thanks for fixing the typo as well, I meant to do it at some point :P.
Not entirely altruistic: it caused aborts with the new code.



More information about the llvm-commits mailing list