AMDGPU: Run internalize symbols at -O0

[arsenm]

Summary

The relocations used for externally visible functions
aren't supported.

I'm not sure if it would be better to actually try
supporting the relocations.

[t-tye]

AFAIK the relocations are supported but you need to be using the PLT so the relocations are against the data section and not the text section.

[rampitec]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

330

Looks like there is now isEntryFunctionCC() which can be used instead of this switch.

[arsenm]

Use isEntryFunctionCC

[rampitec]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

326

Looks like it changes original logic. All functions were internalized except for declarations and kernels.

[arsenm]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

326

It's the same, also no tests fail

[rampitec]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

326

If a function is called and not declaration it used to be internalized, with this change it will not.

[arsenm]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

326

Yes it is. This is the reason for doing this and is the func_used test I added

[rampitec]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

326

Then what happens if function is finally inlined and no calls left? I suppose it will remain in the object, which is not we really want.

[arsenm]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

326

There is no change here in behavior from what was here before other than also internalizing at -O0. Regardless of whether the function is used or not it is internalized

[rampitec]

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

326

There is change in behavior with the optimizations. You have removed this line to indicate this change:

CHECK-NOT: foo_used

With optimizations foo_used shall not be in the output.

test/CodeGen/AMDGPU/internalize.ll

32–33

I have check prefixes ALL, OPT and OPTNONE. The check prefix CHECK does not exist.

[arsenm]

Fix check prefixes, directly return logical expression

[rampitec]

test/CodeGen/AMDGPU/internalize.ll

32–33

Does it work? Code now seems to be right, but this check shall fail. foo_used is always_inline, it shall be internalized, inlined and eliminated.

[arsenm]

test/CodeGen/AMDGPU/internalize.ll

32–33

This test is actually broken as is. The calling convention for amdgpu_kernel doesn't match the call site calling convention, so this gets turned into trap/unreachable which is why it gets eliminated.

32–33

We probably shouldn't allow calls to amdgpu_kernel for the IR calling convention.

[rampitec]

test/CodeGen/AMDGPU/internalize.ll

32–33

Ouch. This is mistake in the test, it shall be just a function.

[arsenm]

Remove broken test

[rampitec]

Can you please restore foo_used and just fix calling convention? This case is not covered now.

test/CodeGen/AMDGPU/internalize.ll

42

Typo in func name?

[arsenm]

In D37887#874523, @rampitec wrote:

Can you please restore foo_used and just fix calling convention? This case is not covered now.

That's the new func_used I added, which is the same thing.

[arsenm]

Fix names

[rampitec]

In D37887#874537, @arsenm wrote:

In D37887#874523, @rampitec wrote:

Can you please restore foo_used and just fix calling convention? This case is not covered now.

That's the new func_used I added, which is the same thing.

Not really, it is noinline. The old one was always inline with the opposite check condition.

[arsenm]

Add different inline attribute functions

[rampitec]

test/CodeGen/AMDGPU/internalize.ll

26

OPT-NOT, not OPT-NONE.

[arsenm]

Fix check not

[rampitec]

LGTM

[arsenm]

r313616

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

 
 static ImmutablePass *createAMDGPUExternalAAWrapperPass() {
   return createExternalAAWrapperPass([](Pass &P, Function &, AAResults &AAR) {
       if (auto *WrapperPass = P.getAnalysisIfAvailable<AMDGPUAAWrapperPass>())
         AAR.addAAResult(WrapperPass->getResult());
       });
 }
 
