[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());
----------------
pravinjagtap wrote:
> 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 
> 
> and this doesn't change the pass behavior
Keeping the pass behavior same was also intentional with new command line argument. This will buy some time for users to adapt to a new command line argument. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148199