[PATCH] D152649: [AMDGPU] Enable Atomic Optimizer and Default to Iterative Scan Strategy.

Sun Jun 18 23:16:31 PDT 2023

pravinjagtap added a comment.

In D152649#4431309 <https://reviews.llvm.org/D152649#4431309>, @jhuber6 wrote:

> In D152649#4431278 <https://reviews.llvm.org/D152649#4431278>, @b-sumner wrote:
>
>> In D152649#4431258 <https://reviews.llvm.org/D152649#4431258>, @jhuber6 wrote:
>>
>>> @jplehr @ronlieb
>>>
>>> This patch seems to have had catastrophic results on the build times in the `libc` project. See the buildbot where the build times have gone from ~15 minutes to over an hour https://lab.llvm.org/staging/#/builders/247. Reverting this patch causes the time for my to build a test to go from 55 seconds to 5 seconds. I can try to get pass timing later.
>>
>> If this is only affecting AMDGPU and libc, is it a problem?  This optimization is very important.  Of course, it would be great if it can be sped up.
>
> I don't know if it's just a `libc` problem, it probably just highlights the problem since it's a considerably large application with a good number of atomic operations thanks to RPC calls. Maybe @ye-luo can let me know if he's had any compile time regressions recently. In any case a ~10x increase in link time is a little steep for a single optimization. I can always disable this pass in the `libc` build but I think that other large applications are going to see similar changes so we need to think of a way to do this that isn't so slow.

The newly added assertions at https://github.com/llvm/llvm-project/blob/1ebbbf1614cfdbf6d78f4f2a665cdea9cbb2beb8/llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp#L774 and https://github.com/llvm/llvm-project/blob/1ebbbf1614cfdbf6d78f4f2a665cdea9cbb2beb8/llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp#L802 during `https://reviews.llvm.org/D147408` seems to be causing this problem.  Verifying dominator tree is very expensive using assertions like this.  The alternative to this check is `-verify-dom-info` which seems to be broken at this moment. Let me investigate.

The time for running `libc` tests is recovered without these assertions.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152649