Make LowerGPCallArg skip arguments with ByVal attribute

obrotowy · igcbot · commit f3d94194a97f · 2025-12-04T10:24:23.000+01:00
Lowering byval arguments had been causing bug, when origin addrspace was different
than private.
diff --git a/IGC/Compiler/Optimizer/OpenCLPasses/ErrorCheckPass/ErrorCheckPass.cpp b/IGC/Compiler/Optimizer/OpenCLPasses/ErrorCheckPass/ErrorCheckPass.cpp
@@ -42,10 +42,26 @@ bool ErrorCheck::runOnFunction(Function &F) {
   if (isEntryFunc(getAnalysis<MetaDataUtilsWrapper>().getMetaDataUtils(), &F)) {
     checkArgsSize(F);
   }
-
+  checkByValAddrSpace(F);
   return m_hasError;
 }
 
+void ErrorCheck::checkByValAddrSpace(Function &F) {
+  auto Ctx = getAnalysis<CodeGenContextWrapper>().getCodeGenContext();
+  for (auto &arg : F.args()) {
+    Type *argTy = arg.getType();
+    if (arg.hasByValAttr()) {
+      if (argTy->getPointerAddressSpace() != ADDRESS_SPACE_GENERIC &&
+          argTy->getPointerAddressSpace() != ADDRESS_SPACE_PRIVATE) {
+        std::string ErrorMsg = "ByVal argument with addrspace different than Generic/Private is not supported.";
+        Ctx->EmitError(ErrorMsg.c_str(), &F);
+
+        m_hasError = true;
+      }
+    }
+  }
+}
+
 void ErrorCheck::checkArgsSize(Function &F) {
   auto Ctx = getAnalysis<CodeGenContextWrapper>().getCodeGenContext();
   auto MdUtils = getAnalysis<MetaDataUtilsWrapper>().getMetaDataUtils();
diff --git a/IGC/Compiler/Optimizer/OpenCLPasses/ErrorCheckPass/ErrorCheckPass.h b/IGC/Compiler/Optimizer/OpenCLPasses/ErrorCheckPass/ErrorCheckPass.h
@@ -40,7 +40,7 @@ class ErrorCheck : public llvm::FunctionPass, public llvm::InstVisitor<ErrorChec
   bool m_hasError = false;
 
   void checkArgsSize(llvm::Function &F);
-
+  void checkByValAddrSpace(llvm::Function &F);
   void handleFP64EmulationMode(llvm::Instruction &I);
 };
 
diff --git a/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/LowerGPCallArg.cpp b/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/LowerGPCallArg.cpp
@@ -73,7 +73,10 @@ bool LowerGPCallArg::runOnModule(llvm::Module &M) {
 
       Type *argTy = arg.getType();
       if (argTy->isPointerTy() && argTy->getPointerAddressSpace() == ADDRESS_SPACE_GENERIC) {
-        if (auto originAddrSpace = getOriginAddressSpace(F, arg.getArgNo()))
+        auto originAddrSpace = getOriginAddressSpace(F, arg.getArgNo());
+        // We don't optimize ByVal arguments if its origin != private because it's been causing issues
+        // We're currently investigating if we should support byval arguments for addrspaces other than private/generic
+        if (originAddrSpace && !(arg.hasByValAttr() && originAddrSpace != ADDRESS_SPACE_PRIVATE))
           genericArgsInfo.push_back({arg.getArgNo(), originAddrSpace.value()});
       }
     }
