[SYCL][ESIMD] Rework -O0 behavior

clang/lib/CodeGen/BackendUtil.cpp

@@ -962,6 +962,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
       PB.registerPipelineStartEPCallback([&](ModulePassManager &MPM,
                                              OptimizationLevel Level) {
         MPM.addPass(ESIMDVerifierPass(LangOpts.SYCLESIMDForceStatelessMem));
+        if (Level == OptimizationLevel::O0)
+          MPM.addPass(ESIMDRemoveOptnoneNoinlinePass());
         MPM.addPass(SYCLPropagateAspectsUsagePass(/*ExcludeAspects=*/{"fp64"}));
         MPM.addPass(SYCLPropagateJointMatrixUsagePass());
       });

clang/lib/Driver/ToolChains/Clang.cpp

@@ -10316,8 +10316,11 @@ static void addArgs(ArgStringList &DstArgs, const llvm::opt::ArgList &Alloc,
 // Partially copied from clang/lib/Frontend/CompilerInvocation.cpp
 static std::string getSYCLPostLinkOptimizationLevel(const ArgList &Args) {
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+    // Pass -O2 when the user passes -O0 due to IGC
+    // debugging limitation. Note this only effects
+    // ESIMD code.
     if (A->getOption().matches(options::OPT_O0))
-      return "-O0";
+      return "-O2";
 
     if (A->getOption().matches(options::OPT_Ofast))
       return "-O3";

clang/test/Driver/sycl-offload-intelfpga-emu.cpp

@@ -251,6 +251,6 @@
 // RUN:   %clang_cl -### -fintelfpga -Zi -Od -Xs "-DFOO1 -DFOO2" %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-TOOLS-IMPLIED-OPTS %s
 // CHK-TOOLS-IMPLIED-OPTS: clang{{.*}} "-fsycl-is-device"{{.*}} "-O0"
-// CHK-TOOLS-IMPLIED-OPTS: sycl-post-link{{.*}} "-O0"
+// CHK-TOOLS-IMPLIED-OPTS: sycl-post-link{{.*}} "-O2"
 // CHK-TOOLS-IMPLIED-OPTS: opencl-aot{{.*}} "--bo=-g -cl-opt-disable" "-DFOO1" "-DFOO2"
 

clang/test/Driver/sycl-offload.c

@@ -632,7 +632,7 @@
 // only the last option is considered
 // RUN:   %clang    -### -fsycl -O2 -O1 %s 2>&1 | FileCheck %s -check-prefixes=CHK-POST-LINK-OPT-LEVEL-O1
 // RUN:   %clang_cl -### -fsycl /O2 /O1 %s 2>&1 | FileCheck %s -check-prefixes=CHK-POST-LINK-OPT-LEVEL-Os
-// CHK-POST-LINK-OPT-LEVEL-O0: sycl-post-link{{.*}} "-O0"
+// CHK-POST-LINK-OPT-LEVEL-O0: sycl-post-link{{.*}} "-O2"
 // CHK-POST-LINK-OPT-LEVEL-O1: sycl-post-link{{.*}} "-O1"
 // CHK-POST-LINK-OPT-LEVEL-O2: sycl-post-link{{.*}} "-O2"
 // CHK-POST-LINK-OPT-LEVEL-O3: sycl-post-link{{.*}} "-O3"

llvm/include/llvm/SYCLLowerIR/ESIMD/LowerESIMD.h

@@ -34,11 +34,14 @@ class PassRegistry;
 /// like intrinsics to a form parsable by the ESIMD-aware SPIRV translator.
 class SYCLLowerESIMDPass : public PassInfoMixin<SYCLLowerESIMDPass> {
 public:
+  SYCLLowerESIMDPass(bool ModuleContainsScalar = true)
+      : ModuleContainsScalarCode(ModuleContainsScalar) {}
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &);
 
 private:
   bool prepareForAlwaysInliner(Module &M);
   size_t runOnFunction(Function &F, SmallPtrSetImpl<Type *> &);
+  bool ModuleContainsScalarCode;
 };
 
 ModulePass *createSYCLLowerESIMDPass();
@@ -86,6 +89,12 @@ class ESIMDRemoveHostCodePass : public PassInfoMixin<ESIMDRemoveHostCodePass> {
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &);
 };
 
+class ESIMDRemoveOptnoneNoinlinePass
+    : public PassInfoMixin<ESIMDRemoveOptnoneNoinlinePass> {
+public:
+  PreservedAnalyses run(Module &M, ModuleAnalysisManager &);
+};
+
 } // namespace llvm
 
 #endif // LLVM_SYCLLOWERIR_LOWERESIMD_H

llvm/lib/Passes/PassRegistry.def

