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

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 13 02:38:35 PDT 2023


foad added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/atomic_load_add.ll:1
-; RUN: llc -march=amdgcn -amdgpu-atomic-optimizations=false -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,SICIVI,FUNC %s
-; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global -amdgpu-atomic-optimizations=false -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,SICIVI,FUNC %s
-; RUN: llc -march=amdgcn -mcpu=gfx900 -amdgpu-atomic-optimizations=false -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX9,FUNC %s
-; RUN: llc -march=r600 -mcpu=redwood -amdgpu-atomic-optimizations=false < %s | FileCheck -check-prefixes=R600,FUNC %s
+; RUN: llc -march=amdgcn -amdgpu-atomic-optimizer-use-dpp=false -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,SICIVI,FUNC %s
+; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global -amdgpu-atomic-optimizer-use-dpp=false -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,SICIVI,FUNC %s
----------------
pravinjagtap wrote:
> foad wrote:
> > I guess these tests really want to disable the atomic optimizer pass, to avoid widespread changes to the generated code. If so, they should continue using -amdgpu-atomic-optimizations and we should continue to support that option with its current meaning.
> If I understand correctly, the tests which uses -amdgpu-atomic-optimizations=*true* can be updated with a new option? whereas the tests which uses -amdgpu-atomic-optimizations=*false* should continue to support old option? 
> 
> I thought we will be deprecating -amdgpu-atomic-optimizations option completely and Atomic Optimizer Pass will depend on only a new option -amdgpu-atomic-optimizer-use-dpp.
> If I understand correctly, the tests which uses -amdgpu-atomic-optimizations=*true* can be updated with a new option? whereas the tests which uses -amdgpu-atomic-optimizations=*false* should continue to support old option?

Yes that makes sense.

> I thought we will be deprecating -amdgpu-atomic-optimizations option completely

That was my first suggestion, but if we have lots of tests that use it then maybe we should keep it just for the tests? I don't feel too strongly about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148199



More information about the llvm-commits mailing list