Skip to content

[SYCL] Add experimental flag to enable front-end optimizations #1376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 25, 2020
Merged

[SYCL] Add experimental flag to enable front-end optimizations #1376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc62a48
[SYCL] Add experimental flag to enable front-end optimizations
bader Mar 23, 2020
432718c
[SYCL] Update the test checking optimized LLVM IR
bader Mar 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/include/clang/Driver/CC1Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,8 @@ def fsycl_std_layout_kernel_params: Flag<["-"], "fsycl-std-layout-kernel-params"
def fsycl_allow_func_ptr : Flag<["-"], "fsycl-allow-func-ptr">,
HelpText<"Allow function pointers in SYCL device.">;
def fno_sycl_allow_func_ptr : Flag<["-"], "fno-sycl-allow-func-ptr">;
def fsycl_enable_optimizations: Flag<["-"], "fsycl-enable-optimizations">,
HelpText<"Experimental flag enabling standard optimization in the front-end.">;

} // let Flags = [CC1Option]

Expand Down
4 changes: 0 additions & 4 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4109,10 +4109,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
}
}

if (Triple.isSPIR()) {
CmdArgs.push_back("-disable-llvm-passes");
}

if (Args.hasFlag(options::OPT_fsycl_allow_func_ptr,
options::OPT_fno_sycl_allow_func_ptr, false)) {
CmdArgs.push_back("-fsycl-allow-func-ptr");
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,10 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
Args.getLastArg(OPT_emit_llvm_uselists, OPT_no_emit_llvm_uselists))
Opts.EmitLLVMUseLists = A->getOption().getID() == OPT_emit_llvm_uselists;

Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
Opts.DisableLLVMPasses =
Args.hasArg(OPT_disable_llvm_passes) ||
(Args.hasArg(OPT_fsycl_is_device) && Triple.isSPIR() &&
!Args.hasArg(OPT_fsycl_enable_optimizations));
Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);

const llvm::Triple::ArchType DebugEntryValueArchs[] = {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenSYCL/inline_asm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64-unknown-unknown-sycldevice -emit-llvm -x c++ %s -o - | FileCheck %s
// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsycl-enable-optimizations -triple spir64-unknown-unknown-sycldevice -emit-llvm -x c++ %s -o - | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this change. If you want to test your new option, add specific test for it. I don't see why you adding this option to unrelated test and check noting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's explained in the commit message.
"[SYCL] Update the test checking optimized LLVM IR"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see. No more questions about the change.
We added a test checking optimized IR. Does such test makes sense if we don't enable optimizations when compiling SYCL application?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+@premanandrao,

It still makes sense because it has a few checks matching asm, which are relevant as target independent optimizations do not optimize inline asm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I agree that it would be better to re-write the checks.


class kernel;

Expand Down