@@ -151,6 +151,7 @@ MODULE_PASS("deadargelim-sycl", DeadArgumentEliminationSYCLPass())
 MODULE_PASS("sycllowerwglocalmemory", SYCLLowerWGLocalMemoryPass())
 MODULE_PASS("lower-esimd-kernel-attrs", SYCLFixupESIMDKernelWrapperMDPass())
 MODULE_PASS("esimd-remove-host-code", ESIMDRemoveHostCodePass());
+MODULE_PASS("esimd-remove-optnone-noinline", ESIMDRemoveOptnoneNoinlinePass());
 MODULE_PASS("sycl-propagate-aspects-usage", SYCLPropagateAspectsUsagePass())
 MODULE_PASS("sycl-propagate-joint-matrix-usage", SYCLPropagateJointMatrixUsagePass())
 MODULE_PASS("sycl-add-opt-level-attribute", SYCLAddOptLevelAttributePass())

llvm/lib/SYCLLowerIR/CMakeLists.txt

@@ -51,6 +51,7 @@ add_llvm_component_library(LLVMSYCLLowerIR
   ESIMD/ESIMDUtils.cpp
   ESIMD/ESIMDVerifier.cpp
   ESIMD/ESIMDRemoveHostCode.cpp
+  ESIMD/ESIMDRemoveOptnoneNoinline.cpp
   ESIMD/LowerESIMD.cpp
   ESIMD/LowerESIMDKernelAttrs.cpp
   CleanupSYCLMetadata.cpp

llvm/lib/SYCLLowerIR/ESIMD/ESIMDRemoveOptnoneNoinline.cpp

@@ -0,0 +1,43 @@
+//=-- ESIMDRemoveOptnoneNoinline.cpp - remove optnone/noinline for ESIMD --=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//=------------------------------------------------------------------------=//
+// The GPU backend for ESIMD does not support debugging and we often see crashes
+// or wrong answers if we do not optimize. Remove optnone and noinline from
+// ESIMD functions so users at least can run their programs and get the
+// right answer.
+//=------------------------------------------------------------------------=//
+
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Pass.h"
+#include "llvm/SYCLLowerIR/ESIMD/ESIMDUtils.h"
+#include "llvm/SYCLLowerIR/ESIMD/LowerESIMD.h"
+#include "llvm/Support/Debug.h"
+
+using namespace llvm;
+using namespace llvm::esimd;
+
+cl::opt<bool> AllowOptnoneNoinline(
+    "esimd-allow-optnone-noinline", llvm::cl::Optional, llvm::cl::Hidden,
+    llvm::cl::desc("Allow optnone and noinline."), llvm::cl::init(false));
+
+PreservedAnalyses ESIMDRemoveOptnoneNoinlinePass::run(Module &M,
+                                                      ModuleAnalysisManager &) {
+  if (AllowOptnoneNoinline)
+    return PreservedAnalyses::all();
+
+  // TODO: Remove this pass once VC supports debugging.
+  bool Modified = false;
+  for (auto &F : M.functions()) {
+    if (!isESIMD(F) || F.hasFnAttribute("CMGenxSIMT"))
+      continue;
+    F.removeFnAttr(Attribute::OptimizeNone);
+    F.removeFnAttr(Attribute::NoInline);
+    Modified = true;
+  }
+  return Modified ? PreservedAnalyses::none() : PreservedAnalyses::all();
+}

llvm/lib/SYCLLowerIR/ESIMD/LowerESIMD.cpp

@@ -1834,6 +1834,15 @@ bool SYCLLowerESIMDPass::prepareForAlwaysInliner(Module &M) {
       continue;
     }
 
+    // If we are splitting by ESIMD, we are guarenteed the entire
+    // module only contains ESIMD code, so remove optnone/noinline
+    // from ALL functions as VC does not support these attributes
+    // and programs produce wrong answers or crash if they are kept.
+    if (!ModuleContainsScalarCode && !F.hasFnAttribute("CMGenxSIMT")) {
+      F.removeFnAttr(Attribute::NoInline);
+      F.removeFnAttr(Attribute::OptimizeNone);
+    }
+
     // TODO: The next code and comment was placed to ESIMDLoweringPass
     // 2 years ago, when GPU VC BE did not support function calls and
     // required everything to be inlined right into the kernel unless
@@ -1939,17 +1948,6 @@ size_t SYCLLowerESIMDPass::runOnFunction(Function &F,
   SmallVector<CallInst *, 32> ESIMDIntrCalls;
   SmallVector<Instruction *, 8> ToErase;
 
-  // The VC backend doesn't support debugging, and trying to use
-  // non-optimized code often produces crashes or wrong answers.
-  // The recommendation from the VC team was always optimize code,
-  // even if the user requested no optimization. We already drop
-  // debugging flags in the SYCL runtime, so also drop optnone and
-  // noinline here.
-  if (isESIMD(F) && F.hasFnAttribute(Attribute::OptimizeNone)) {
-    F.removeFnAttr(Attribute::OptimizeNone);
-    F.removeFnAttr(Attribute::NoInline);
-  }
-
   for (Instruction &I : instructions(F)) {
     if (auto CastOp = dyn_cast<llvm::CastInst>(&I)) {
       llvm::Type *DstTy = CastOp->getDestTy();

llvm/test/SYCLLowerIR/ESIMD/optnone.ll

@@ -1,5 +1,5 @@
-; This ensures we remove optnone from ESIMD functions
-; RUN: opt < %s -passes=LowerESIMD -S | FileCheck %s
+; This ensures we remove optnone from ESIMD functions unless they are SIMT.
+; RUN: opt < %s -passes=esimd-remove-optnone-noinline -S | FileCheck %s
 
 target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
 target triple = "spir64-unknown-unknown"
@@ -9,12 +9,21 @@ define dso_local spir_func void @esimd() #0 !sycl_explicit_simd !0 {
   ret void
 }
 
+define dso_local spir_func void @esimd_simt() #1 !sycl_explicit_simd !0 {
+; CHECK: void @esimd_simt() #[[#ESIMDSIMTAttr:]]
+  ret void
+}
+
 define dso_local spir_func void @sycl() #0 {
 ; CHECK: spir_func void @sycl() #[[#SYCLAttr:]]
   ret void
 }
 ; CHECK: attributes #[[#ESIMDAttr]] =
 ; CHECK-NOT: optnone
+; CHECK: attributes #[[#ESIMDSIMTAttr]] = {{{.*}}optnone
+; CHECK-SAME: "CMGenxSIMT"
+; CHECK-SAME: }
 ; CHECK: attributes #[[#SYCLAttr]] = {{{.*}}optnone{{.*}}}
 attributes #0 = { noinline optnone convergent }
