[PATCH] D148199: [AMDGPU] Add a new command line argument amdgpu-atomic-optimizer-use-dpp

[Pravin Jagtap via Phabricator via llvm-commits]

pravinjagtap added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:1121-1122
 
-  if (isPassEnabled(EnableAtomicOptimizations, CodeGenOpt::Less)) {
+  if (isPassEnabled(EnableAtomicOptimizations, CodeGenOpt::Less) ||
+      isPassEnabled(EnableAtomicOptimizationsUsingDPP, CodeGenOpt::Less)) {
     addPass(createAMDGPUAtomicOptimizerPass());
----------------
foad wrote:
> arsenm wrote:
> > Two flags to enable the same pass is bad, and this doesn't change the pass behavior. The behavior change could be a pass parameter.
> > Two flags to enable the same pass is bad
> 
> That's intentional and temporary. It's to avoid a flag-day change for downstream users like LLPC when they switch over to the new option.
> Two flags to enable the same pass is bad, and this doesn't change the pass behavior. The behavior change could be a pass parameter.




================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:1121-1122
 
-  if (isPassEnabled(EnableAtomicOptimizations, CodeGenOpt::Less)) {
+  if (isPassEnabled(EnableAtomicOptimizations, CodeGenOpt::Less) ||
+      isPassEnabled(EnableAtomicOptimizationsUsingDPP, CodeGenOpt::Less)) {
     addPass(createAMDGPUAtomicOptimizerPass());
----------------
pravinjagtap wrote:
> foad wrote:
> > arsenm wrote:
> > > Two flags to enable the same pass is bad, and this doesn't change the pass behavior. The behavior change could be a pass parameter.
> > > Two flags to enable the same pass is bad
> > 
> > That's intentional and temporary. It's to avoid a flag-day change for downstream users like LLPC when they switch over to the new option.
> > Two flags to enable the same pass is bad, and this doesn't change the pass behavior. The behavior change could be a pass parameter.
> 
> 
> The behavior change could be a pass parameter.
If I understand correctly you are suggesting to have `string` here instead of boolean `true` or `false` value as parameter? Something like:

-amdgpu-atomic-optimizer-scan=`dpp` or -amdgpu-atomic-optimizer-scan=`iterative`  and based on the parameter value of this new command line we will be taking different code paths 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148199/new/

https://reviews.llvm.org/D148199