+/// Predicate for Internalize pass.
+bool mustPreserveGV(const GlobalValue &GV) {
+  if (const Function *F = dyn_cast<Function>(&GV))
+    return F->isDeclaration() || AMDGPU::isEntryFunctionCC(F->getCallingConv());
+
+  return !GV.use_empty();
+}
+
 void AMDGPUTargetMachine::adjustPassManager(PassManagerBuilder &Builder) {
   Builder.DivergentTarget = true;
 
   bool EnableOpt = getOptLevel() > CodeGenOpt::None;
-  bool Internalize = InternalizeSymbols && EnableOpt &&
-                     (getTargetTriple().getArch() == Triple::amdgcn);
+  bool Internalize = InternalizeSymbols;
   bool EarlyInline = EarlyInlineAll && EnableOpt;
   bool AMDGPUAA = EnableAMDGPUAliasAnalysis && EnableOpt;
   bool LibCallSimplify = EnableLibCallSimplify && EnableOpt;
 
+  if (Internalize) {
+    // If we're generating code, we always have the whole program available. The
+    // relocations expected for externally visible functions aren't supported,
+    // so make sure every non-entry function is hidden.
+    Builder.addExtension(
+      PassManagerBuilder::EP_EnabledOnOptLevel0,
+      [](const PassManagerBuilder &, legacy::PassManagerBase &PM) {
+        PM.add(createInternalizePass(mustPreserveGV));
+      });
+  }
+
   Builder.addExtension(
     PassManagerBuilder::EP_ModuleOptimizerEarly,
     [Internalize, EarlyInline, AMDGPUAA](const PassManagerBuilder &,
                                          legacy::PassManagerBase &PM) {
       if (AMDGPUAA) {
         PM.add(createAMDGPUAAWrapperPass());
         PM.add(createAMDGPUExternalAAWrapperPass());
       }
       PM.add(createAMDGPUUnifyMetadataPass());
       if (Internalize) {
-        PM.add(createInternalizePass([=](const GlobalValue &GV) -> bool {
-          if (const Function *F = dyn_cast<Function>(&GV)) {
-            if (F->isDeclaration())
-                return true;
-            switch (F->getCallingConv()) {
-            default:
-              return false;
-            case CallingConv::AMDGPU_VS:
-            case CallingConv::AMDGPU_HS:
-            case CallingConv::AMDGPU_GS:
-            case CallingConv::AMDGPU_PS:
-            case CallingConv::AMDGPU_CS:
-            case CallingConv::AMDGPU_KERNEL:
-            case CallingConv::SPIR_KERNEL:
-              return true;
-            }
-          }
-          return !GV.use_empty();
-        }));
+        PM.add(createInternalizePass(mustPreserveGV));
         PM.add(createGlobalDCEPass());
       }
       if (EarlyInline)
         PM.add(createAMDGPUAlwaysInlinePass(false));
   });
 
   Builder.addExtension(
     PassManagerBuilder::EP_EarlyAsPossible,

test/CodeGen/AMDGPU/internalize.ll

-; RUN: opt -O1 -S -mtriple=amdgcn-unknown-amdhsa -amdgpu-internalize-symbols < %s | FileCheck %s
-; CHECK-NOT: unused
-; CHECK-NOT: foo_used
-; CHECK: gvar_used
-; CHECK: main_kernel
+; RUN: opt -O1 -S -mtriple=amdgcn-unknown-amdhsa -amdgpu-internalize-symbols < %s | FileCheck -check-prefix=ALL -check-prefix=OPT %s
+; RUN: opt -O0 -S -mtriple=amdgcn-unknown-amdhsa -amdgpu-internalize-symbols < %s | FileCheck -check-prefix=ALL -check-prefix=OPTNONE %s
 
+; OPT-NOT: gvar_unused
+; OPTNONE: gvar_unused
 @gvar_unused = addrspace(1) global i32 undef, align 4
+
+; ALL: gvar_used
 @gvar_used = addrspace(1) global i32 undef, align 4
 
-; Function Attrs: alwaysinline nounwind
-define amdgpu_kernel void @foo_unused(i32 addrspace(1)* %out) local_unnamed_addr #1 {
+; ALL: define internal fastcc void @func_used(
+define fastcc void @func_used(i32 addrspace(1)* %out, i32 %tid) #1 {
+entry:
+  store volatile i32 %tid, i32 addrspace(1)* %out
+  ret void
+}
+
+; ALL: define internal fastcc void @func_used_noinline(
+define fastcc void @func_used_noinline(i32 addrspace(1)* %out, i32 %tid) #2 {
+entry:
+  store volatile i32 %tid, i32 addrspace(1)* %out
+  ret void
+}
+
+; OPTNONE: define internal fastcc void @func_used_alwaysinline(
+; OPT-NOT: @func_used_alwaysinline
+define fastcc void @func_used_alwaysinline(i32 addrspace(1)* %out, i32 %tid) #3 {
 entry:
-  store i32 1, i32 addrspace(1)* %out
+  store volatile i32 %tid, i32 addrspace(1)* %out
   ret void
 }
 
-; Function Attrs: alwaysinline nounwind
-define amdgpu_kernel void @foo_used(i32 addrspace(1)* %out, i32 %tid) local_unnamed_addr #1 {
+; OPTNONE: define internal void @func_unused(
+; OPT-NOT: @func_unused
+define void @func_unused(i32 addrspace(1)* %out, i32 %tid) #2 {
 entry:
-  store i32 %tid, i32 addrspace(1)* %out
+  store volatile i32 %tid, i32 addrspace(1)* %out
   ret void
 }
 
+; ALL: define amdgpu_kernel void @kernel_unused(
+define amdgpu_kernel void @kernel_unused(i32 addrspace(1)* %out) #1 {
+entry:
+  store volatile i32 1, i32 addrspace(1)* %out
+  ret void
+}
+
+; ALL: define amdgpu_kernel void @main_kernel()
+; ALL: tail call i32 @llvm.amdgcn.workitem.id.x
+; ALL: tail call fastcc void @func_used
+; ALL: tail call fastcc void @func_used_noinline
+; ALL: store volatile
+; ALL: ret void
 define amdgpu_kernel void @main_kernel() {
 entry:
-  %tid = call i32 @llvm.amdgcn.workitem.id.x()
-  tail call void @foo_used(i32 addrspace(1)* @gvar_used, i32 %tid) nounwind
+  %tid = tail call i32 @llvm.amdgcn.workitem.id.x()
+  tail call fastcc void @func_used(i32 addrspace(1)* @gvar_used, i32 %tid)
+  tail call fastcc void @func_used_noinline(i32 addrspace(1)* @gvar_used, i32 %tid)
+  tail call fastcc void @func_used_alwaysinline(i32 addrspace(1)* @gvar_used, i32 %tid)
   ret void
 }
 
 declare i32 @llvm.amdgcn.workitem.id.x() #0
 
 attributes #0 = { nounwind readnone }
-
-attributes #1 = { alwaysinline nounwind }
+attributes #1 = { nounwind }
+attributes #2 = { noinline nounwind }
+attributes #3 = { alwaysinline nounwind }