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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPULibCalls.cpp:636
+  FInfo.setFunctionType(FTy);
+  auto *F = AMDGPULibFunc::getOrInsertFunction(M, FInfo);
+  auto *NCI = B.CreateCall(F, Args);
----------------
The point of using it is to check its result and bail if null. Also no modifications shall be done before this check.


================
Comment at: lib/Target/AMDGPU/AMDGPULibCalls.cpp:67
   typedef llvm::AMDGPULibFunc FuncInfo;
+  typedef llvm::AMDGPUMangledLibFunc MangledFuncInfo;
+  typedef llvm::AMDGPUUnmangledLibFunc UnmangledFuncInfo;
----------------
yaxunl wrote:
> rampitec wrote:
> > 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.
> The handling of mangled lib function requires function argument information, which is not available for unmangled lib function.
> 
> The transformation of unmangled lib function does not require function argument information. There is no point of implementing many of the member functions of mangled lib functions.
The whole point of moving logic to distinguish between mangled and unmangled into the parser is to have no massive changes in the client like this and to let client to not bother differentiating on every other line.


================
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)*
----------------
rampitec wrote:
> yaxunl wrote:
> > rampitec wrote:
> > > Run tests through opt -instnamer.
> > will fix.
> Not done.
I still see it.


https://reviews.llvm.org/D36831