[PATCH] D36831: [AMDGPU] Transform __read_pipe_* and __write_pipe_*

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec requested changes to this revision.
rampitec added a comment.
This revision now requires changes to proceed.

You still fail to use getFunction and thus also fail to check prelink.



================
Comment at: lib/Target/AMDGPU/AMDGPULibCalls.cpp:616
+
+  auto *F = M->getOrInsertFunction(Name, FTy);
+  auto *NCI = B.CreateCall(F, Args);
----------------
yaxunl wrote:
> rampitec wrote:
> > You should not use getOrInsertFunction directly, but use AMDGPULibCalls::getFunction().
> > Since you are not using it you also fail to check we are in pre-link.
> whether in pre-link can be checked by whether the function is declaration.
> 
> `__read_pipe_*` and `__write_pipe_*` are unmangled functions. AMDGPULibFunc::getFunction relies on argument information encoded in mangled function names, which does not work for unmangled functions.
You are still using M->getOrInsertFunction() directly.


================
Comment at: lib/Target/AMDGPU/AMDGPULibCalls.cpp:67
   typedef llvm::AMDGPULibFunc FuncInfo;
+  typedef llvm::AMDGPUMangledLibFunc MangledFuncInfo;
+  typedef llvm::AMDGPUUnmangledLibFunc UnmangledFuncInfo;
----------------
It is better hide these details from the client. AMDGPULibFunc should care about all of it and FuncInfo shall be sufficient structure to describe a function. Simplifier should not care about details.


================
Comment at: lib/Target/AMDGPU/AMDGPULibFunc.cpp:987
+bool UnmangledFuncInfo::lookup(StringRef Name, ID &Id) {
+  for (unsigned I = 0; I != TableSize; ++I) {
+    if (Name == Table[I].Name) {
----------------
I believe we can go without bruteforce search loop.


================
Comment at: lib/Target/AMDGPU/AMDGPULibFunc.h:240
+    // Library functions with unmangled name.
+    EI_READ_PIPE_2,
+    EI_READ_PIPE_4,
----------------
There is already EI_READ_PIPE above. Is it mangled? Then why these are unmangled?


================
Comment at: lib/Target/AMDGPU/AMDGPULibFunc.h:366
 
-  void          reset();
+  static StringRef getUnmangledName(const StringRef &mangledName);
 
----------------
StringRef designed to be passed by value, here and below.


================
Comment at: test/CodeGen/AMDGPU/simplify-libcalls.ll:693
+entry:
+  %0 = bitcast i32 addrspace(1)* %ptr to i8 addrspace(1)*
+  %1 = addrspacecast i8 addrspace(1)* %0 to i8 addrspace(4)*
----------------
Run tests through opt -instnamer.


https://reviews.llvm.org/D36831