+attributes #1 = { noinline optnone convergent "CMGenxSIMT"}
 !0 = !{}

llvm/test/tools/sycl-post-link/sycl-esimd/optnone.ll

@@ -0,0 +1,32 @@
+; This ensures we remove optnone from ESIMD functions unless they are SIMT or we didn't split ESIMD code out.
+; RUN: sycl-post-link -split-esimd -lower-esimd -S < %s -o %t.table
+; RUN: FileCheck %s -input-file=%t_esimd_0.ll --check-prefixes CHECK,CHECK-ESIMD-SPLIT
+
+; RUN: sycl-post-link -lower-esimd -S < %s -o %t1.table
+; RUN: FileCheck %s -input-file=%t1_esimd_0.ll --check-prefixes CHECK,CHECK-NO-ESIMD-SPLIT
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
+target triple = "spir64-unknown-unknown"
+
+define dso_local spir_func void @esimd() #0 !sycl_explicit_simd !0 {
+; CHECK: void @esimd() #[[#ESIMDAttr:]]
+  ret void
+}
+
+define dso_local spir_func void @esimd_simt() #1 !sycl_explicit_simd !0 {
+; CHECK: void @esimd_simt() #[[#ESIMDSIMTAttr:]]
+  ret void
+}
+
+define dso_local spir_func void @sycl() #0 {
+; CHECK: spir_func void @sycl() #[[#ESIMDAttr]]
+  ret void
+}
+; CHECK-ESIMD-SPLIT: attributes #[[#ESIMDAttr]] =
+; CHECK-ESIMD-SPLIT-NOT: optnone
+; CHECK-NO-ESIMD-SPLIT: attributes #[[#ESIMDAttr]] = {{{.*}}optnone{{.*}}}
+; CHECK: attributes #[[#ESIMDSIMTAttr]] = {{{.*}}optnone
+; CHECK-SAME: "CMGenxSIMT"
+; CHECK-SAME: }
+attributes #0 = { noinline optnone convergent }
+attributes #1 = { noinline optnone convergent "CMGenxSIMT"}
+!0 = !{}

llvm/tools/sycl-post-link/sycl-post-link.cpp

