CMABI : fix generation by-val copies

igorban-intel · igcbot · commit 3fbae083eccb · 2025-02-12T07:32:56.000+01:00
Do not copy byval-arguments for cases when
there are no stores by value.
diff --git a/IGC/VectorCompiler/lib/GenXOpts/CMTrans/CMABI.cpp b/IGC/VectorCompiler/lib/GenXOpts/CMTrans/CMABI.cpp
@@ -1,6 +1,6 @@
 /*========================== begin_copyright_notice ============================
 
-Copyright (C) 2017-2024 Intel Corporation
+Copyright (C) 2017-2025 Intel Corporation
 
 SPDX-License-Identifier: MIT
 
@@ -421,6 +421,43 @@ template <class NodeType> Function *getFunction(NodeType i) {
   return nullptr;
 }
 
+// Analyzes the uses of a ByVal argument to determine if it is written to.
+static inline bool isReadOnlyArg(Argument &Arg) {
+  SmallVector<Use *, 8> Uses;
+  for (auto &U : Arg.uses())
+    Uses.push_back(&U);
+
+  while (!Uses.empty()) {
+    auto *U = Uses.back();
+    Uses.pop_back();
+    if (auto *SI = dyn_cast<StoreInst>(U->getUser())) {
+      if (SI->getPointerOperand() == U->get())
+        return false;
+      continue; // sutable usage
+    }
+
+    if (auto *LI = dyn_cast<LoadInst>(U->getUser()))
+      continue; // sutable usage
+
+    if (auto *GEP = dyn_cast<GetElementPtrInst>(U->getUser())) {
+      if (GEP->getPointerOperand() == U->get())
+        for (auto &GU : GEP->uses())
+          Uses.push_back(&GU);
+
+      continue;
+    }
+
+    if (auto *Cast = dyn_cast<CastInst>(U->getUser())) {
+      for (auto &CU : Cast->uses())
+        Uses.push_back(&CU);
+      continue;
+    }
+
+    return false;
+  }
+  return true;
+}
+
 template <class CallGraphImpl>
 bool CMABIBase<CallGraphImpl>::runOnCallGraphImpl(CallGraphImpl &SCC) {
   bool Changed = false;
@@ -451,7 +488,6 @@ bool CMABIBase<CallGraphImpl>::runOnCallGraphImpl(CallGraphImpl &SCC) {
         if (auto *CGN = ProcessNode(&*I)) {
           LocalChange = true;
           auto *NewF = getFunction(CGN);
-          ;
           SCC.getOuterRefSCC().replaceNodeFunction(*I, *NewF);
         }
       }
@@ -466,7 +502,8 @@ bool CMABIBase<CallGraphImpl>::runOnCallGraphImpl(CallGraphImpl &SCC) {
       continue;
     for (auto &Arg : F->args()) {
       auto *ArgTy = Arg.getType();
-      if (!Arg.hasAttribute(Attribute::ByVal) || !ArgTy->isPointerTy())
+      if (!Arg.hasAttribute(Attribute::ByVal) || !ArgTy->isPointerTy() ||
+          isReadOnlyArg(Arg))
         continue;
       auto *M = F->getParent();
       auto *InsertBefore = F->getEntryBlock().getFirstNonPHI();
diff --git a/IGC/VectorCompiler/test/CMABI/byval_sret.ll b/IGC/VectorCompiler/test/CMABI/byval_sret.ll
@@ -78,6 +78,7 @@ define internal spir_func void @store_float_internal_byval(%struct4* byval(%stru
   %s = load %struct4, %struct4* %s_ptr
   %f = extractvalue %struct4 %s, 0
   store float %f, float* %f_ptr
+  store %struct4 %s, %struct4* %s_ptr
   ret void
 ; SMALL-OFF-NEXT: %[[COPY_STR:[^ ]+]] = alloca %struct4, align 8
 ; SMALL-OFF-TYPED-PTRS-NEXT: %[[MEMCPY_DST:[^ ]+]] = bitcast %struct4* %[[COPY_STR]] to i8*
@@ -98,10 +99,10 @@ define internal spir_func void @store_float_internal_byval(%struct4* byval(%stru
 ; SMALL-ON-OPAQUE-PTRS-NEXT: %s = load %struct4, ptr %s_ptr
 ; COMMON-NEXT: %f = extractvalue %struct4 %s, 0
 ; COMMON-TYPED-PTRS-NEXT: store float %f, float* %f_ptr
-; COMMON-TYPED-PTRS-NEXT: %[[FLOAT_VAL:[^ ]+]] = load float, float* %f_ptr
+; COMMON-TYPED-PTRS: %[[FLOAT_VAL:[^ ]+]] = load float, float* %f_ptr
 ; COMMON-OPAQUE-PTRS-NEXT: store float %f, ptr %f_ptr
-; COMMON-OPAQUE-PTRS-NEXT: %[[FLOAT_VAL:[^ ]+]] = load float, ptr %f_ptr
-; COMMON-NEXT: ret float %[[FLOAT_VAL]]
+; COMMON-OPAQUE-PTRS: %[[FLOAT_VAL:[^ ]+]] = load float, ptr %f_ptr
+; COMMON: ret float %[[FLOAT_VAL]]
 }
 
 ; COM: not internal function with byval attribute:
@@ -113,6 +114,7 @@ define spir_func void @store_float(%struct4* byval(%struct4) %s_ptr, float* %f_p
   %s = load %struct4, %struct4* %s_ptr
   %f = extractvalue %struct4 %s, 0
   store float %f, float* %f_ptr
+  store %struct4 %s, %struct4* %s_ptr
   ret void
 ; COMMON-NEXT: %[[COPY_STR:[^ ]+]] = alloca %struct4, align 8
 ; COMMON-TYPED-PTRS-NEXT: %[[MEMCPY_DST:[^ ]+]] = bitcast %struct4* %[[COPY_STR]] to i8*
@@ -126,7 +128,7 @@ define spir_func void @store_float(%struct4* byval(%struct4) %s_ptr, float* %f_p
 ; COMMON-NEXT: %f = extractvalue %struct4 %s, 0
 ; COMMON-TYPED-PTRS-NEXT: store float %f, float* %f_ptr
 ; COMMON-OPAQUE-PTRS-NEXT: store float %f, ptr %f_ptr
-; COMMON-NEXT: ret void
+; COMMON: ret void
 }
 
 ; COM: internal function with array access on ptr:
@@ -224,6 +226,8 @@ define spir_func void @store_sum(%struct24* byval(%struct24) %s_ptr) #0 {
   %fv_ptr = load <2 x float>*, <2 x float>** %ptr3
   %sum = fadd <2 x float> %fv1, %fv2
   store <2 x float> %sum, <2 x float>* %fv_ptr
+  %tmp = load %struct24, %struct24* %s_ptr
+  store %struct24 %tmp, %struct24* %s_ptr
   ret void
 ; COMMON-NEXT: %[[COPY_STR:[^ ]+]] = alloca %struct24, align 8
 ; COMMON-TYPED-PTRS-NEXT: %[[MEMCPY_DST:[^ ]+]] = bitcast %struct24* %[[COPY_STR]] to i8*
@@ -247,7 +251,7 @@ define spir_func void @store_sum(%struct24* byval(%struct24) %s_ptr) #0 {
 ; COMMON-NEXT: %sum = fadd <2 x float> %fv1, %fv2
 ; COMMON-TYPED-PTRS-NEXT: store <2 x float> %sum, <2 x float>* %fv_ptr
 ; COMMON-OPAQUE-PTRS-NEXT: store <2 x float> %sum, ptr %fv_ptr
-; COMMON-NEXT: ret void
+; COMMON: ret void
 }
 
 ; COM: struct4s tests structure with structure inside.
diff --git a/IGC/VectorCompiler/test/CMABI/read_only_arg.ll b/IGC/VectorCompiler/test/CMABI/read_only_arg.ll
@@ -0,0 +1,105 @@
+;=========================== begin_copyright_notice ============================
+;
+; Copyright (C) 2025 Intel Corporation
+;
+; SPDX-License-Identifier: MIT
+;
+;============================ end_copyright_notice =============================
+
+; RUN: %opt_typed_ptrs %use_old_pass_manager% -CMABILegacy -march=genx64 -mcpu=XeHPG -S < %s | FileCheck %s --check-prefixes=CHECK
+; RUN: %opt_opaque_ptrs %use_old_pass_manager% -CMABILegacy -march=genx64 -mcpu=XeHPG -S < %s | FileCheck %s --check-prefixes=CHECK
+
+; RUN: %opt_new_pm_typed -passes=CMABI -march=genx64 -mcpu=XeHPG -S < %s | FileCheck %s --check-prefixes=CHECK
+; RUN: %opt_new_pm_opaque -passes=CMABI -march=genx64 -mcpu=XeHPG -S < %s | FileCheck %s --check-prefixes=CHECK
+
+target datalayout = "e-p:64:64-i64:64-n8:16:32:64"
+target triple = "spir64-unknown-unknown"
+
+; check that the test did not abort
+%test.struct = type <{ i8 addrspace(4)*, i32, [4 x i8] }>
+%struct4 = type { float }
+
+; CHECK-LABEL: test0
+define internal spir_func <3 x i32> @test0(%test.struct addrspace(4)* byval(%test.struct) %0) {
+; CHECK-NOT: alloca
+  ret <3 x i32> zeroinitializer
+}
+
+; CHECK-LABEL: test1
+define spir_func void @test1(%struct4* byval(%struct4) %s_ptr, float* %f_ptr) #0 {
+; CHECK: alloca
+; Alloca -> store
+  %s = load %struct4, %struct4* %s_ptr
+  store %struct4 %s, %struct4* %s_ptr
+  ret void
+}
+
+; CHECK-LABEL: test2
+define spir_func void @test2(%struct4* byval(%struct4) %s_ptr, float* %f_ptr) #0 {
+; CHECK: alloca
+; Alloca -> cast -> store
+  %s = load %struct4, %struct4* %s_ptr
+  %cast1 = bitcast %struct4* %s_ptr to i8*
+  %cast2 = bitcast i8* %cast1 to i16*
+  %cast3 = bitcast i16* %cast2 to i32*
+  %cast4 = bitcast i32* %cast3 to %struct4*
+  store %struct4 %s, %struct4* %cast4
+  ret void
+}
+
+; CHECK-LABEL: test3
+define spir_func void @test3(%struct4* byval(%struct4) %s_ptr, float* %f_ptr) #0 {
+; CHECK: alloca
+; Alloca -> GEP -> store
+  %s = load %struct4, %struct4* %s_ptr
+  %load_ptr = getelementptr %struct4, %struct4* %s_ptr, i32 1, i32 0
+  %f = load float, float* %load_ptr
+  store float %f, float* %load_ptr
+  ret void
+}
+
+; CHECK-LABEL: test4
+define spir_func void @test4(%struct4* byval(%struct4) %s_ptr, float* %f_ptr) #0 {
+; CHECK-NOT: alloca
+; No Alloca -> GEP, cast, load
+  %load_ptr = getelementptr %struct4, %struct4* %s_ptr, i32 1, i32 0
+  %f = load float, float* %load_ptr
+  %cast1 = bitcast %struct4* %s_ptr to i8*
+  %cast2 = bitcast i8* %cast1 to i16*
+  %cast3 = bitcast i16* %cast2 to i32*
+  %cast4 = bitcast i32* %cast3 to %struct4*
+  %f2 = load %struct4, %struct4* %cast4
+  ret void
+}
+
+; CHECK-LABEL: kern
+define spir_kernel void @kern(float* %RET, float* %aFOO, i64 %privBase) #1 {
+  %str4_ptr = alloca %struct4, i32 2
+  %f_value = load float, float* %aFOO
+  %1 = call spir_func <3 x i32> @test0(%test.struct addrspace(4)* null)
+  call spir_func void @test1(%struct4* %str4_ptr, float* %RET)
+  call spir_func void @test2(%struct4* %str4_ptr, float* %RET)
+  call spir_func void @test3(%struct4* %str4_ptr, float* %RET)
+  call spir_func void @test4(%struct4* %str4_ptr, float* %RET)
+  ret void
+}
+
+attributes #0 = { noinline nounwind "CMStackCall" }
+attributes #1 = { noinline nounwind "CMGenxMain" "oclrt"="1" }
+
+!llvm.module.flags = !{!0}
+!opencl.ocl.version = !{!1}
+
+!0 = !{i32 1, !"genx.useGlobalMem", i32 1}
+!1 = !{i32 0, i32 0}
+!genx.kernels = !{!2}
+!genx.kernel.internal = !{!7}
+!2 = !{void (float*, float*, i64)* @kern, !"kern", !3, i32 0, !4, !5, !6, i32 0}
+!3 = !{i32 0, i32 0, i32 96}
+!4 = !{i32 72, i32 80, i32 64}
+!5 = !{i32 0, i32 0}
+!6 = !{!"", !""}
+!7 = !{void (float*, float*, i64)* @kern, !8, !9, !10, null}
+!8 = !{i32 0, i32 0, i32 0}
+!9 = !{i32 0, i32 1, i32 2}
+!10 = !{}
