[AMDGPU] Add an AMDGPU pass to promote bitcast calls

[scott.linder]

Summary

AMDGPU only supports direct calls, but at lower optimization levels it fails to lower statically direct calls which appear indirect due to a bitcast, e.g. calls of the form call ... bitcast (... @func to ...)(...)

Update CallPromotionUtils to handle both bitcast and pointer cast of argument and return types and use it in an AMDGPU pass to promote all possible calls before inlining.

[rampitec]

LGTM

[scott.linder]

@mssimpso Are you OK with reviewing the changes to CallPromotionUtils.cpp in this patch? I wasn't sure who the correct code owner was.

[mssimpso]

In D52741#1251525, @scott.linder wrote:

@mssimpso Are you OK with reviewing the changes to CallPromotionUtils.cpp in this patch? I wasn't sure who the correct code owner was.

I'm not sure there is a code owner, but the CallPromotionUtils part looks good to me.

[scott.linder]

In D52741#1251534, @mssimpso wrote:

In D52741#1251525, @scott.linder wrote:

@mssimpso Are you OK with reviewing the changes to CallPromotionUtils.cpp in this patch? I wasn't sure who the correct code owner was.

I'm not sure there is a code owner, but the CallPromotionUtils part looks good to me.

Sounds good, thank you for taking a look.

[arsenm]

How does this differ from WebAssemblyFixFunctionBitcasts? Should we have a common pass for this?

The generic utils changes are also a separate patch

lib/Target/AMDGPU/AMDGPUFixFunctionBitcasts.cpp

31–33

This can be dropped

test/CodeGen/AMDGPU/call-constexpr.ll

2

Should have an opt run of just the pass

[scott.linder]

The WebAsm code essentially does the same thing, but also creates wrapper functions when the number of arguments differ, or when the return value differs (it seems like this could just be a cast as well). I will move the WebAsm pass to lib/Transform and use it in both backends. I am not sure who from WebAsm to ask to take a look once I'm done?

[scott.linder]

After reading the WebAsm code more closely it seems like our goals are different. They seem to support indirect calls, but only if certain properties of the functions match (namely number of arguments and return type). For example, the existing pass does not transform anything (e.g. leaves a bitcast function call present) if the bitcast only modifies argument types in a trivial way. For AMDGPU we need to always eliminate the bitcast, and we don't necessarily need to be generating wrapper functions.

@sbc100 I am not sure who to ask, but you touched the WebAssemblyFixFunctionBitcasts pass recently; is my assessment of the intent of the pass accurate?

[sbc100]

Your assessment is about right yes. We assembly supports indirect calls but the function signatures have to match in the webassembly type system. In fact the executable won't even validate (won't load) if it contains such invalid calls. So it sounds like the goals are not the same.

[scott.linder]

