AMDGPU: Introduce a flag to control enable/disable instruction sink pass

[cfang]

Summary

Instruction sink pass tries to sink instructions to the lowest possible point, with an effort
to bring instructions closer to their users. But it does not have heuristic regarding actual
benefit to do so. For example, when you sink an instruction, it may increase the live ranges
of it uses.
We have observe a greaterr than 10% performance benefit on some applications if we completely
turn off this pass. A flag to control this pass will allow us to tune the performance. It will also help us
isolate potential correctness issue that may be introduced from the llvm community.

[arsenm]

Just adding another flag isn't really fixing anything

[cfang]

In D89095#2322548, @arsenm wrote:

Just adding another flag isn't really fixing anything

Right, But we are providing a way to do performance tuning as well as triaging bugs.

[nhaehnle]

I agree with Matt here. You should be able to do experiments locally. Perhaps sinking should be disabled entirely, or perhaps sinking should be improved to take register liveness into account.

[cfang]

In D89095#2325401, @nhaehnle wrote:

I agree with Matt here. You should be able to do experiments locally. Perhaps sinking should be disabled entirely, or perhaps sinking should be improved to take register liveness into account.

Right, my point of view is that the Sink pass should be re-evaluated. Since it has been introduced into AMDGPU pipeline many years ago, there must have been unexpected dependencies
on this pass.

Local experiments are absolutely necessary, but not sufficient. I propose to introduce a flag which could enable a broad range of experiments (from CQE for example). And then we can make decisions:

1. if it is mostly negative, we can disable it completely;
2. if it is mostly positive, we can keep it as it is;
3. if it it is in the middle, we can make effort to enhance it;

[nhaehnle]

Why does this require a commit upstream?

[cfang]

In D89095#2325539, @nhaehnle wrote:

Why does this require a commit upstream?

So everyone can use it. Also,
If my understand is correct, every optimization pass should have an associated flag to turn it on/off.

[arsenm]

Assuming this is irrelevant now

[foad]

See also D130170.

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

   cl::init(true),
   cl::Hidden);
 
 static cl::opt<bool> EnableStructurizerWorkarounds(
     "amdgpu-enable-structurizer-workarounds",
     cl::desc("Enable workarounds for the StructurizeCFG pass"), cl::init(true),
     cl::Hidden);
 
+static cl::opt<bool> EnableInstructionSink(
+    "amdgpu-enable-instruction-sink",
+    cl::desc("Enable instruction sink pass"), cl::init(true),
+    cl::Hidden);
+
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAMDGPUTarget() {
   // Register the target
   RegisterTargetMachine<R600TargetMachine> X(getTheAMDGPUTarget());
   RegisterTargetMachine<GCNTargetMachine> Y(getTheGCNTarget());
 
   PassRegistry *PR = PassRegistry::getPassRegistry();
   initializeR600ClauseMergePassPass(*PR);
   initializeR600ControlFlowFinalizerPass(*PR);

   addPass(&AMDGPUUnifyDivergentExitNodesID);
   if (!LateCFGStructurize) {
     if (EnableStructurizerWorkarounds) {
       addPass(createFixIrreduciblePass());
       addPass(createUnifyLoopExitsPass());
     }
     addPass(createStructurizeCFGPass(false)); // true -> SkipUniformRegions
   }
-  addPass(createSinkingPass());
+  if (EnableInstructionSink)
+    addPass(createSinkingPass());
   addPass(createAMDGPUAnnotateUniformValues());
   if (!LateCFGStructurize) {
     addPass(createSIAnnotateControlFlowPass());
   }
   addPass(createLCSSAPass());
 
   return false;
 }