[PATCH] D129813: [AMDGPU] Add IPO pass to infer pointer argument address spaces. WIP.

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 14 23:40:26 PDT 2022


rampitec added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/InferArgumentAddressSpaces.cpp:139
+                      << "' to address space " << I.second << '\n');
+
+    PointerType *PT = dyn_cast<PointerType>(Arg->getType());
----------------
rampitec wrote:
> arsenm wrote:
> > It would be better to replace the actual argument list with the new type too, and update all the users
> This would be a lot of code, at least copy most of the stuff from InferAddressSpaces itself, cloning function, updating calls, updating SCC...
> 
> Is it worth it? I.e. what do your think it would make better?
> 
> In any way, if needed it is better to go in a separate change.
Actually I see how it can be beneficial by avoiding an actual cast of a pointer before the call and inside the function, and also giving a freedom to move this pass in the pipeline untieing it from the InferAddressSpaces.

Though I would really like to separate such changes simply because actual signature changing is massive by itself. I think it will need a big portion of the InferAddressSpaces to be extracted into a separate utility, the stuff which actually rewrite instructions.


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

https://reviews.llvm.org/D129813



More information about the llvm-commits mailing list