[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

This revision was automatically updated to reflect the committed changes. Closed by commit rGf22ba5187350: [Remarks] Emit optimization remarks for atomics generating CAS loop (authored by gandhi21299). Changed prior to commit: https://reviews.llvm.org/D106891?vs=366683&id=366735#toc Repository

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

arsenm accepted this revision. arsenm added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a comment. Will do, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

rampitec accepted this revision. rampitec added a comment. LGTM, but please wait for others too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___ cfe-commits

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366683. gandhi21299 added a comment. - ORE does not need to be a pointer anymore, it is constructed as local variable with this patch as requested by reviewer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added inline comments. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:175 + ORE = std::make_unique(&F); auto &TM = TPC->getTM(); rampitec wrote: > gandhi21299 wrote: > > rampitec wrote: > > > Is there a reason to construct it upfront and not

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

rampitec added inline comments. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:175 + ORE = std::make_unique(&F); auto &TM = TPC->getTM(); gandhi21299 wrote: > rampitec wrote: > > Is there a reason to construct it upfront and not just use a local variable

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added inline comments. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:175 + ORE = std::make_unique(&F); auto &TM = TPC->getTM(); rampitec wrote: > Is there a reason to construct it upfront and not just use a local variable > only when needed

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

rampitec added inline comments. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:175 + ORE = std::make_unique(&F); auto &TM = TPC->getTM(); Is there a reason to construct it upfront and not just use a local variable only when needed? Like in StackProtecto

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a comment. Alright, please let me know if this patch is good for merge at your convenience. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 __

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

xbolva00 added a comment. Not related to your patch, feel free to ignore Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___ cfe-commits mailing list cfe-commit

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a comment. Any ideas on what could be causing the failure in windows pre-merge checks? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___ cfe

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366517. gandhi21299 added a comment. - eliminated changes in PowerPC/O3 -pipeline.ll, as requested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ http

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

xbolva00 added a comment. Please drop a change in PowerPC/O3 -pipeline.ll Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366514. gandhi21299 added a comment. - fixing breaking tests by eliminating passes that are no longer in the pass pipelines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.or

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366507. gandhi21299 added a reviewer: nikic. gandhi21299 added a comment. - removing analysis requirement as requested + nikic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

nikic added a comment. You also need to drop the `getAnalysisUsage()` change, otherwise the pass manager will still create the ORE itself. PS: This revision still has incorrectly configured permissions, which prevents me from leaving line comments. Revisions on this Phabricator instance should

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366504. gandhi21299 added a comment. reverting type of ORE from `OptimizationRemarkEmitter * ` back to `std::unique_ptr ` and constructing it in AtomicExpand to avoid DTC and LI overhead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a comment. Okay, sorry about that. Thanks for reverting my commit. I will use a unique_ptr and wait for another approval. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

nikic added a comment. In D106891#2945342 , @arsenm wrote: > I don’t think constructing in the pass is the solution. Why exactly is this > introducing such a big slowdown? The reason are the additional DominatorTree and LoopInfo calculations. These hav

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

xbolva00 added a comment. sqlite3 +1.3% increase of compile time for -O0 -g is simply not acceptable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___ cfe-co

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

xbolva00 added a comment. http://llvm-compile-time-tracker.com/compare.php?from=530aa7e4da14fb22493ab7e175f8c34dd10333d3&to=435785214f73ff0c92e97f2ade6356e3ba3bf661&stat=instructions Still same problem with -O0 -g. Please wait a bit for more people, dont rush. Repository: rG LLVM Github Monor

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

This revision was automatically updated to reflect the committed changes. Closed by commit rG435785214f73: [Remarks] Emit optimization remarks for atomics generating CAS loop (authored by gandhi21299). Changed prior to commit: https://reviews.llvm.org/D106891?vs=366467&id=366475#toc Repository

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a comment. @xbolva00 I timed X86/opt-pipeline.ll passes and DTC executed in 0.1% of the total compile time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 _

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366467. gandhi21299 added a comment. reverting changes back to declaring ORE using `getAnalysis` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 Files: clang/te

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a subscriber: nikic. gandhi21299 added a comment. Also, @nikic suggested to construct ORE here if we cannot usefully preserve them. I am not sure if preserving the information is useful though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a comment. I will actually revert my changes back with only the tests updated to see if the times are reasonable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 marked an inline comment as done. gandhi21299 added a comment. @xbolva00 is concerned about Dominator Tree Construction Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

arsenm added a comment. I don’t think constructing in the pass is the solution. Why exactly is this introducing such a big slowdown? Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:180 + ORE = std::make_shared(&F); auto &TM = TPC->getTM(); There’s basi

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366463. gandhi21299 added a comment. Herald added a subscriber: nemanjai. - fixed breaking tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 Files: clang/te

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366455. gandhi21299 added a comment. - changed type of ORE from OptimizationRemarkEmitter* to std::shared_ptr and construct it within AtomicExpandPass, this solution is implemented to address for the regressions in many backends due to prerequisite pass

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

xbolva00 added a comment. http://llvm-compile-time-tracker.com/compare.php?from=1f2d40c47f5f8fd01d91d73a1f52044fe1c83225&to=c4e5425aa579d21530ef1766d7144b38a347f247&stat=instructions Compile time regressions especially for -O0 -g are higher than expected with this patch. Repository: rG LLVM

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4e5425aa579: [Remarks] Emit optimization remarks for atomics generating CAS loop (authored by gandhi21299). Repository: rG LLVM Github Monorepo

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added a comment. Thanks a lot for the review! I will merge this patch in as soon as the CI passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

rampitec accepted this revision. rampitec added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 ___

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366357. gandhi21299 marked an inline comment as done. gandhi21299 added a comment. - combined all tests into one Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 F

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

rampitec added inline comments. Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:33 +float atomic_cas(__global atomic_float *d, float a) { + return __opencl_atomic_fetch_add(d, a, memory_order_relaxed, memory_scope_work_group); +} Just combine all

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366349. gandhi21299 marked an inline comment as done. gandhi21299 added a comment. - adding more tests in clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl for various scopes, memory_scope_work_item is called out as invalid by the compiler so excluded t

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

rampitec added inline comments. Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:32 +// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} syncscope("workgroup-one-as") monotonic +float atomic_cas_system(__global atomic_float *d, float a) { + return __opencl_at

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366343. gandhi21299 added a comment. - removed StringExtras.h inclusion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 Files: clang/test/CodeGenCUDA/atomics-re

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366338. gandhi21299 added a comment. - replaced the OpenCL test - renamed filenames - added 'atomic' to the remark and tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.or

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

rampitec added a comment. Please restore opencl test. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:622 +return OptimizationRemark(DEBUG_TYPE, "Passed", AI->getFunction()) + << "A compare and swap loop was generated for an " + << AI->getO

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 updated this revision to Diff 366311. gandhi21299 added a comment. - eliminated irrelevant changes to this patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106891/new/ https://reviews.llvm.org/D106891 Files: llvm/lib/CodeGen/Atomi

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

gandhi21299 added inline comments. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:622 +return OptimizationRemark(DEBUG_TYPE, "Passed", AI->getFunction()) + << "A compare and swap loop was generated for an " + << AI->getOperationName(AI->getO

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

arsenm added inline comments. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:622 +return OptimizationRemark(DEBUG_TYPE, "Passed", AI->getFunction()) + << "A compare and swap loop was generated for an " + << AI->getOperationName(AI->getOperat