[PATCH] D102954: [AMDGPU] Lower kernel LDS into a sorted structure

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added a comment.

Couple of minor comments above, overall approach looks sound to me



================
Comment at: llvm/lib/IR/Value.cpp:535
 
+void Value::replaceUsesWithIf(Value *New,
+                              llvm::function_ref<bool(Use &U)> ShouldReplace) {
----------------
since this hazard affects all the targets, we should probably fix it outside of the amdgpu specific LDS patch


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:37
+
+  if (auto *C = dyn_cast<ConstantExpr>(U)) {
+    return all_of(U->users(), [F](const User *U) {
----------------
Is this safe (enough) for space? Wonder if it's another place for a heap allocated worklist


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

https://reviews.llvm.org/D102954