[MIParser] Set RegClassOrRegBank during instruction parsing

[Petar.Avramovic]

Summary

MachineRegisterInfo::createGenericVirtualRegister sets
RegClassOrRegBank to static_cast<RegisterBank *>(nullptr).
MIParser on the other hand doesn't. When we attempt to constrain
Register Class on such VReg, additional COPY is generated.
This way we avoid COPY instructions showing in test that have MIR
input while they are not present with llvm-ir input that was used
to create given MIR for a -run-pass test.

[Petar.Avramovic]

Motivation is having same behavior for GlobalISel legalizer tests for -stop-after=legalizer (llvm-ir input) and -run-pass=legalizer (mir input created from output of same llvm-ir input by pass before legalizer), later has few extra COPY instructions.
Mips has builtin functions (not available as generic opcode) that correspond to a machine instruction, and as such can be selected during legalization.
When selected, register classes also have to be constrained because other passes leave target specific instructions as they are.
Such builtin functions turned to intrinsic and are handled in MipsLegalizerInfo::legalizeIntrinsic.

[arsenm]

Should have dedicated parser test

[Petar.Avramovic]

Add dedicated test for MIParser.

[arsenm]

test/CodeGen/MIR/Mips/setRegClassOrRegBank.mir

2

The parser test shouldn't be running the legalizer? -O0 and -global-isel are also unnecessary

[Petar.Avramovic]

Rebased.

[Petar.Avramovic]

test/CodeGen/MIR/Mips/setRegClassOrRegBank.mir

2

The change affects internal state of MachineRegisterInfo and is not visible if we only do the parsing ( -run-pass=none ), but is visible when we try to constrainSelectedInstRegOperands.
I saw a few tests with -run-pass other then none, e.g.
test/CodeGen/MIR/AArch64/print-parse-verify-failedISel-property.mir
test/CodeGen/MIR/AArch64/print-parse-overloaded-intrinsics.mir.
Any suggestions how to make better test?

[arsenm]

I'm still not sure I mechanically follow how this ends up causing a problem? Surely the VRegInfo field was zero initialized already?

[Petar.Avramovic]

RegClassOrRegBank is a PointerUnion<const TargetRegisterClass *, const RegisterBank *>.
Plain nullptr is a const TargetRegisterClass * (the first template type)
but static_cast<RegisterBank *>(nullptr) is nullptr of const RegisterBank * type.

