[GlobalISel] Constrain the dest reg of IMPLICT_DEF

[aemerson]

Summary

This fixes a crash where the user is a COPY, which deliberately does not constrain its source operands, resulting in a vreg without a reg class escaping selection.

[aemerson]

Meant to say, I'm *not* familiar with this part of the codebase.

[aditya_nandakumar]

lib/CodeGen/GlobalISel/InstructionSelect.cpp

186 ↗

(On Diff #132001)

After ISel, is it at all possible that vregs will have no regclass assigned? The above code assumes that specifically.

[aemerson]

lib/CodeGen/GlobalISel/InstructionSelect.cpp

186 ↗

(On Diff #132001)

I'm not sure, it did seem odd. Perhaps the underlying issue is manifesting before this.

[aemerson]

New fix, I think the issue was that IMPLICIT_DEFs weren't constraining their dest register, while COPY selection didn't constrain the source.

[aemerson]

Ping.

[qcolombet]

LGTM.

While I was here, I notice that selectCopy was not using getRegClassForTypeOnBank. Could you double check this is intended?

[aemerson]

Thanks, I'll commit this first and follow up on that. At first glance they're using different slightly different register classes so I'm not sure how correct it is to use that.

llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp

case TargetOpcode::G_FCMP: {
     I.eraseFromParent();
     return true;
   }
   case TargetOpcode::G_VASTART:
     return STI.isTargetDarwin() ? selectVaStartDarwin(I, MF, MRI)
                                 : selectVaStartAAPCS(I, MF, MRI);
   case TargetOpcode::G_IMPLICIT_DEF:
     I.setDesc(TII.get(TargetOpcode::IMPLICIT_DEF));
+    const LLT DstTy = MRI.getType(I.getOperand(0).getReg());
+    const unsigned DstReg = I.getOperand(0).getReg();
+    const RegisterBank &DstRB = *RBI.getRegBank(DstReg, MRI, TRI);
+    const TargetRegisterClass *DstRC =
+        getRegClassForTypeOnBank(DstTy, DstRB, RBI);
+    RBI.constrainGenericRegister(DstReg, *DstRC, MRI);
     return true;
   }
 
   return false;
 }
 
 /// SelectArithImmed - Select an immediate value that can be represented as
 /// a 12-bit value shifted left by either 0 or 12.  If so, return true with

llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-implicit-def.mir

 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 # RUN: llc -O0 -mtriple=aarch64-- -run-pass=instruction-select -verify-machineinstrs -global-isel %s -o - | FileCheck %s
 
 --- |
   target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
 
   define void @implicit_def() { ret void }
+  define void @implicit_def_copy() { ret void }
 ...
 
 ---
 name:            implicit_def
 legalized:       true
 regBankSelected: true
 registers:
   - { id: 0, class: gpr }
   - { id: 1, class: gpr }
 
 body:             |
   bb.0:
     ; CHECK-LABEL: name: implicit_def
     ; CHECK: [[DEF:%[0-9]+]]:gpr32 = IMPLICIT_DEF
     ; CHECK: [[ADDWrr:%[0-9]+]]:gpr32 = ADDWrr [[DEF]], [[DEF]]
     ; CHECK: $w0 = COPY [[ADDWrr]]
     %0(s32) = G_IMPLICIT_DEF
     %1(s32) = G_ADD %0, %0
     $w0 = COPY %1(s32)
 ...
+---
+name:            implicit_def_copy
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: implicit_def_copy
+    ; CHECK: [[DEF:%[0-9]+]]:gpr32 = IMPLICIT_DEF
+    ; CHECK: [[COPY:%[0-9]+]]:gpr32all = COPY [[DEF]]
+    ; CHECK: %w0 = COPY [[COPY]]
+    %0(s32) = G_IMPLICIT_DEF
+    %1(s32) = COPY %0(s32)
+    %w0 = COPY %1(s32)
+...