GlobalISel: Fix infinite loop in legalization artifact combiner

[Petar.Avramovic]

Summary

ArtifactValueFinder keeps trying to combine g_unmerge_values in some cases.
Fix is to skip combine attempt for dead defs.

[Petar.Avramovic]

My best guess is that this happens when dead-def can be found by looking through insert but there is nothing to replace with the value we found.
TODO: avoid using G_INSERT.

[arsenm]

llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

770

Probably should be use_nodbg_empty

llvm/test/CodeGen/AMDGPU/GlobalISel/bug-legalization-artifact-combiner-dead-def.ll

21

Should also add a MIR test. Also should add a case where there is only a debug info user

[aemerson]

llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

770

If the def is dead, we should also set the dead bit in DeadDefs.

[Petar.Avramovic]

Mark dead bit in DeadDefs and add mir test. MIR in last two tests is from just before artifact combiner. In one we can see dead def in unmerge, other has only dbg use.

[Petar.Avramovic]

llvm/test/CodeGen/AMDGPU/GlobalISel/bug-legalization-artifact-combiner-dead-def.mir

115–116 ↗

(On Diff #362393)

With use_nodbg_empty, this will look like DBG_VALUE %dbg_use(s32) because we skip combine (replace register with found value). It also seems wrong to mark register as dead in that case. Should we treat DBG instrs in special way in this combine?

[aemerson]

llvm/test/CodeGen/AMDGPU/GlobalISel/bug-legalization-artifact-combiner-dead-def.mir

115–116 ↗

(On Diff #362393)

If we don't ignore the debug user, then we could have potentially differing codegen between release and debug builds.

[aemerson]

By the way, I just refactored some of this code in f984b0e177f8 but it's just a case of moving your change to a different function.

llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

bool tryCombineUnmergeValues(GUnmerge &MI,
     Builder.setInstrAndDebugLoc(MI);
 
     auto tryCombineViaValueFinder = [&]() {
       ArtifactValueFinder ValueFinder(MRI, Builder, LI);
 
       SmallBitVector DeadDefs(NumDefs);
       for (unsigned DefIdx = 0; DefIdx < NumDefs; ++DefIdx) {
         Register DefReg = MI.getReg(DefIdx);
+        if (MRI.use_empty(DefReg))
+          continue;
         Register FoundVal =
             ValueFinder.findValueFromDef(DefReg, 0, DestTy.getSizeInBits());
         if (!FoundVal || FoundVal == DefReg)
           continue;
         if (MRI.getType(FoundVal) != DestTy)
           continue;
 
         replaceRegOrBuildCopy(DefReg, FoundVal, MRI, Builder, UpdatedDefs,

llvm/test/CodeGen/AMDGPU/GlobalISel/bug-legalization-artifact-combiner-dead-def.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -global-isel -mtriple=amdgcn-amd-amdpal -mcpu=gfx1010 < %s | FileCheck -check-prefix=GFX10 %s
+
+define void @value_finder_bug(<2 x float> addrspace(5)* %store_ptr, <4 x float> addrspace(4)* %ptr) {
+; GFX10-LABEL: value_finder_bug:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX10-NEXT:    s_waitcnt_vscnt null, 0x0
+; GFX10-NEXT:    global_load_dwordx4 v[1:4], v[1:2], off
+; GFX10-NEXT:    s_waitcnt vmcnt(0)
+; GFX10-NEXT:    buffer_store_dword v3, v0, s[0:3], 0 offen
+; GFX10-NEXT:    buffer_store_dword v4, v0, s[0:3], 0 offen offset:4
+; GFX10-NEXT:    s_waitcnt_vscnt null, 0x0
+; GFX10-NEXT:    s_setpc_b64 s[30:31]
+  %vec = load <4 x float>, <4 x float> addrspace(4)* %ptr, align 4
+  %vec.3 = extractelement <4 x float> %vec, i32 3
+  %shuffle = shufflevector <4 x float> %vec, <4 x float> undef, <2 x i32> <i32 2, i32 undef>
+  %new_vec = insertelement <2 x float> %shuffle, float %vec.3, i32 1
+  store <2 x float> %new_vec, <2 x float> addrspace(5)* %store_ptr, align 8
+  ret void
+}