[X86DomainReassignment] Don't delete IMPLICIT_DEF nodes

[loladiro]

Summary

We cannot simply delete IMPLICIT_DEF nodes. They may be used
later (e.g. by a PHI) and deleting them will cause later passes (e.g.
LiveVariables) to crash. However, it seems fine to ignore them for
purposes of the domain reassignment (as we do with PHI).

Fixes PR37430
Fixes JuliaLang/julia#27080

[craig.topper]

test/CodeGen/X86/domain-reassignment-implicit-def.ll

1 ↗

(On Diff #146490)

These don't seem to have been generated by the script. The checks don't cover the whole function.

[loladiro]

test/CodeGen/X86/domain-reassignment-implicit-def.ll

1 ↗

(On Diff #146490)

Well, they were generated by the script, but then the output seemed excessively verbose, so I cut it down to what I wanted to see. Do you want me to remove the comment or have the checks cover the whole function?

[craig.topper]

You can just remove the comment.

[loladiro]

Remove incorrect comment from test case.

[craig.topper]

LGTM

llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp

public:
   }
 
   double getExtraCost(const MachineInstr *MI,
                       MachineRegisterInfo *MRI) const override {
     return 0;
   }
 };
 
-/// An Instruction Converter which completely deletes an instruction.
-/// For example, IMPLICIT_DEF instructions can be deleted when converting from
-/// GPR to mask.
-class InstrDeleter : public InstrConverterBase {
-public:
-  InstrDeleter(unsigned SrcOpcode) : InstrConverterBase(SrcOpcode) {}
-
-  bool convertInstr(MachineInstr *MI, const TargetInstrInfo *TII,
-                    MachineRegisterInfo *MRI) const override {
-    assert(isLegal(MI, TII) && "Cannot convert instruction");
-    return true;
-  }
-
-  double getExtraCost(const MachineInstr *MI,
-                      MachineRegisterInfo *MRI) const override {
-    return 0;
-  }
-};
-
 // Key type to be used by the Instruction Converters map.
 // A converter is identified by <destination domain, source opcode>
 typedef std::pair<int, unsigned> InstrConverterBaseKeyTy;
 
 typedef DenseMap<InstrConverterBaseKeyTy, InstrConverterBase *>
     InstrConverterBaseMap;
 
 /// A closure is a set of virtual register representing all of the edges in
void X86DomainReassignment::buildClosure(Closure &C, unsigned Reg) {
   }
 }
 
 void X86DomainReassignment::initConverters() {
   Converters[{MaskDomain, TargetOpcode::PHI}] =
       new InstrIgnore(TargetOpcode::PHI);
 
   Converters[{MaskDomain, TargetOpcode::IMPLICIT_DEF}] =
-      new InstrDeleter(TargetOpcode::IMPLICIT_DEF);
+      new InstrIgnore(TargetOpcode::IMPLICIT_DEF);
 
   Converters[{MaskDomain, TargetOpcode::INSERT_SUBREG}] =
       new InstrReplaceWithCopy(TargetOpcode::INSERT_SUBREG, 2);
 
   Converters[{MaskDomain, TargetOpcode::COPY}] =
       new InstrCOPYReplacer(TargetOpcode::COPY, MaskDomain, TargetOpcode::COPY);
 
   auto createReplacerDstCOPY = [&](unsigned From, unsigned To) {

llvm/trunk/test/CodeGen/X86/domain-reassignment-implicit-def.ll

+; RUN: llc -mcpu=skylake-avx512 -mtriple=x86_64-unknown-linux-gnu %s -o - | FileCheck %s
+
+; Check that the X86 Domain Reassignment pass doesn't drop IMPLICIT_DEF nodes,
+; which would later cause crashes (e.g. in LiveVariables) - see PR37430
+define void @domain_reassignment_implicit_def(i1 %cond, i8 *%mem, float %arg) {
+; CHECK:    vxorps %xmm1, %xmm1, %xmm1
+; CHECK:    vcmpneqss %xmm1, %xmm0, %k0
+; CHECK:    kmovb %k0, (%rsi)
+top:
+  br i1 %cond, label %L19, label %L15
+
+L15:                                              ; preds = %top
+  %tmp47 = fcmp une float 0.000000e+00, %arg
+  %tmp48 = zext i1 %tmp47 to i8
+  br label %L21
+
+L19:                                              ; preds = %top
+  br label %L21
+
+L21:                                              ; preds = %L19, %L15
+  %.sroa.0.0 = phi i8 [ undef, %L19 ], [ %tmp48, %L15 ]
+  store i8 %.sroa.0.0, i8* %mem, align 1
+  ret void
+}