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

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 14 15:00:55 PDT 2016


Hi Matthias,

> On Jul 14, 2016, at 11:35 AM, Matthias Braun <matze at braunis.de> wrote:
> 
> MatzeB added a comment.
> 
> I just looked at the MachineRegisterInfo parts. This is also a highlevel comment, not necessarily about this patch (which just continues the current direction): It seems to me like generic regs and "normal" vregs don't have that much in common, they mostly share the same kind of MachineOperand and the same def/use lists. Apart from that they contain different information (register bank + size vs. register class). Wouldn't it make sense to introduce a new MachineOperand kind for them and separate them from "normal" vregs?

What kind of gain do you see by having a new kind of MachineOperand?

From my point of view, this is not needed. Yes, register banks are a higher abstraction than register classes, but other than that they are the same kind of registers. The size thing is just encoded in the register class, but technically, you could ask for the size of a “normal” vreg.

The bottom line is, the way we deal with both is identical except from the register bank/class field.

Ultimately, all virtual registers would have a register bank (dynamically deduce from there current register class). Right now this is not the case because RegisterBanks are in a different, optional, library.

Cheers,
-Quentin

> 
> 
> ================
> Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:38
> @@ +37,3 @@
> +
> +static void reportSelectionError(const MachineInstr &MI, StringRef Message) {
> +  const MachineFunction &MF = *MI.getParent()->getParent();
> ----------------
> Twine Message?
> 
> 
> https://reviews.llvm.org/D22373
> 
> 
> 



More information about the llvm-commits mailing list