LiveIntervalAnalysis: fix a crash in repairOldRegInRange

[nhaehnle]

Summary

I'm not very familiar with this code, so feel free to take over if you
have a better approach to fixing this.

See the new test case for one that was (non-deterministically) crashing
on trunk and deterministically hit the assertion that I added in D23302.
Basically, the machine function contains a sequence

DS_WRITE_B32 %vreg4, %vreg14:sub0, ...
DS_WRITE_B32 %vreg4, %vreg14:sub0, ...
%vreg14:sub1<def> = COPY %vreg14:sub0

and SILoadStoreOptimizer::mergeWrite2Pair merges the two DS_WRITE_B32
instructions into one before calling repairIntervalsInRange.

Now repairIntervalsInRange wants to repair %vreg14, in particular, and
ends up trying to repair %vreg14:sub1 as well, but that only becomes
active _after_ the range that is to be repaired, hence the crash due
to LR.find(...) == LR.begin() at the start of repairOldRegInRange.

I believe that just skipping those subrange is fine, but again, not too
familiar with that code.

[kparzysz]

This looks ok to me.

lib/CodeGen/LiveIntervalAnalysis.cpp

1404

LaneMask is probably always non-zero.

1406

Remove the "else".

[nhaehnle]

Rearrange the check.

[MatzeB]

llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp

1399–1403 ↗

(On Diff #67571)

I am not sure this is correct.

This seems to hit all cases where the first live segments ends after endIdx, though the comment sounds like it wants to hit the cases where the first live segments begins after endIdx.

[MatzeB]

Reviewing this because of http://llvm.org/PR31204.

I think this should be:

if (LII != LR.end() && LII->start < endIdx)
  lastUseIdx = LII->end;
else if (LII == LR.begin()) {
  // We may not have a liverange at all if this is a subregister untouched
  // between \p Begin and \p End.
} else
  --LII;

Maybe you can test/update this before it gets pulled into 3.9.x?

lib/CodeGen/LiveIntervalAnalysis.cpp

void LiveIntervals::repairOldRegInRange(const MachineBasicBlock::iterator Begin,
                                         const MachineBasicBlock::iterator End,
                                         const SlotIndex endIdx,
                                         LiveRange &LR, const unsigned Reg,
                                         LaneBitmask LaneMask) {
   LiveInterval::iterator LII = LR.find(endIdx);
   SlotIndex lastUseIdx;
   if (LII != LR.end() && LII->start < endIdx)
     lastUseIdx = LII->end;
-  else
+  else if (LII == LR.begin()) {
+    // This happens when the function is called for a subregister that only
+    // occurs _after_ the range that is to be repaired.
+    assert(LaneMask != 0);
+    return;
+  } else
     --LII;
 
   for (MachineBasicBlock::iterator I = End; I != Begin;) {
     --I;
     MachineInstr &MI = *I;
     if (MI.isDebugValue())
       continue;
 

test/CodeGen/AMDGPU/merge-store-crash.ll

+; RUN: llc -march=amdgcn -mcpu=verde -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck %s
+
+; This is used to crash in LiveIntervalAnalysis via SILoadStoreOptimizer
+; while fixing up the merge of two ds_write instructions.
+
+@tess_lds = external addrspace(3) global [8192 x i32]
+
+; CHECK-LABEL: {{^}}main:
+; CHECK: ds_write2_b32
+; CHECK: v_mov_b32_e32 v1, v0
+; CHECK: tbuffer_store_format_xyzw v[0:3],
+define amdgpu_vs void @main(i32 inreg %arg) {
+main_body:
+  %tmp = load float, float addrspace(3)* undef, align 4
+  %tmp1 = load float, float addrspace(3)* undef, align 4
+  store float %tmp, float addrspace(3)* null, align 4
+  %tmp2 = bitcast float %tmp to i32
+  %tmp3 = add nuw nsw i32 0, 1
+  %tmp4 = zext i32 %tmp3 to i64
+  %tmp5 = getelementptr [8192 x i32], [8192 x i32] addrspace(3)* @tess_lds, i64 0, i64 %tmp4
+  %tmp6 = bitcast i32 addrspace(3)* %tmp5 to float addrspace(3)*
+  store float %tmp1, float addrspace(3)* %tmp6, align 4
+  %tmp7 = bitcast float %tmp1 to i32
+  %tmp8 = insertelement <4 x i32> undef, i32 %tmp2, i32 0
+  %tmp9 = insertelement <4 x i32> %tmp8, i32 %tmp7, i32 1
+  %tmp10 = insertelement <4 x i32> %tmp9, i32 undef, i32 2
+  %tmp11 = insertelement <4 x i32> %tmp10, i32 undef, i32 3
+  call void @llvm.SI.tbuffer.store.v4i32(<16 x i8> undef, <4 x i32> %tmp11, i32 4, i32 undef, i32 %arg, i32 0, i32 14, i32 4, i32 1, i32 0, i32 1, i32 1, i32 0)
+  ret void
+}
+
+; Function Attrs: nounwind
+declare void @llvm.SI.tbuffer.store.v4i32(<16 x i8>, <4 x i32>, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32) #0
+
+attributes #0 = { nounwind }