Add a verifier test to check the access on both sides of COPY are the same

[aditya_nandakumar]

Summary

Only checks if there are no sub registers involved for now.
There are lots of failures currently which need to be fixed by various backends.

Splitting this off from https://reviews.llvm.org/D37640

[igorb]

Failed to build as a shared library -DBUILD_SHARED_LIBS=1

[igorb]

lib/CodeGen/MachineVerifier.cpp

953

For X86 back-end this is not always correct.
I think in case a few register classes with different size are mapped to the same physical registers, it is impossible to identify when the COPY is illegal without some additional target specific info.

def FR32 : RegisterClass<"X86", [f32], 32, (sequence "XMM%u", 0, 15)>;
def VR128 : RegisterClass<"X86", [v4f32, v2f64, v16i8, v8i16, v4i32, v2i64], 128, (add FR32)>;

For example the test fail DAG/GISEL (with the patch)
./bin/llc -O0 -mtriple=x86_64-linux-gnu -verify-machineinstrs -stop-before=legalizer
./bin/llc -O0 -mtriple=x86_64-linux-gnu -global-isel -verify-machineinstrs -stop-before=legalizer

define float @test_add_float(float %arg1, float %arg2) {
  %ret = fadd float %arg1, %arg2
  ret float %ret
}

for the correct MIR. (xmm size is 128)

%0(s32) = COPY %xmm0
%1(s32) = COPY %xmm1
%2(s32) = G_FADD %0, %1
%xmm0 = COPY %2(s32)

Thanks

[qcolombet]

lib/CodeGen/MachineVerifier.cpp

953

I'd argue that when generic types are on one side of a copy, i.e., this is a pre-isel copy, we want the size to exactly match.
Thus, if the target wants to do truncate and whatnot at this point, it needs to use G_TRUNC, G_ANYEXT, and so on.

That being said, the check is too broad. We need to check that only for pre-isel copy. Regular copies require indeed target knowledge (e.g., eflags = copy gpr is valid on x86).

[aditya_nandakumar]

lib/CodeGen/MachineVerifier.cpp

953

I can update the check to do it only when there are generic types involved (ie one of src and dst has a generic virtual register).

[qcolombet]

lib/CodeGen/MachineVerifier.cpp

953

That would be perfect.

[aditya_nandakumar]

Updated to check only if one of Src/Dst has generic virtual registers.

[aditya_nandakumar]

Updated to refactor getRegSizeInBits into TargetRegisterInfo. There are 57 failures in total.

[qcolombet]

LGTM with one suggestion

lib/CodeGen/MachineVerifier.cpp

957

Seeing this pattern being actually the implementation of RegisterBankInfo::getSizeInBits, I would suggest to move the whole RegisterBankInfo::getSizeInBits directly in TargetRegisterInfo.

[aditya_nandakumar]

Also, there are various tests that need to be updated by the various backends, and I can only land once those are updated. I haven't had time to update all the backends.

lib/CodeGen/MachineVerifier.cpp

957

I thought TargetRegisterInfo doesn't have LLT headers pulled in (but looks like it is getting transitively pulled in). I can pretty much move the implementation of RegisterBankInfo::getSizeInBits into TargetRegisterInfo.

[aditya_nandakumar]

moved getSizeInBits to TargetRegisterInfo as suggested by Quentin.