[SYCL][Driver][FPGA] Remove support for FPGA related options #16864

mdtoguchi · 2025-01-31T18:35:20Z

Removes support for any FPGA related options and any options that use FPGA specific arguments. Upon usage of any of these options, a specific error will be emitted:

clang++ -fintelfpga file.cpp
clang++: error: option '-fintelfpga' is not supported and has been removed from the compiler. Please see the compiler documentation for more details

The following options are being handled:

-fintelfpga
-fsycl-targets=spir64_fpga[-unknown-unknown]
-fsycl-link=early/image
-Xsycl-target-backend=spir64_fpga "opt"
-reuse-exe=arg
-fsycl-help=fpga

This mainly removes support of the options as specified, subsequent cleanup of driver behaviors will be done in a later change.

Due to the broad impact of these changes, all associated LIT tests have been updated/removed.

…ions Removes support for any FPGA related options and any options that use FPGA specific arguments. First round to double check any fallout in testing

Remove FPGA references from the user manual and update or remove tests that are using FPGA specific behaviors.

againull

llvm-reviewers-runtime owned files LGTM

keryell · 2025-02-06T03:13:27Z

Why are you removing these options???

premanandrao

FE changes LGTM

srividya-sundaram · 2025-02-07T02:06:31Z

clang/include/clang/Driver/Driver.h

@@ -617,7 +617,8 @@ class Driver {

  /// getSYCLDeviceTriple - Returns the SYCL device triple for the
  /// specified subarch


The comment could be updated to mention the additional argument llvm::opt::Arg *Arg = nullptr added.

srividya-sundaram · 2025-02-07T02:08:29Z

clang/include/clang/Driver/Options.td

-def reuse_exe_EQ : Joined<["-"], "reuse-exe=">, Visibility<[ClangOption, CLOption, DXCOption]>,
-  HelpText<"Speed up FPGA aoc compile if the device code in <exe> is unchanged.">,
+def reuse_exe_EQ : Joined<["-"], "reuse-exe=">,
+  Visibility<[ClangOption, CLOption, DXCOption]>, Flags<[UnsupportedRemoved]>,


If this option is visible in DXC mode, are those tests removed as well?

AFAIK, there are no DXC specific tests for -reuse-exe.

srividya-sundaram · 2025-02-07T02:13:23Z

clang/lib/Driver/Driver.cpp

  Arg *SYCLLink = getArgRequiringSYCLRuntime(options::OPT_fsycl_link_EQ);
-  checkSingleArgValidity(SYCLLink, {"early", "image"});
+  checkSingleArgValidity(SYCLLink, {"early", "image", "default"});


If use of -fsycl-link=early and -fsycl-link=image are not supported, do we still need to pass "early" and "image" as AllowedValues to checkSingleArgValidity ?

No - this isn't necessary. The purpose of this change was to introduce the functionality to reject the options. Cleanup will occur later.

srividya-sundaram · 2025-02-07T02:15:22Z

clang/lib/Driver/Driver.cpp

@@ -7845,7 +7881,7 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
    if (!UseNewOffloadingDriver)
      OffloadBuilder->makeHostLinkAction(LinkerInputs);
    types::ID LinkType(types::TY_Image);
-    if (Args.hasArg(options::OPT_fsycl_link_EQ))
+    if (Args.hasArgNoClaim(options::OPT_fsycl_link_EQ))


Why has these been updated to hasArgNoClaim?

This change was for a different solution I was working for the -fsycl-link cleanup. This isn't necessary anymore - I will update.

srividya-sundaram · 2025-02-07T02:27:30Z

clang/include/clang/Basic/DiagnosticDriverKinds.td

@@ -22,6 +22,9 @@ def err_drv_unsupported_option_argument : Error<
  "unsupported argument '%1' to option '%0'">;
 def err_drv_unsupported_option_argument_for_target : Error<
  "unsupported argument '%1' to option '%0' for target '%2'">;
+def err_drv_unsupported_opt_removed : Error<


Is there a test case added for this diagnostic?

Yes - here:

llvm/clang/test/Driver/sycl-unsupported.cpp

Line 59 in 8bfe08d

// FPGA support has been removed, usage of any FPGA specific options and any

- revert usage of hasArgNoClaim - add comment for new parameter usage

gmlueck · 2025-02-07T21:40:46Z

Why are you removing these options???

Hi @keryell, see #16929 for an explanation.

mdtoguchi added 3 commits January 31, 2025 10:32

In Progress- [SYCL][Driver][FPGA] Remove support for FPGA related opt…

7489d8c

…ions Removes support for any FPGA related options and any options that use FPGA specific arguments. First round to double check any fallout in testing

Merge remote-tracking branch 'intel_llvm/sycl' into fpga-remove-opts

fc42795

additional test updates

1bb16a6

mdtoguchi had a problem deploying to WindowsCILock January 31, 2025 22:21 — with GitHub Actions Error

clang format

626def9

mdtoguchi had a problem deploying to WindowsCILock January 31, 2025 22:44 — with GitHub Actions Failure

mdtoguchi temporarily deployed to WindowsCILock January 31, 2025 23:22 — with GitHub Actions Inactive

mdtoguchi added 2 commits February 3, 2025 10:29

Update and remove FPGA specific testing and doc update

c915700

Remove FPGA references from the user manual and update or remove tests that are using FPGA specific behaviors.

Merge remote-tracking branch 'intel_llvm/sycl' into fpga-remove-opts

695c700

mdtoguchi temporarily deployed to WindowsCILock February 3, 2025 18:41 — with GitHub Actions Inactive

mdtoguchi had a problem deploying to WindowsCILock February 3, 2025 19:50 — with GitHub Actions Failure

Additional cleanup

7409e74

mdtoguchi temporarily deployed to WindowsCILock February 3, 2025 22:51 — with GitHub Actions Inactive

mdtoguchi temporarily deployed to WindowsCILock February 3, 2025 23:37 — with GitHub Actions Inactive

Missed update for -fsycl-link behavior

33aefd5

mdtoguchi temporarily deployed to WindowsCILock February 4, 2025 15:20 — with GitHub Actions Inactive

mdtoguchi temporarily deployed to WindowsCILock February 4, 2025 16:32 — with GitHub Actions Inactive

tests and fsycl-link update

8d39c2c

mdtoguchi temporarily deployed to WindowsCILock February 5, 2025 18:21 — with GitHub Actions Inactive

mdtoguchi had a problem deploying to WindowsCILock February 5, 2025 19:00 — with GitHub Actions Error

clang format

8bfe08d

mdtoguchi temporarily deployed to WindowsCILock February 5, 2025 19:32 — with GitHub Actions Inactive

mdtoguchi temporarily deployed to WindowsCILock February 5, 2025 20:27 — with GitHub Actions Inactive

mdtoguchi changed the title ~~In Progress- [SYCL][Driver][FPGA] Remove support for FPGA related opt…~~ [SYCL][Driver][FPGA] Remove support for FPGA related options Feb 5, 2025

mdtoguchi marked this pull request as ready for review February 5, 2025 22:07

mdtoguchi requested review from a team as code owners February 5, 2025 22:07

mdtoguchi requested a review from againull February 5, 2025 22:07

againull approved these changes Feb 5, 2025

View reviewed changes

premanandrao approved these changes Feb 6, 2025

View reviewed changes

srividya-sundaram reviewed Feb 7, 2025

View reviewed changes

Address review comments

d085ff7

- revert usage of hasArgNoClaim - add comment for new parameter usage

mdtoguchi temporarily deployed to WindowsCILock February 7, 2025 18:31 — with GitHub Actions Inactive

mdtoguchi temporarily deployed to WindowsCILock February 7, 2025 19:49 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL][Driver][FPGA] Remove support for FPGA related options #16864

