[AMDGPU] Ignore call graph node which does not have function info.

[hsmhsm]

Summary

While collecting reachable callees (from kernels), ignore call graph node which
does not have associated function or associated function is not a definition.

[foad]

llvm/test/CodeGen/AMDGPU/replace-lds-by-ptr-call-to-declare-only-func.ll

2

Surely you don't need -amdgpu-enable-lds-replace-with-pointer=true here.

[hsmhsm]

llvm/test/CodeGen/AMDGPU/replace-lds-by-ptr-call-to-declare-only-func.ll

2

Correct, but now the pass is disabled (off by default) because of the two openmp issues reported. Please refer https://reviews.llvm.org/D104962.

[rampitec]

llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp

73

What will happen if we have a call to a real function, but bitcasted/aliased? getFunction() will return nullptr, but the function is still reachable.

[hsmhsm]

llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp

73

We have not yet handled bitcasted/aliased function pointers. We have planned it as a separate patch. We definetely need to relook into it at that point. But, for now, this check is essentailly required. This is the root cause behind the openmp compile time issue reported at https://reviews.llvm.org/D103225.

[rampitec]

llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp

73

I understand. But is it safe if we have a bitcast and you reenable the pass?

[hsmhsm]

llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp

73

If we have bitcast, then this pass will not reach those bitcasted functions. So, it will not replace any LDS used within those functions. Then LDS lowering pass will directly pack those LDS into a module struct which possibly results in unoptimized use of LDS but the program is safe.

[rampitec]

LGTM

llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp

73

OK, this is conservatively correct but suboptimal. That is what I wanted to know.

llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp

SmallPtrSet<Function *, 8> collectReachableCallees(Function *K) {
     SmallVector<CallGraphNode *, 8> CGNStack(df_begin(KCGN), df_end(KCGN));
     SmallPtrSet<CallGraphNode *, 8> VisitedCGNodes;
     while (!CGNStack.empty()) {
       auto *CGN = CGNStack.pop_back_val();
 
       if (!VisitedCGNodes.insert(CGN).second)
         continue;
 
+      // Ignore call graph node which does not have associated function or
+      // associated function is not a definition.
+      if (!CGN->getFunction() || CGN->getFunction()->isDeclaration())
+        continue;
+
       for (auto GI = CGN->begin(), GE = CGN->end(); GI != GE; ++GI) {
         auto *RCB = cast<CallBase>(GI->first.getValue());
         auto *RCGN = GI->second;
 
         if (auto *DCallee = RCGN->getFunction()) {
           ReachableCallees.insert(DCallee);
         } else if (RCB->isIndirectCall()) {
           auto *RCBFTy = RCB->getFunctionType();

llvm/test/CodeGen/AMDGPU/replace-lds-by-ptr-call-to-declare-only-func.ll

+; RUN: opt -S -mtriple=amdgcn--  -amdgpu-replace-lds-use-with-pointer -amdgpu-enable-lds-replace-with-pointer=true < %s | FileCheck %s
+
+; DESCRIPTION:
+;
+; The kernel 'kern' makes a call to declared only function `foo`, hence `foo`
+; is not considered as reachable callee, and is ignored. The function `goo`
+; which uses LDS is not called from kernel 'kern', hence it is also ignored.
+;
+
+; Original LDS should exist.
+; CHECK: @lds = internal local_unnamed_addr addrspace(3) global i32 undef, align 4
+@lds = internal local_unnamed_addr addrspace(3) global i32 undef, align 4
+
+; Pointer should not be created.
+; CHECK-NOT: @lds.ptr = internal unnamed_addr addrspace(3) global i16 undef, align 2
+
+; CHECK: declare i32 @foo()
+declare i32 @foo()
+
+; No change
+define internal void @goo() {
+; CHECK-LABEL: entry:
+; CHECK:   store i32 undef, i32 addrspace(3)* @lds, align 4
+; CHECK:   ret void
+entry:
+  store i32 undef, i32 addrspace(3)* @lds, align 4
+  ret void
+}
+
+; No change
+define weak amdgpu_kernel void @kern() {
+; CHECK-LABEL: entry:
+; CHECK-LABEL:   %nt = call i32 @foo()
+; CHECK-LABEL:   ret void
+entry:
+  %nt = call i32 @foo()
+  ret void
+}