[PATCH] D42287: [GlobalISel][X86] Fixing failures after https://reviews.llvm.org/D37775
Quentin Colombet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 19 14:40:15 PST 2018
qcolombet added a comment.
> 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?
I would just push the autogenerate checks first directly (no review needed).
That way, the tests will only have the FP/COPY/ANYEXT changes + their impact on autogenerated checks.
================
Comment at: lib/Target/X86/X86InstructionSelector.cpp:686
if (!SrcRC)
return false;
----------------
aivchenk wrote:
> 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
I meant only the part that checks that the RegisterClasses are not nullptr.
If they are nullptr, the checks you're adding won't match anyway :).
================
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
----------------
aivchenk wrote:
> qcolombet wrote:
> > Do you know why this got better all of the sudden?
> No, this change is not connected with my patch
Ok, hopefully this will disappear when you rebase your patch after pushing the autogenerated checks.
https://reviews.llvm.org/D42287
More information about the llvm-commits
mailing list