file: lib/CodeGen/GlobalISel/RegisterBankInfo.cpp in ...RegisterBankInfo::constrainGenericRegister(...
line :
if (RegClassOrBank.is<const TargetRegisterClass *>())
makes the difference.

if we go through IRTranslator, it uses createGenericVirtualRegister which also sets RegClassOrRegBank to nullptr RegisterBank *

file: lib/CodeGen/MachineRegisterInfo.cpp in ...MachineRegisterInfo::createGenericVirtualRegister(...

// FIXME: Should we use a dummy register class?
VRegInfo[Reg].first = static_cast<RegisterBank *>(nullptr);
setType(Reg, Ty);

[arsenm]

LGTM. I think the "dummy register class" FIXME is a pretty definite no

lib/CodeGen/MIRParser/MIParser.cpp

if (consumeIfPresent(MIToken::lparen)) {
           return error("expected tied-def or low-level type after '('");
 
         if (expectAndConsume(MIToken::rparen))
           return true;
 
         if (MRI.getType(Reg).isValid() && MRI.getType(Reg) != Ty)
           return error("inconsistent type for generic virtual register");
 
+        MRI.setRegClassOrRegBank(Reg, static_cast<RegisterBank *>(nullptr));
         MRI.setType(Reg, Ty);
       }
     }
   } else if (consumeIfPresent(MIToken::lparen)) {
     // Virtual registers may have a tpe with GlobalISel.
     if (!Register::isVirtualRegister(Reg))
       return error("unexpected type on physical register");
 
     LLT Ty;
     if (parseLowLevelType(Token.location(), Ty))
       return true;
 
     if (expectAndConsume(MIToken::rparen))
       return true;
 
     if (MRI.getType(Reg).isValid() && MRI.getType(Reg) != Ty)
       return error("inconsistent type for generic virtual register");
 
+    MRI.setRegClassOrRegBank(Reg, static_cast<RegisterBank *>(nullptr));
     MRI.setType(Reg, Ty);
   } else if (Register::isVirtualRegister(Reg)) {
     // Generic virtual registers must have a type.
     // If we end up here this means the type hasn't been specified and
     // this is bad!
     if (RegInfo->Kind == VRegInfo::GENERIC ||
         RegInfo->Kind == VRegInfo::REGBANK)
       return error("generic virtual registers must have a type");

test/CodeGen/MIR/Mips/setRegClassOrRegBank.mir

 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 # RUN: llc -mtriple=mipsel-linux-gnu -mcpu=mips32r5 -mattr=+msa,+fp64,+nan2008 -run-pass=legalizer -verify-machineinstrs %s -o - | FileCheck %s -check-prefixes=P5600
 
-# Check there are COPY instructions surrounding ADDVI_W instruction.
-# MIParser does not set RegClassOrRegBank for parsed virtual registers.
+# Check there are no COPY instructions surrounding ADDVI_W instruction.
+# MIParser sets RegClassOrRegBank for parsed virtual registers.
 # Constraining register classes when G_INTRINSIC intrinsic(@llvm.mips.addvi.w)
-# gets selected into ADDVI_W creates additional copies.
-# FixMe: Make sure this test has same output as setRegClassOrRegBank.ll.
+# gets selected into ADDVI_W works as expected.
+# Check that setRegClassOrRegBank.ll has same output.
 
 --- |
 
   declare <4 x i32> @llvm.mips.addvi.w(<4 x i32>, i32 immarg)
   define void @add_v4i32_builtin_imm(<4 x i32>* %a, <4 x i32>* %c) { entry: ret void }
 
 ...
 ---
 name:            add_v4i32_builtin_imm
 alignment:       4
 tracksRegLiveness: true
 body:             |
   bb.1.entry:
     liveins: $a0, $a1
 
     ; P5600-LABEL: name: add_v4i32_builtin_imm
     ; P5600: liveins: $a0, $a1
     ; P5600: [[COPY:%[0-9]+]]:_(p0) = COPY $a0
     ; P5600: [[COPY1:%[0-9]+]]:_(p0) = COPY $a1
-    ; P5600: [[LOAD:%[0-9]+]]:_(<4 x s32>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
-    ; P5600: [[COPY2:%[0-9]+]]:msa128w = COPY [[LOAD]](<4 x s32>)
-    ; P5600: [[ADDVI_W:%[0-9]+]]:msa128w = ADDVI_W [[COPY2]], 25
-    ; P5600: [[COPY3:%[0-9]+]]:_(<4 x s32>) = COPY [[ADDVI_W]]
-    ; P5600: G_STORE [[COPY3]](<4 x s32>), [[COPY1]](p0) :: (store 16 into %ir.c)
+    ; P5600: [[LOAD:%[0-9]+]]:msa128w(<4 x s32>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
+    ; P5600: [[ADDVI_W:%[0-9]+]]:msa128w(<4 x s32>) = ADDVI_W [[LOAD]](<4 x s32>), 25
+    ; P5600: G_STORE [[ADDVI_W]](<4 x s32>), [[COPY1]](p0) :: (store 16 into %ir.c)
     ; P5600: RetRA
     %0:_(p0) = COPY $a0
     %1:_(p0) = COPY $a1
     %2:_(<4 x s32>) = G_LOAD %0(p0) :: (load 16 from %ir.a)
     %3:_(<4 x s32>) = G_INTRINSIC intrinsic(@llvm.mips.addvi.w), %2(<4 x s32>), 25
     G_STORE %3(<4 x s32>), %1(p0) :: (store 16 into %ir.c)
     RetRA
 
 ...

test/CodeGen/Mips/GlobalISel/legalizer/add_vec_builtin.mir

 body:             |
   bb.1.entry:
     liveins: $a0, $a1
 
     ; P5600-LABEL: name: add_v16i8_builtin_imm
     ; P5600: liveins: $a0, $a1
     ; P5600: [[COPY:%[0-9]+]]:_(p0) = COPY $a0
     ; P5600: [[COPY1:%[0-9]+]]:_(p0) = COPY $a1
-    ; P5600: [[LOAD:%[0-9]+]]:_(<16 x s8>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
-    ; P5600: [[COPY2:%[0-9]+]]:msa128b = COPY [[LOAD]](<16 x s8>)
-    ; P5600: [[ADDVI_B:%[0-9]+]]:msa128b = ADDVI_B [[COPY2]], 3
-    ; P5600: [[COPY3:%[0-9]+]]:_(<16 x s8>) = COPY [[ADDVI_B]]
-    ; P5600: G_STORE [[COPY3]](<16 x s8>), [[COPY1]](p0) :: (store 16 into %ir.c)
+    ; P5600: [[LOAD:%[0-9]+]]:msa128b(<16 x s8>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
+    ; P5600: [[ADDVI_B:%[0-9]+]]:msa128b(<16 x s8>) = ADDVI_B [[LOAD]](<16 x s8>), 3
+    ; P5600: G_STORE [[ADDVI_B]](<16 x s8>), [[COPY1]](p0) :: (store 16 into %ir.c)
     ; P5600: RetRA
     %0:_(p0) = COPY $a0
     %1:_(p0) = COPY $a1
     %2:_(<16 x s8>) = G_LOAD %0(p0) :: (load 16 from %ir.a)
     %3:_(<16 x s8>) = G_INTRINSIC intrinsic(@llvm.mips.addvi.b), %2(<16 x s8>), 3
     G_STORE %3(<16 x s8>), %1(p0) :: (store 16 into %ir.c)
     RetRA
 
 ...
 ---
 name:            add_v8i16_builtin_imm
 alignment:       4
 tracksRegLiveness: true
 body:             |
   bb.1.entry:
     liveins: $a0, $a1
 
     ; P5600-LABEL: name: add_v8i16_builtin_imm
     ; P5600: liveins: $a0, $a1
     ; P5600: [[COPY:%[0-9]+]]:_(p0) = COPY $a0
     ; P5600: [[COPY1:%[0-9]+]]:_(p0) = COPY $a1
-    ; P5600: [[LOAD:%[0-9]+]]:_(<8 x s16>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
-    ; P5600: [[COPY2:%[0-9]+]]:msa128h = COPY [[LOAD]](<8 x s16>)
-    ; P5600: [[ADDVI_H:%[0-9]+]]:msa128h = ADDVI_H [[COPY2]], 18
-    ; P5600: [[COPY3:%[0-9]+]]:_(<8 x s16>) = COPY [[ADDVI_H]]
-    ; P5600: G_STORE [[COPY3]](<8 x s16>), [[COPY1]](p0) :: (store 16 into %ir.c)
+    ; P5600: [[LOAD:%[0-9]+]]:msa128h(<8 x s16>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
+    ; P5600: [[ADDVI_H:%[0-9]+]]:msa128h(<8 x s16>) = ADDVI_H [[LOAD]](<8 x s16>), 18
+    ; P5600: G_STORE [[ADDVI_H]](<8 x s16>), [[COPY1]](p0) :: (store 16 into %ir.c)
     ; P5600: RetRA
     %0:_(p0) = COPY $a0
     %1:_(p0) = COPY $a1
     %2:_(<8 x s16>) = G_LOAD %0(p0) :: (load 16 from %ir.a)
     %3:_(<8 x s16>) = G_INTRINSIC intrinsic(@llvm.mips.addvi.h), %2(<8 x s16>), 18
     G_STORE %3(<8 x s16>), %1(p0) :: (store 16 into %ir.c)
     RetRA
 
 ...
 ---
 name:            add_v4i32_builtin_imm
 alignment:       4
 tracksRegLiveness: true
 body:             |
   bb.1.entry:
     liveins: $a0, $a1
 
     ; P5600-LABEL: name: add_v4i32_builtin_imm
     ; P5600: liveins: $a0, $a1
     ; P5600: [[COPY:%[0-9]+]]:_(p0) = COPY $a0
     ; P5600: [[COPY1:%[0-9]+]]:_(p0) = COPY $a1
-    ; P5600: [[LOAD:%[0-9]+]]:_(<4 x s32>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
-    ; P5600: [[COPY2:%[0-9]+]]:msa128w = COPY [[LOAD]](<4 x s32>)
-    ; P5600: [[ADDVI_W:%[0-9]+]]:msa128w = ADDVI_W [[COPY2]], 25
-    ; P5600: [[COPY3:%[0-9]+]]:_(<4 x s32>) = COPY [[ADDVI_W]]
-    ; P5600: G_STORE [[COPY3]](<4 x s32>), [[COPY1]](p0) :: (store 16 into %ir.c)
+    ; P5600: [[LOAD:%[0-9]+]]:msa128w(<4 x s32>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
+    ; P5600: [[ADDVI_W:%[0-9]+]]:msa128w(<4 x s32>) = ADDVI_W [[LOAD]](<4 x s32>), 25
+    ; P5600: G_STORE [[ADDVI_W]](<4 x s32>), [[COPY1]](p0) :: (store 16 into %ir.c)
     ; P5600: RetRA
     %0:_(p0) = COPY $a0
     %1:_(p0) = COPY $a1
     %2:_(<4 x s32>) = G_LOAD %0(p0) :: (load 16 from %ir.a)
     %3:_(<4 x s32>) = G_INTRINSIC intrinsic(@llvm.mips.addvi.w), %2(<4 x s32>), 25
     G_STORE %3(<4 x s32>), %1(p0) :: (store 16 into %ir.c)
     RetRA
 
 ...
 ---
 name:            add_v2i64_builtin_imm
 alignment:       4
 tracksRegLiveness: true
 body:             |
   bb.1.entry:
     liveins: $a0, $a1
 
     ; P5600-LABEL: name: add_v2i64_builtin_imm
     ; P5600: liveins: $a0, $a1
     ; P5600: [[COPY:%[0-9]+]]:_(p0) = COPY $a0
     ; P5600: [[COPY1:%[0-9]+]]:_(p0) = COPY $a1
-    ; P5600: [[LOAD:%[0-9]+]]:_(<2 x s64>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
-    ; P5600: [[COPY2:%[0-9]+]]:msa128d = COPY [[LOAD]](<2 x s64>)
-    ; P5600: [[ADDVI_D:%[0-9]+]]:msa128d = ADDVI_D [[COPY2]], 31
-    ; P5600: [[COPY3:%[0-9]+]]:_(<2 x s64>) = COPY [[ADDVI_D]]
-    ; P5600: G_STORE [[COPY3]](<2 x s64>), [[COPY1]](p0) :: (store 16 into %ir.c)
+    ; P5600: [[LOAD:%[0-9]+]]:msa128d(<2 x s64>) = G_LOAD [[COPY]](p0) :: (load 16 from %ir.a)
+    ; P5600: [[ADDVI_D:%[0-9]+]]:msa128d(<2 x s64>) = ADDVI_D [[LOAD]](<2 x s64>), 31
+    ; P5600: G_STORE [[ADDVI_D]](<2 x s64>), [[COPY1]](p0) :: (store 16 into %ir.c)
     ; P5600: RetRA
     %0:_(p0) = COPY $a0
     %1:_(p0) = COPY $a1
     %2:_(<2 x s64>) = G_LOAD %0(p0) :: (load 16 from %ir.a)
     %3:_(<2 x s64>) = G_INTRINSIC intrinsic(@llvm.mips.addvi.d), %2(<2 x s64>), 31
     G_STORE %3(<2 x s64>), %1(p0) :: (store 16 into %ir.c)
     RetRA
 
 ...

test/CodeGen/Mips/GlobalISel/legalizer/sitofp_and_uitofp.mir

 alignment:       4
 tracksRegLiveness: true
 body:             |
   bb.1.entry:
     liveins: $a0
 
     ; FP32-LABEL: name: u32tof32
     ; FP32: liveins: $a0
-    ; FP32: [[COPY:%[0-9]+]]:_(s32) = COPY $a0
+    ; FP32: [[COPY:%[0-9]+]]:gpr32(s32) = COPY $a0
     ; FP32: [[C:%[0-9]+]]:gpr32(s32) = G_CONSTANT i32 1127219200
-    ; FP32: [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY]](s32)
-    ; FP32: [[BuildPairF64_:%[0-9]+]]:afgr64(s64) = BuildPairF64 [[COPY1]], [[C]](s32)
+    ; FP32: [[BuildPairF64_:%[0-9]+]]:afgr64(s64) = BuildPairF64 [[COPY]](s32), [[C]](s32)
     ; FP32: [[C1:%[0-9]+]]:_(s64) = G_FCONSTANT double 0x4330000000000000
     ; FP32: [[FSUB:%[0-9]+]]:_(s64) = G_FSUB [[BuildPairF64_]], [[C1]]
     ; FP32: [[FPTRUNC:%[0-9]+]]:_(s32) = G_FPTRUNC [[FSUB]](s64)
     ; FP32: $f0 = COPY [[FPTRUNC]](s32)
     ; FP32: RetRA implicit $f0
     ; FP64-LABEL: name: u32tof32
     ; FP64: liveins: $a0
-    ; FP64: [[COPY:%[0-9]+]]:_(s32) = COPY $a0
+    ; FP64: [[COPY:%[0-9]+]]:gpr32(s32) = COPY $a0
     ; FP64: [[C:%[0-9]+]]:gpr32(s32) = G_CONSTANT i32 1127219200
-    ; FP64: [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY]](s32)
-    ; FP64: [[BuildPairF64_64_:%[0-9]+]]:fgr64(s64) = BuildPairF64_64 [[COPY1]], [[C]](s32)
+    ; FP64: [[BuildPairF64_64_:%[0-9]+]]:fgr64(s64) = BuildPairF64_64 [[COPY]](s32), [[C]](s32)
     ; FP64: [[C1:%[0-9]+]]:_(s64) = G_FCONSTANT double 0x4330000000000000
     ; FP64: [[FSUB:%[0-9]+]]:_(s64) = G_FSUB [[BuildPairF64_64_]], [[C1]]
     ; FP64: [[FPTRUNC:%[0-9]+]]:_(s32) = G_FPTRUNC [[FSUB]](s64)
     ; FP64: $f0 = COPY [[FPTRUNC]](s32)
     ; FP64: RetRA implicit $f0
     %0:_(s32) = COPY $a0
     %1:_(s32) = G_UITOFP %0(s32)
     $f0 = COPY %1(s32)

 alignment:       4
 tracksRegLiveness: true
 body:             |
   bb.1.entry:
     liveins: $a0
 
     ; FP32-LABEL: name: u32tof64
     ; FP32: liveins: $a0
-    ; FP32: [[COPY:%[0-9]+]]:_(s32) = COPY $a0
+    ; FP32: [[COPY:%[0-9]+]]:gpr32(s32) = COPY $a0
     ; FP32: [[C:%[0-9]+]]:gpr32(s32) = G_CONSTANT i32 1127219200
-    ; FP32: [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY]](s32)
-    ; FP32: [[BuildPairF64_:%[0-9]+]]:afgr64(s64) = BuildPairF64 [[COPY1]], [[C]](s32)
+    ; FP32: [[BuildPairF64_:%[0-9]+]]:afgr64(s64) = BuildPairF64 [[COPY]](s32), [[C]](s32)
     ; FP32: [[C1:%[0-9]+]]:_(s64) = G_FCONSTANT double 0x4330000000000000
     ; FP32: [[FSUB:%[0-9]+]]:_(s64) = G_FSUB [[BuildPairF64_]], [[C1]]
     ; FP32: $d0 = COPY [[FSUB]](s64)
     ; FP32: RetRA implicit $d0
     ; FP64-LABEL: name: u32tof64
     ; FP64: liveins: $a0
-    ; FP64: [[COPY:%[0-9]+]]:_(s32) = COPY $a0
+    ; FP64: [[COPY:%[0-9]+]]:gpr32(s32) = COPY $a0
     ; FP64: [[C:%[0-9]+]]:gpr32(s32) = G_CONSTANT i32 1127219200
-    ; FP64: [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY]](s32)
-    ; FP64: [[BuildPairF64_64_:%[0-9]+]]:fgr64(s64) = BuildPairF64_64 [[COPY1]], [[C]](s32)
+    ; FP64: [[BuildPairF64_64_:%[0-9]+]]:fgr64(s64) = BuildPairF64_64 [[COPY]](s32), [[C]](s32)
     ; FP64: [[C1:%[0-9]+]]:_(s64) = G_FCONSTANT double 0x4330000000000000
     ; FP64: [[FSUB:%[0-9]+]]:_(s64) = G_FSUB [[BuildPairF64_64_]], [[C1]]
     ; FP64: $d0 = COPY [[FSUB]](s64)
     ; FP64: RetRA implicit $d0
     %0:_(s32) = COPY $a0
     %1:_(s64) = G_UITOFP %0(s32)
     $d0 = COPY %1(s64)
     RetRA implicit $d0