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

[Quentin Colombet via llvm-commits]

> On Jul 5, 2016, at 2:03 PM, Tim Northover <t.p.northover at gmail.com> wrote:
> 
> 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?

Yeah, that is what I was going to say, the generic code will know if something is floating point or int. That’s probably something we will need at some point elsewhere and we could have that live as a query function like isPreISelOpcode.
That being said, given the current patch achieve what we need, I’d say we can leave that for a future refactoring.

Thanks,
Q.

> 
> Tim.
> 
> 
> ================
> 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.

Heh.

> 
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D22008
> 
> 
>