Move CallPromotionUtils changes to new review (https://reviews.llvm.org/D52792)

Address feedback, and also remove call to setPreservesCFG as CallPromotionUtils can modify the CFG.

[scott.linder]

Thanks, Sam. I think the requirements of the two passes are different enough that combining them is not particularly useful.

[arsenm]

lib/Target/AMDGPU/AMDGPUFixFunctionBitcasts.cpp

36

Should use visitCallSite to handle invokes as well

38–39

How can this happen?

[scott.linder]

lib/Target/AMDGPU/AMDGPUFixFunctionBitcasts.cpp

38–39

I'm not entirely sure, but the same check is present in CallSite::isIndirectCall() so I assumed there was a reason. Looking at the implementation I don't see how it is possible, so I will remove it.

[scott.linder]

Address feedback

[scott.linder]

Ping

[scott.linder]

Ping

[scott.linder]

Add test for promoting invoke instruction

[arsenm]

LGTM

[scott.linder]

Move addPass for new pass

[arsenm]

LGTM

lib/Target/AMDGPU/AMDGPU.h

 Pass *createAMDGPUAnnotateKernelFeaturesPass();
 void initializeAMDGPUAnnotateKernelFeaturesPass(PassRegistry &);
 extern char &AMDGPUAnnotateKernelFeaturesID;
 
 ModulePass *createAMDGPULowerIntrinsicsPass();
 void initializeAMDGPULowerIntrinsicsPass(PassRegistry &);
 extern char &AMDGPULowerIntrinsicsID;
 
+ModulePass *createAMDGPUFixFunctionBitcastsPass();
+void initializeAMDGPUFixFunctionBitcastsPass(PassRegistry &);
+extern char &AMDGPUFixFunctionBitcastsID;
+
 FunctionPass *createAMDGPULowerKernelArgumentsPass();
 void initializeAMDGPULowerKernelArgumentsPass(PassRegistry &);
 extern char &AMDGPULowerKernelArgumentsID;
 
 ModulePass *createAMDGPULowerKernelAttributesPass();
 void initializeAMDGPULowerKernelAttributesPass(PassRegistry &);
 extern char &AMDGPULowerKernelAttributesID;
 

lib/Target/AMDGPU/AMDGPUFixFunctionBitcasts.cpp

+//===-- AMDGPUFixFunctionBitcasts.cpp - Fix function bitcasts -------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Promote indirect (bitcast) calls to direct calls when they are statically
+/// known to be direct. Required when InstCombine is not run (e.g. at OptNone)
+/// because AMDGPU does not support indirect calls.
+///
+//===----------------------------------------------------------------------===//
+
+#include "AMDGPU.h"
+#include "llvm/IR/InstVisitor.h"
+#include "llvm/Transforms/Utils/CallPromotionUtils.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "amdgpu-fix-function-bitcasts"
+
+namespace {
+class AMDGPUFixFunctionBitcasts final
+    : public ModulePass,
+      public InstVisitor<AMDGPUFixFunctionBitcasts> {
+
+  bool runOnModule(Module &M) override;
+
+  bool Modified;
+
+public:
+  void visitCallSite(CallSite CS) {
+    if (CS.getCalledFunction())
+      return;
+    auto Callee = dyn_cast<Function>(CS.getCalledValue()->stripPointerCasts());
+    if (Callee && isLegalToPromote(CS, Callee)) {
+      promoteCall(CS, Callee);
+      Modified = true;
+    }
+  }
+
+  static char ID;
+  AMDGPUFixFunctionBitcasts() : ModulePass(ID) {}
+};
+} // End anonymous namespace
+
+char AMDGPUFixFunctionBitcasts::ID = 0;
+char &llvm::AMDGPUFixFunctionBitcastsID = AMDGPUFixFunctionBitcasts::ID;
+INITIALIZE_PASS(AMDGPUFixFunctionBitcasts, DEBUG_TYPE,
+                "Fix function bitcasts for AMDGPU", false, false)
+
+ModulePass *llvm::createAMDGPUFixFunctionBitcastsPass() {
+  return new AMDGPUFixFunctionBitcasts();
+}
+
+bool AMDGPUFixFunctionBitcasts::runOnModule(Module &M) {
+  Modified = false;
+  visit(M);
+  return Modified;
+}

lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

extern "C" void LLVMInitializeAMDGPUTarget() {
   initializeSILowerI1CopiesPass(*PR);
   initializeSIFixSGPRCopiesPass(*PR);
   initializeSIFixVGPRCopiesPass(*PR);
   initializeSIFoldOperandsPass(*PR);
   initializeSIPeepholeSDWAPass(*PR);
   initializeSIShrinkInstructionsPass(*PR);
   initializeSIOptimizeExecMaskingPreRAPass(*PR);
   initializeSILoadStoreOptimizerPass(*PR);
+  initializeAMDGPUFixFunctionBitcastsPass(*PR);
   initializeAMDGPUAlwaysInlinePass(*PR);
   initializeAMDGPUAnnotateKernelFeaturesPass(*PR);
   initializeAMDGPUAnnotateUniformValuesPass(*PR);
   initializeAMDGPUArgumentUsageInfoPass(*PR);
   initializeAMDGPULowerKernelArgumentsPass(*PR);
   initializeAMDGPULowerKernelAttributesPass(*PR);
   initializeAMDGPULowerIntrinsicsPass(*PR);
   initializeAMDGPUOpenCLEnqueuedBlockLoweringPass(*PR);

 void AMDGPUPassConfig::addIRPasses() {
   const AMDGPUTargetMachine &TM = getAMDGPUTargetMachine();
 
   // There is no reason to run these.
   disablePass(&StackMapLivenessID);
   disablePass(&FuncletLayoutID);
   disablePass(&PatchableFunctionID);
 
+  // This must occur before inlining, as the inliner will not look through
+  // bitcast calls.
+  addPass(createAMDGPUFixFunctionBitcastsPass());
+
   addPass(createAMDGPULowerIntrinsicsPass());
 
   if (TM.getTargetTriple().getArch() == Triple::r600 ||
       !EnableAMDGPUFunctionCalls) {
     // Function calls are not supported, so make sure we inline everything.
     addPass(createAMDGPUAlwaysInlinePass());
     addPass(createAlwaysInlinerLegacyPass());
     // We need to add the barrier noop pass, otherwise adding the function

lib/Target/AMDGPU/CMakeLists.txt

add_llvm_target(AMDGPUCodeGen
   AMDGPUAliasAnalysis.cpp
   AMDGPUAlwaysInlinePass.cpp
   AMDGPUAnnotateKernelFeatures.cpp
   AMDGPUAnnotateUniformValues.cpp
   AMDGPUArgumentUsageInfo.cpp
   AMDGPUAsmPrinter.cpp
   AMDGPUCallLowering.cpp
   AMDGPUCodeGenPrepare.cpp
+  AMDGPUFixFunctionBitcasts.cpp
   AMDGPUFrameLowering.cpp
   AMDGPUHSAMetadataStreamer.cpp
   AMDGPUInstrInfo.cpp
   AMDGPUInstructionSelector.cpp
   AMDGPUIntrinsicInfo.cpp
   AMDGPUISelDAGToDAG.cpp
   AMDGPUISelLowering.cpp
   AMDGPULegalizerInfo.cpp

test/CodeGen/AMDGPU/call-constexpr.ll

+; RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -amdgpu-fix-function-bitcasts < %s | FileCheck -check-prefix=OPT %s
+
+; GCN-LABEL: {{^}}test_bitcast_return_type_noinline:
+; GCN: s_getpc_b64
+; GCN: s_add_u32 s{{[0-9]+}}, s{{[0-9]+}}, ret_i32_noinline@rel32@lo+4
+; GCN: s_addc_u32 s{{[0-9]+}}, s{{[0-9]+}}, ret_i32_noinline@rel32@hi+4
+; GCN: s_swappc_b64
+; OPT-LABEL: @test_bitcast_return_type_noinline(
+; OPT: %val = call i32 @ret_i32_noinline()
+; OPT: bitcast i32 %val to float
+define amdgpu_kernel void @test_bitcast_return_type_noinline() #0 {
+  %val = call float bitcast (i32()* @ret_i32_noinline to float()*)()
+  %op = fadd float %val, 1.0
+  store volatile float %op, float addrspace(1)* undef
+  ret void
+}
+
+; GCN-LABEL: {{^}}test_bitcast_return_type_alwaysinline:
+; GCN-NOT: s_getpc_b64
+; GCN-NOT: s_add_u32 s{{[0-9]+}}, s{{[0-9]+}}, ret_i32_alwaysinline@rel32@lo+4
+; GCN-NOT: s_addc_u32 s{{[0-9]+}}, s{{[0-9]+}}, ret_i32_alwaysinline@rel32@hi+4
+; GCN-NOT: s_swappc_b64
+; OPT-LABEL: @test_bitcast_return_type_alwaysinline(
+; OPT: %val = call i32 @ret_i32_alwaysinline()
+; OPT: bitcast i32 %val to float
+define amdgpu_kernel void @test_bitcast_return_type_alwaysinline() #0 {
+  %val = call float bitcast (i32()* @ret_i32_alwaysinline to float()*)()
+  %op = fadd float %val, 1.0
+  store volatile float %op, float addrspace(1)* undef
+  ret void
+}
+
+; GCN-LABEL: {{^}}test_bitcast_argument_type:
+; GCN: s_getpc_b64
+; GCN: s_add_u32 s{{[0-9]+}}, s{{[0-9]+}}, ident_i32@rel32@lo+4
+; GCN: s_addc_u32 s{{[0-9]+}}, s{{[0-9]+}}, ident_i32@rel32@hi+4
+; GCN: s_swappc_b64
+; OPT-LABEL: @test_bitcast_argument_type(
+; OPT: %1 = bitcast float 2.000000e+00 to i32
+; OPT: %val = call i32 @ident_i32(i32 %1)
+; OPT-NOT: bitcast i32 %val to float
+define amdgpu_kernel void @test_bitcast_argument_type() #0 {
+  %val = call i32 bitcast (i32(i32)* @ident_i32 to i32(float)*)(float 2.0)
+  %op = add i32 %val, 1
+  store volatile i32 %op, i32 addrspace(1)* undef
+  ret void
+}
+
+; GCN-LABEL: {{^}}test_bitcast_argument_and_return_types:
+; GCN: s_getpc_b64
+; GCN: s_add_u32 s{{[0-9]+}}, s{{[0-9]+}}, ident_i32@rel32@lo+4
+; GCN: s_addc_u32 s{{[0-9]+}}, s{{[0-9]+}}, ident_i32@rel32@hi+4
+; GCN: s_swappc_b64
+; OPT-LABEL: @test_bitcast_argument_and_return_types(
+; OPT: %1 = bitcast float 2.000000e+00 to i32
+; OPT: %val = call i32 @ident_i32(i32 %1)
+; OPT: bitcast i32 %val to float
+define amdgpu_kernel void @test_bitcast_argument_and_return_types() #0 {
+  %val = call float bitcast (i32(i32)* @ident_i32 to float(float)*)(float 2.0)
+  %op = fadd float %val, 1.0
+  store volatile float %op, float addrspace(1)* undef
+  ret void
+}
+
+; GCN-LABEL: {{^}}use_workitem_id_x:
+; GCN: s_waitcnt
+; GCN-NEXT: v_add_i32_e32 v0, vcc, v1, v0
+; GCN-NEXT: s_setpc_b64
+define i32 @use_workitem_id_x(i32 %arg0) #0 {
+  %id = call i32 @llvm.amdgcn.workitem.id.x()
+  %op = add i32 %id, %arg0
+  ret i32 %op
+}
+
+; GCN-LABEL: {{^}}test_bitcast_use_workitem_id_x:
+; GCN: v_mov_b32_e32 v1, v0
+; GCN: s_getpc_b64
+; GCN: s_add_u32 s{{[0-9]+}}, s{{[0-9]+}}, use_workitem_id_x@rel32@lo+4
+; GCN: s_addc_u32 s{{[0-9]+}}, s{{[0-9]+}}, use_workitem_id_x@rel32@hi+4
+; GCN: v_mov_b32_e32 v0, 9
+; GCN: s_swappc_b64
+; GCN: v_add_f32_e32
+; OPT-LABEL: @use_workitem_id_x(
+; OPT: %val = call i32 @use_workitem_id_x(i32 9)
+; OPT: bitcast i32 %val to float
+define amdgpu_kernel void @test_bitcast_use_workitem_id_x() #0 {
+  %val = call float bitcast (i32(i32)* @use_workitem_id_x to float(i32)*)(i32 9)
+  %op = fadd float %val, 1.0
+  store volatile float %op, float addrspace(1)* undef
+  ret void
+}
+
+; Callees appears last in source file to test that we still lower their
+; arguments before we lower any calls to them.
+
+define i32 @ret_i32_noinline() #0 {
+  ret i32 4
+}
+
+define i32 @ret_i32_alwaysinline() #1 {
+  ret i32 4
+}
+
+define i32 @ident_i32(i32 %i) #0 {
+  ret i32 %i
+}
+
+declare i32 @llvm.amdgcn.workitem.id.x() #2
+
+attributes #0 = { nounwind noinline }
+attributes #1 = { alwaysinline nounwind }
+attributes #2 = { nounwind readnone speculatable }

test/CodeGen/AMDGPU/promote-alloca-bitcast-function.ll

-; RUN: not llc -march=amdgcn < %s 2>&1 | FileCheck %s
-
-; FIXME: Error is misleading because it's not an indirect call.
-
-; CHECK: error: <unknown>:0:0: in function crash_call_constexpr_cast void (): unsupported indirect call to function foo
+; RUN: opt -data-layout=A5 -S -mtriple=amdgcn-unknown-unknown -amdgpu-promote-alloca -disable-promote-alloca-to-vector < %s | FileCheck %s
 
 ; Make sure that AMDGPUPromoteAlloca doesn't crash if the called
 ; function is a constantexpr cast of a function.
 
 declare void @foo(float addrspace(5)*) #0
 declare void @foo.varargs(...) #0
 
-; XCHECK: in function crash_call_constexpr_cast{{.*}}: unsupported call to function foo
+; CHECK-LABEL: @crash_call_constexpr_cast(
+; CHECK: alloca
+; CHECK: call void
 define amdgpu_kernel void @crash_call_constexpr_cast() #0 {
   %alloca = alloca i32, addrspace(5)
   call void bitcast (void (float addrspace(5)*)* @foo to void (i32 addrspace(5)*)*)(i32 addrspace(5)* %alloca) #0
   ret void
 }
 
-; XCHECK: in function crash_call_constexpr_cast{{.*}}: unsupported call to function foo.varargs
+; CHECK-LABEL: @crash_call_constexpr_cast_varargs(
+; CHECK: alloca
+; CHECK: call void
 define amdgpu_kernel void @crash_call_constexpr_cast_varargs() #0 {
   %alloca = alloca i32, addrspace(5)
   call void bitcast (void (...)* @foo.varargs to void (i32 addrspace(5)*)*)(i32 addrspace(5)* %alloca) #0
   ret void
 }
 
 attributes #0 = { nounwind }

test/CodeGen/AMDGPU/unsupported-calls.ll

 ; R600: in function test_call_varargs{{.*}}: unsupported call to function external.varargs
 define void @test_call_varargs() {
   call void (i32, double, i64, ...) @external.varargs(i32 42, double 1.0, i64 12, i8 3, i16 1, i32 4, float 1.0, double 2.0)
   ret void
 }
 
 declare i32 @extern_variadic(...)
 
-; GCN: in function test_tail_call_bitcast_extern_variadic{{.*}}: unsupported indirect call to function extern_variadic
+; GCN: in function test_tail_call_bitcast_extern_variadic{{.*}}: unsupported call to variadic function extern_variadic
 ; R600: in function test_tail_call_bitcast_extern_variadic{{.*}}: unsupported call to function extern_variadic
 define i32 @test_tail_call_bitcast_extern_variadic(<4 x float> %arg0, <4 x float> %arg1, i32 %arg2) {
   %add = fadd <4 x float> %arg0, %arg1
   %call = tail call i32 bitcast (i32 (...)* @extern_variadic to i32 (<4 x float>)*)(<4 x float> %add)
   ret i32 %call
 }
 
 ; GCN: :0:0: in function test_indirect_call void (void ()*): unsupported indirect call to function <unknown>