[AMDGPU] Don't remove short branches over kills

[foad]

Summary

D68092 introduced a new SIRemoveShortExecBranches optimization pass and
broke some graphics shaders. The problem is that it was removing
branches over KILL pseudo instructions, and the fix is to explicitly
check for that in mustRetainExeczBranch.

[foad]

I realise that this needs a test case, but I wanted to submit it early as a heads-up to anyone else who is having problems with D68092.

[merge_guards_bot]

Unit tests: pass. 62357 tests passed, 0 failed and 839 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[critson]

After some investigation the root cause of this appears to be that in some cases cbranch_execz instructions (relating to kills) are removed which leaves the SIInsertSkips pass unable to determine that a kill is being executed in branching control flow. The result is that SIInsertSkips inserts skipIfDead code where it shouldn't because it is unaware the exec mask does not contain allow active threads. This change is the least invasive solution, but the alternative would be to reengineer SIInsertSkips. In either case this seems to be a blind spot in the skip-if-dead lit tests.

[cdevadas]

I am moving the kill intrinsic handling into a separate pass. It is partially done. Fixing some corner cases now.
SIInsertSkips will eventually go away.
The handling/optimizaton of cbranch_execz insertion has been moved to the new pass SIRemoveShortExecBranches.

[nhaehnle]

Ugh, this is yet another recent regression caused by this rework, and we should fix it quickly. I think this change should go in. Please do add a test though.

[foad]

Add test case from @critson.

[merge_guards_bot]

Unit tests: fail. 62406 tests passed, 1 failed and 839 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_timedmutex_requirements/thread_timedmutex_recursive/try_lock_until.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

llvm/lib/Target/AMDGPU/SIRemoveShortExecBranches.cpp

for (MachineBasicBlock::const_iterator I = MBB.begin(), E = MBB.end();
       // leaving the loop might never be taken when EXEC = 0.
       // Hence we should retain cbranch out of the loop lest it become infinite.
       if (I->isConditionalBranch())
         return true;
 
       if (TII->hasUnwantedEffectsWhenEXECEmpty(*I))
         return true;
 
+      if (TII->isKillTerminator(I->getOpcode()))
+        return true;
+
       // These instructions are potentially expensive even if EXEC = 0.
       if (TII->isSMRD(*I) || TII->isVMEM(*I) || TII->isFLAT(*I) ||
           I->getOpcode() == AMDGPU::S_WAITCNT)
         return true;
 
       ++NumInstr;
       if (NumInstr >= SkipThreshold)
         return true;

llvm/test/CodeGen/AMDGPU/skip-if-dead.ll

 bb8:                                              ; preds = %bb9, %bb4
   store volatile i32 9, i32 addrspace(1)* undef
   ret void
 
 bb9:                                              ; preds = %bb4
   ret void
 }
 
+; CHECK-LABEL: {{^}}cbranch_kill:
+; CHECK-NOT: exp null off, off, off, off done vm
+define amdgpu_ps void @cbranch_kill(i32 inreg %0, <2 x float> %1) {
+.entry:
+  %val0 = extractelement <2 x float> %1, i32 0
+  %val1 = extractelement <2 x float> %1, i32 1
+  %p0 = call float @llvm.amdgcn.interp.p1(float %val0, i32 immarg 0, i32 immarg 1, i32 %0) #2
+  %sample = call float @llvm.amdgcn.image.sample.l.2darray.f32.f32(i32 1, float %p0, float %p0, float %p0, float 0.000000e+00, <8 x i32> undef, <4 x i32> undef, i1 false, i32 0, i32 0)
+  %cond0 = fcmp ugt float %sample, 0.000000e+00
+  br i1 %cond0, label %live, label %kill
+
+kill:
+  call void @llvm.amdgcn.kill(i1 false)
+  br label %export
+
+live:
+  %i0 = call float @llvm.amdgcn.interp.p1(float %val0, i32 immarg 0, i32 immarg 0, i32 %0) #2
+  %i1 = call float @llvm.amdgcn.interp.p2(float %i0, float %val1, i32 immarg 0, i32 immarg 0, i32 %0) #2
+  %i2 = call float @llvm.amdgcn.interp.p1(float %val0, i32 immarg 1, i32 immarg 0, i32 %0) #2
+  %i3 = call float @llvm.amdgcn.interp.p2(float %i2, float %val1, i32 immarg 1, i32 immarg 0, i32 %0) #2
+  %scale.i0 = fmul reassoc nnan nsz arcp contract float %i0, %sample
+  %scale.i1 = fmul reassoc nnan nsz arcp contract float %i1, %sample
+  %scale.i2 = fmul reassoc nnan nsz arcp contract float %i2, %sample
+  %scale.i3 = fmul reassoc nnan nsz arcp contract float %i3, %sample
+  br label %export
+
+export:
+  %proxy.0.0 = phi float [ undef, %kill ], [ %scale.i0, %live ]
+  %proxy.0.1 = phi float [ undef, %kill ], [ %scale.i1, %live ]
+  %proxy.0.2 = phi float [ undef, %kill ], [ %scale.i2, %live ]
+  %proxy.0.3 = phi float [ undef, %kill ], [ %scale.i3, %live ]
+  %out.0 = call <2 x half> @llvm.amdgcn.cvt.pkrtz(float %proxy.0.0, float %proxy.0.1) #2
+  %out.1 = call <2 x half> @llvm.amdgcn.cvt.pkrtz(float %proxy.0.2, float %proxy.0.3) #2
+  call void @llvm.amdgcn.exp.compr.v2f16(i32 immarg 0, i32 immarg 15, <2 x half> %out.0, <2 x half> %out.1, i1 immarg true, i1 immarg true) #3
+  ret void
+}
+
+declare float @llvm.amdgcn.interp.p1(float, i32 immarg, i32 immarg, i32) #2
+declare float @llvm.amdgcn.interp.p2(float, float, i32 immarg, i32 immarg, i32) #2
+declare void @llvm.amdgcn.exp.compr.v2f16(i32 immarg, i32 immarg, <2 x half>, <2 x half>, i1 immarg, i1 immarg) #3
+declare <2 x half> @llvm.amdgcn.cvt.pkrtz(float, float) #2
+declare float @llvm.amdgcn.image.sample.l.2darray.f32.f32(i32 immarg, float, float, float, float, <8 x i32>, <4 x i32>, i1 immarg, i32 immarg, i32 immarg) #1
 declare <4 x float> @llvm.amdgcn.image.sample.c.1d.v4f32.f32(i32, float, float, <8 x i32>, <4 x i32>, i1, i32, i32) #1
 declare void @llvm.amdgcn.kill(i1) #0
 
 attributes #0 = { nounwind }
 attributes #1 = { nounwind readonly }
+attributes #2 = { nounwind readnone speculatable }
+attributes #3 = { inaccessiblememonly nounwind writeonly }