[PATCH] D42287: [GlobalISel][X86] Fixing failures after https://reviews.llvm.org/D37775

[Alexander Ivchenko via Phabricator via llvm-commits]

aivchenk added a comment.

Hi Quentin, first of all thanks for the review.

I realize my mistake about the tests, but not clear what would be the best direction.
Should I submit autogenerated tests that are not FP/G_COPY/G_TRUNC related, but that still need to be updated for https://reviews.llvm.org/D37775 as a separate patch? Should I have a review for it or can I just submit?

As for the tests that are affected by that patch, should I keep them here, even though they are autogenerated?



================
Comment at: lib/Target/X86/X86InstructionSelector.cpp:686
   if (!SrcRC)
     return false;
 
----------------
qcolombet wrote:
> Should we move these ![...]RC checks up?
I'm not sure, to be honest. Added lines should be before

 if (DstRB.getID() != X86::GPRRegBankID)
    return false;

Cause otherwise we will return in that check. At the same time those checks that you mention are after X86::GPRRegBankID one, so that move will change the logic. And I was trying to avoid changing the logic as I'm not that familiar with that code yet


================
Comment at: test/CodeGen/X86/GlobalISel/callingconv.ll:329
+; X86_64-NEXT:    .cfi_offset %rbx, -16
+; X86_64-NEXT:    movzbl (%rdi), %ebx
+; X86_64-NEXT:    movl %ebx, %edi
----------------
qcolombet wrote:
> Do you know why this got better all of the sudden?
No, this change is not connected with my patch


================
Comment at: test/CodeGen/X86/GlobalISel/callingconv.ll:404
+; X86_64-NEXT:    movq %rax, %xmm0
+; X86_64-NEXT:    movb $1, %al
+; X86_64-NEXT:    callq variadic_callee
----------------
qcolombet wrote:
> Why did the scheduling/RA change?
That's again not connected


https://reviews.llvm.org/D42287