[SYCL][Driver][FPGA] Remove support for FPGA related options #16864

mdtoguchi commented Jan 31, 2025 •

edited

Loading

againull left a comment •

edited

Loading

keryell commented Feb 6, 2025

premanandrao left a comment

srividya-sundaram Feb 7, 2025

srividya-sundaram Feb 7, 2025

mdtoguchi Feb 7, 2025

srividya-sundaram Feb 7, 2025

mdtoguchi Feb 7, 2025

srividya-sundaram Feb 7, 2025

mdtoguchi Feb 7, 2025

srividya-sundaram Feb 7, 2025

mdtoguchi Feb 7, 2025

gmlueck commented Feb 7, 2025

		@@ -617,7 +617,8 @@ class Driver {

		/// getSYCLDeviceTriple - Returns the SYCL device triple for the
		/// specified subarch

[SYCL][Driver][FPGA] Remove support for FPGA related options #16864

Are you sure you want to change the base?

[SYCL][Driver][FPGA] Remove support for FPGA related options #16864

Conversation

mdtoguchi commented Jan 31, 2025 • edited Loading

againull left a comment • edited Loading

Choose a reason for hiding this comment

keryell commented Feb 6, 2025

premanandrao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmlueck commented Feb 7, 2025

mdtoguchi commented Jan 31, 2025 •

edited

Loading

againull left a comment •

edited

Loading