[PATCH] D135260: [AMDGPU][AMDGPULowerKernelAttributes] Use stripAndAccumulateConstantOffset
Juan Manuel Martinez CaamaƱo via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 5 06:14:33 PDT 2022
jmmartinez added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp:84
+ return M.getFunction(Name);
+}
+
----------------
jdoerfert wrote:
> This function and uses are unrelated.
I'm sorry, I do not understand this remark. What do you mean?
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp:89
+ return L->isSimple() ? L : nullptr;
+ }
+
----------------
jdoerfert wrote:
> This does break with the "unique" property below. Two users, one simple one not, will result in a load. Is that expected?
It's ok if there is another non-simple `load`.
In fact, this pass could be modified to support multiple loads. My guess is that for simplicity only one is supported for the moment.
I renamed the function instead into `getUniqueSimpleLoadUser`
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp:138
continue;
+ }
----------------
jdoerfert wrote:
> I would use the `Attributor::checkForAllUses`, also unsure why the loads need to be unique. Depending on the call `AAPointerInfo` would allow you to handle way more things without any of this manual tracing. Either seems to be a useful rewrite beyond the scope of this. Feel free to ignore for now.
I'm not familiar with it. I'm going to take a look at it. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135260/new/
https://reviews.llvm.org/D135260
More information about the llvm-commits
mailing list