[AMDGPU] Add VGPR copies post regalloc fix pass

[rampitec]

Summary

Regalloc creates COPY instructions which do not formally use VALU.
That results in v_mov instructions displaced after exec mask modification.
One pass which do it is SIOptimizeExecMasking, but potentially it can be
done by other passes too.

This patch adds a pass immediately after regalloc to add implicit exec
use operand to all VGPR copy instructions.

[vpykhtin]

Looks good, but is there a way to customize COPY insertions without addtitional pass?

[rampitec]

In D28874#650332, @vpykhtin wrote:

Looks good, but is there a way to customize COPY insertions without addtitional pass?

Not without patching regalloc and other places.

[arsenm]

Why is this necessary? I've been trying to avoid this. This isn't a question for only copies, any of the generic instructions need to be usually considered VALU. With exec writes as scheduling barriers, I don't think this should be needed

[arsenm]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

619 ↗

(On Diff #84892)

I would also expect if we need this that it would more important for pre-regalloc. We also can't generally expect to add these to every copy that any generic pass is allowed to produce

[rampitec]

The problem is that RA inserts these copy instructions, so doing it before RA does not help. Here is the result:

s_and_saveexec_b64 s[4:5], vcc                             // 0000000170EC: BE84206A
v_mov_b32_e32 v149, v15                                    // 0000000170F0: 7F2A030F
v_mov_b32_e32 v148, v14                                    // 0000000170F4: 7F28030E
v_mov_b32_e32 v14, v16                                     // 0000000170F8: 7E1C0310
v_mov_b32_e32 v192, v21                                    // 0000000170FC: 7F800315
v_mov_b32_e32 v15, v17                                     // 000000017100: 7E1E0311
v_mov_b32_e32 v191, v20                                    // 000000017104: 7F7E0314
v_mov_b32_e32 v62, v11                                     // 000000017108: 7E7C030B
v_mov_b32_e32 v156, v13                                    // 00000001710C: 7F38030D
v_mov_b32_e32 v16, v86                                     // 000000017110: 7E200356
v_mov_b32_e32 v143, v230                                   // 000000017114: 7F1E03E6
v_mov_b32_e32 v20, v236                                    // 000000017118: 7E2803EC
s_xor_b64 s[4:5], exec, s[4:5]                             // 00000001711C: 8884047E
v_mov_b32_e32 v61, v10                                     // 000000017120: 7E7A030A
v_mov_b32_e32 v155, v12                                    // 000000017124: 7F36030C
v_mov_b32_e32 v17, v87                                     // 000000017128: 7E220357
v_mov_b32_e32 v144, v231                                   // 00000001712C: 7F2003E7
v_mov_b32_e32 v21, v237                                    // 000000017130: 7E2A03ED
s_cbranch_execz BB1_28                                     // 000000017134: BF880138

This is not a question of scheduling, where scheduling barrier may help. The problem is that generic instructions do not have dependency on exec.
To fix the above code I can make a special check inside SIOptimizeExecMasking, but this is less general and more a hack than adding required dependency.

[rampitec]

Ping.

[vpykhtin]

I think we can try to go with it as this is affect correctness until we find better solution.

llvm/trunk/lib/Target/AMDGPU/AMDGPU.h

 extern char &SIFoldOperandsID;
 
 void initializeSIShrinkInstructionsPass(PassRegistry&);
 extern char &SIShrinkInstructionsID;
 
 void initializeSIFixSGPRCopiesPass(PassRegistry &);
 extern char &SIFixSGPRCopiesID;
 
+void initializeSIFixVGPRCopiesPass(PassRegistry &);
+extern char &SIFixVGPRCopiesID;
+
 void initializeSILowerI1CopiesPass(PassRegistry &);
 extern char &SILowerI1CopiesID;
 
 void initializeSILoadStoreOptimizerPass(PassRegistry &);
 extern char &SILoadStoreOptimizerID;
 
 void initializeSIWholeQuadModePass(PassRegistry &);
 extern char &SIWholeQuadModeID;

llvm/trunk/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

 extern "C" void LLVMInitializeAMDGPUTarget() {
   // Register the target
   RegisterTargetMachine<R600TargetMachine> X(getTheAMDGPUTarget());
   RegisterTargetMachine<GCNTargetMachine> Y(getTheGCNTarget());
 
   PassRegistry *PR = PassRegistry::getPassRegistry();
   initializeSILowerI1CopiesPass(*PR);
   initializeSIFixSGPRCopiesPass(*PR);
+  initializeSIFixVGPRCopiesPass(*PR);
   initializeSIFoldOperandsPass(*PR);
   initializeSIShrinkInstructionsPass(*PR);
   initializeSIFixControlFlowLiveIntervalsPass(*PR);
   initializeSILoadStoreOptimizerPass(*PR);
   initializeAMDGPUAnnotateKernelFeaturesPass(*PR);
   initializeAMDGPUAnnotateUniformValuesPass(*PR);
   initializeAMDGPUPromoteAllocaPass(*PR);
   initializeAMDGPUCodeGenPreparePass(*PR);
void GCNPassConfig::addOptimizedRegAlloc(FunctionPass *RegAllocPass) {
   // TwoAddressInstructions, otherwise the processing of the tied operand of
   // SI_ELSE will introduce a copy of the tied operand source after the else.
   insertPass(&PHIEliminationID, &SILowerControlFlowID, false);
 
   TargetPassConfig::addOptimizedRegAlloc(RegAllocPass);
 }
 
 void GCNPassConfig::addPostRegAlloc() {
+  addPass(&SIFixVGPRCopiesID);
   addPass(&SIOptimizeExecMaskingID);
   TargetPassConfig::addPostRegAlloc();
 }
 
 void GCNPassConfig::addPreSched2() {
 }
 
 void GCNPassConfig::addPreEmitPass() {

llvm/trunk/lib/Target/AMDGPU/CMakeLists.txt

add_llvm_target(AMDGPUCodeGen
   R600MachineScheduler.cpp
   R600OptimizeVectorRegisters.cpp
   R600Packetizer.cpp
   R600RegisterInfo.cpp
   SIAnnotateControlFlow.cpp
   SIDebuggerInsertNops.cpp
   SIFixControlFlowLiveIntervals.cpp
   SIFixSGPRCopies.cpp
+  SIFixVGPRCopies.cpp
   SIFoldOperands.cpp
   SIFrameLowering.cpp
   SIInsertSkips.cpp
   SIInsertWaits.cpp
   SIInstrInfo.cpp
   SIISelLowering.cpp
   SILoadStoreOptimizer.cpp
   SILowerControlFlow.cpp

llvm/trunk/lib/Target/AMDGPU/SIFixVGPRCopies.cpp

+//===-- SIFixVGPRCopies.cpp - Fix VGPR Copies after regalloc --------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file
+/// \brief Add implicit use of exec to vector register copies.
+///
+//===----------------------------------------------------------------------===//
+
+#include "AMDGPU.h"
+#include "AMDGPUSubtarget.h"
+#include "SIInstrInfo.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "si-fix-vgpr-copies"
+
+namespace {
+
+class SIFixVGPRCopies : public MachineFunctionPass {
+public:
+  static char ID;
+
+public:
+  SIFixVGPRCopies() : MachineFunctionPass(ID) {
+    initializeSIFixVGPRCopiesPass(*PassRegistry::getPassRegistry());
+  }
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+
+  StringRef getPassName() const override { return "SI Fix VGPR copies"; }
+};
+
+} // End anonymous namespace.
+
+INITIALIZE_PASS(SIFixVGPRCopies, DEBUG_TYPE, "SI Fix VGPR copies", false, false)
+
+char SIFixVGPRCopies::ID = 0;
+
+char &llvm::SIFixVGPRCopiesID = SIFixVGPRCopies::ID;
+
+bool SIFixVGPRCopies::runOnMachineFunction(MachineFunction &MF) {
+  const SISubtarget &ST = MF.getSubtarget<SISubtarget>();
+  const SIRegisterInfo *TRI = ST.getRegisterInfo();
+  const SIInstrInfo *TII = ST.getInstrInfo();
+  bool Changed = false;
+
+  for (MachineBasicBlock &MBB : MF) {
+    for (MachineInstr &MI : MBB) {
+      switch (MI.getOpcode()) {
+      case AMDGPU::COPY:
+        if (TII->isVGPRCopy(MI) && !MI.readsRegister(AMDGPU::EXEC, TRI)) {
+          MI.addOperand(MF,
+                        MachineOperand::CreateReg(AMDGPU::EXEC, false, true));
+          DEBUG(dbgs() << "Add exec use to " << MI);
+          Changed = true;
+        }
+        break;
+      default:
+        break;
+      }
+    }
+  }
+
+  return Changed;
+}

llvm/trunk/test/CodeGen/AMDGPU/fix-vgpr-copies.mir

+# RUN: llc -march=amdgcn -start-after=greedy -stop-after=si-optimize-exec-masking -o - %s | FileCheck %s
+# Check that we first do all vector instructions and only then change exec
+# CHECK-DAG:  COPY %vgpr10_vgpr11
+# CHECK-DAG:  COPY %vgpr12_vgpr13
+# CHECK:      %exec = COPY
+
+---
+name:            main
+alignment:       0
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+liveins:
+  - { reg: '%sgpr4_sgpr5' }
+  - { reg: '%sgpr6' }
+  - { reg: '%vgpr0' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    4
+  adjustsStack:    false
+  hasCalls:        false
+  maxCallFrameSize: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+body:             |
+  bb.0.entry:
+    liveins: %vgpr3, %vgpr10_vgpr11, %vgpr12_vgpr13
+
+    %vcc = V_CMP_NE_U32_e64 0, killed %vgpr3, implicit %exec
+    %sgpr4_sgpr5 = COPY %exec, implicit-def %exec
+    %sgpr6_sgpr7 = S_AND_B64 %sgpr4_sgpr5, killed %vcc, implicit-def dead %scc
+    %sgpr4_sgpr5 = S_XOR_B64 %sgpr6_sgpr7, killed %sgpr4_sgpr5, implicit-def dead %scc
+    %vgpr61_vgpr62 = COPY %vgpr10_vgpr11
+    %vgpr155_vgpr156 = COPY %vgpr12_vgpr13
+    %exec = S_MOV_B64_term killed %sgpr6_sgpr7
+...