diff --git a/IGC/Compiler/tests/ErrorCheck/unsupported-byval-argument-typed-pointers.ll b/IGC/Compiler/tests/ErrorCheck/unsupported-byval-argument-typed-pointers.ll
@@ -0,0 +1,23 @@
+;=========================== begin_copyright_notice ============================
+;
+; Copyright (C) 2025 Intel Corporation
+;
+; SPDX-License-Identifier: MIT
+;
+;============================ end_copyright_notice =============================
+
+; RUN: igc_opt --igc-error-check -S < %s 2>&1 | FileCheck %s
+; ------------------------------------------------
+; ErrorCheck
+; ------------------------------------------------
+
+; CHECK: error: ByVal argument with addrspace different than Generic/Private is not supported.
+
+define void @test(i32 addrspace(3)* byval (i32) %p1) {
+  ret void
+}
+
+!0 = !{void (i32 addrspace(3)*)* @test, !1}
+!1 = !{!2, !3}
+!2 = !{!"function_type", i32 0}
+!3 = !{!"implicit_arg_desc"}
diff --git a/IGC/Compiler/tests/ErrorCheck/unsupported-byval-argument.ll b/IGC/Compiler/tests/ErrorCheck/unsupported-byval-argument.ll
@@ -0,0 +1,23 @@
+;=========================== begin_copyright_notice ============================
+;
+; Copyright (C) 2025 Intel Corporation
+;
+; SPDX-License-Identifier: MIT
+;
+;============================ end_copyright_notice =============================
+
+; RUN: igc_opt --opaque-pointers --igc-error-check -S < %s 2>&1 | FileCheck %s
+; ------------------------------------------------
+; ErrorCheck
+; ------------------------------------------------
+
+; CHECK: error: ByVal argument with addrspace different than Generic/Private is not supported.
+
+define void @test(i32 addrspace(3)* byval (i32) %p1) {
+  ret void
+}
+
+!0 = !{void (i32 addrspace(3)*)* @test, !1}
+!1 = !{!2, !3}
+!2 = !{!"function_type", i32 0}
+!3 = !{!"implicit_arg_desc"}
diff --git a/IGC/Compiler/tests/LowerGPCallArg/unsupported-byval-addrspace-typed-pointers.ll b/IGC/Compiler/tests/LowerGPCallArg/unsupported-byval-addrspace-typed-pointers.ll
@@ -0,0 +1,33 @@
+;=========================== begin_copyright_notice ============================
+;
+; Copyright (C) 2025 Intel Corporation
+;
+; SPDX-License-Identifier: MIT
+;
+;============================ end_copyright_notice =============================
+
+; RUN: igc_opt --typed-pointers --igc-lower-gp-arg -S < %s 2>&1 | FileCheck %s
+
+@a = internal addrspace(3) global i32 0
+; CHECK: [[A:@.*]] = internal addrspace(3) global i32 0
+
+define i32 @func(i32 addrspace(4)* byval (i32) %p1) {
+  ; CHECK: define i32 [[FUNC:@.*]](i32 addrspace(4)* byval(i32) [[P1:%.*]]) {
+  %1 = load i32, i32 addrspace(4)* %p1
+  ret i32 %1
+}
+
+define void @kernel() {
+  ; CHECK: [[TMP1:%.*]] = addrspacecast i32 addrspace(3)* [[A]] to i32 addrspace(4)*
+  %1 = addrspacecast i32 addrspace(3)* @a to i32 addrspace(4)*
+  call i32 @func(i32 addrspace(4)* byval(i32) %1)
+  ret void
+}
+
+!igc.functions = !{!0, !3}
+
+!0 = !{void ()* @kernel, !1}
+!3 = !{i32 (i32 addrspace(4)*)* @func, !1}
+
+!1 = !{!2}
+!2 = !{!"function_type", i32 0}
diff --git a/IGC/Compiler/tests/LowerGPCallArg/unsupported-byval-addrspace.ll b/IGC/Compiler/tests/LowerGPCallArg/unsupported-byval-addrspace.ll
@@ -0,0 +1,33 @@
+;=========================== begin_copyright_notice ============================
+;
+; Copyright (C) 2025 Intel Corporation
+;
+; SPDX-License-Identifier: MIT
+;
+;============================ end_copyright_notice =============================
+
+; RUN: igc_opt --opaque-pointers --igc-lower-gp-arg -S < %s 2>&1 | FileCheck %s
+
+@a = internal addrspace(3) global i32 0
+; CHECK: [[A:@.*]] = internal addrspace(3) global i32 0
+
+define i32 @func(i32 addrspace(4)* byval (i32) %p1) {
+  ; CHECK: define i32 [[FUNC:@.*]](ptr addrspace(4) byval(i32) [[P1:%.*]]) {
+  %1 = load i32, i32 addrspace(4)* %p1
+  ret i32 %1
+}
+
+define void @kernel() {
+  ; CHECK: [[TMP1:%.*]] = addrspacecast ptr addrspace(3) [[A]] to ptr addrspace(4)
+  %1 = addrspacecast i32 addrspace(3)* @a to i32 addrspace(4)*
+  call i32 @func(i32 addrspace(4)* byval(i32) %1)
+  ret void
+}
+
+!igc.functions = !{!0, !3}
+
+!0 = !{void ()* @kernel, !1}
+!3 = !{i32 (i32 addrspace(4)*)* @func, !1}
+
+!1 = !{!2}
+!2 = !{!"function_type", i32 0}

Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,10 @@ bool LowerGPCallArg::runOnModule(llvm::Module &M) {`
`73`	`73`
`74`	`74`	`Type *argTy = arg.getType();`
`75`	`75`	`if (argTy->isPointerTy() && argTy->getPointerAddressSpace() == ADDRESS_SPACE_GENERIC) {`
`76`		`- if (auto originAddrSpace = getOriginAddressSpace(F, arg.getArgNo()))`
	`76`	`+ auto originAddrSpace = getOriginAddressSpace(F, arg.getArgNo());`
	`77`	`+ // We don't optimize ByVal arguments if its origin != private because it's been causing issues`
	`78`	`+ // We're currently investigating if we should support byval arguments for addrspaces other than private/generic`
	`79`	`+ if (originAddrSpace && !(arg.hasByValAttr() && originAddrSpace != ADDRESS_SPACE_PRIVATE))`
`77`	`80`	`genericArgsInfo.push_back({arg.getArgNo(), originAddrSpace.value()});`
`78`	`81`	`}`
`79`	`82`	`}`