@@ -624,7 +624,7 @@ bool lowerEsimdConstructs(module_split::ModuleDesc &MD) {
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
   ModulePassManager MPM;
-  MPM.addPass(SYCLLowerESIMDPass{});
+  MPM.addPass(SYCLLowerESIMDPass(!SplitEsimd));
 
   if (!OptLevelO0) {
     FunctionPassManager FPM;

sycl/test-e2e/ESIMD/slm_alloc_many_kernels_many_funcs.cpp

@@ -25,16 +25,6 @@
 //   Y       Z
 //
 // SLM offsets are expected to be:
-// 1) no inlining case:
-// --- Kernel0
-// X  - N1
-// Y  - N1 + X
-// Z  - max(N1 + X, N2)
-// --- Kernel2
-// X  - 0 (not reachable, offset not updated in the result)
-// Y  - 0 (not reachable, offset not updated in the result)
-// Z  - max(N1 + X, N2)
-// 2) forced inlining case:
 // --- Kernel0
 // X  - N1
 // Y  - N1 + X
@@ -45,9 +35,6 @@
 // X  - 0 (not reachable, offset not updated in the result)
 // Y  - 0 (not reachable, offset not updated in the result)
 // Z  - N2
-// Note the difference in SLM offset for Z in the inlining/no-inlining cases for
-// Z scope. This is because inlining effectively splits Z scope when inlining
-// into kernel0 and kernel2
 
 #include <sycl/ext/intel/esimd.hpp>
 #include <sycl/sycl.hpp>
@@ -157,26 +144,14 @@ int main(void) {
   // Z  - max(N1 + X, N2)
   gold_arr0[x_off_ind] = SLM_N1;
   gold_arr0[y_off_ind] = SLM_N1 + SLM_X;
-
-  // For kernel0 inline/no-inline case splits for Z:
-#ifdef FORCE_INLINE
   gold_arr0[z_off_ind] = SLM_N1;
-#else
-  gold_arr0[z_off_ind] = std::max(SLM_N1 + SLM_X, SLM_N2);
-#endif // FORCE_INLINE
 
   // For kernel1 inline/no-inline results are the same for X and Y:
   // X  - 0
   // Y  - 0
   gold_arr1[x_off_ind] = 0;
   gold_arr1[y_off_ind] = 0;
-
-  // For kernel1 inline/no-inline case splits for Z:
-#ifdef FORCE_INLINE
   gold_arr1[z_off_ind] = SLM_N2;
-#else
-  gold_arr1[z_off_ind] = std::max(SLM_N1 + SLM_X, SLM_N2);
-#endif // FORCE_INLINE
 
   T *test_arr = arr;
   int err_cnt = 0;

sycl/test/esimd/genx_func_attr.cpp

@@ -1,5 +1,5 @@
 // RUN: %clangxx -O2 -fsycl -fsycl-device-only -Xclang -emit-llvm %s -o %t
-// RUN: sycl-post-link -split-esimd -lower-esimd -O2 -S %t -o %t.table
+// RUN: sycl-post-link -lower-esimd -O2 -S %t -o %t.table
 // RUN: FileCheck %s -input-file=%t_esimd_0.ll
 
 // Checks ESIMD intrinsic translation.

sycl/test/esimd/intrins_trans.cpp

@@ -1,13 +1,14 @@
-// RUN: %clangxx -O0 -fsycl -fsycl-device-only -fno-sycl-esimd-force-stateless-mem -Xclang -emit-llvm %s -o %t
+// RUN: %clangxx %clang_O0 -fsycl -fsycl-device-only -fno-sycl-esimd-force-stateless-mem -Xclang -emit-llvm %s -o %t
 // RUN: sycl-post-link -split-esimd -lower-esimd -lower-esimd-force-stateless-mem=false -O0 -S %t -o %t.table
 // RUN: FileCheck %s -input-file=%t_esimd_0.ll --check-prefixes=CHECK,CHECK-STATEFUL
 
-// RUN: %clangxx -O0 -fsycl -fsycl-device-only -fsycl-esimd-force-stateless-mem -Xclang -emit-llvm %s -o %t
+// RUN: %clangxx %clang_O0 -fsycl -fsycl-device-only -fsycl-esimd-force-stateless-mem -Xclang -emit-llvm %s -o %t
 // RUN: sycl-post-link -split-esimd -lower-esimd -lower-esimd-force-stateless-mem=true -O0 -S %t -o %t.table
 // RUN: FileCheck %s -input-file=%t_esimd_0.ll --check-prefixes=CHECK,CHECK-STATELESS
 
 // Checks ESIMD intrinsic translation with opaque pointers.
-// NOTE: must be run in -O0, as optimizer optimizes away some of the code
+// NOTE: must be run with %clang_O0, as optimizer optimizes away some of the
+// code
 
 #include <sycl/detail/image_ocl_types.hpp>
 #include <sycl/ext/intel/esimd.hpp>

sycl/test/esimd/lit.local.cfg

@@ -0,0 +1,3 @@
+import platform
+
+config.substitutions.append(("%clang_O0", "-O0 -mllvm -esimd-allow-optnone-noinline"))