Skip to content

[clang][Driver][AVR] Reject c/c++ compilation for avr1 devices #111798

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.

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
Oct 14, 2024

Conversation

benshi001
Copy link
Member

@benshi001 benshi001 commented Oct 10, 2024

avr-gcc also rejects since these devices has no SRAM.

Fixes #96881

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2024

@llvm/pr-subscribers-clang

Author: Ben Shi (benshi001)

Changes

avr-gcc also rejects since these devices has no SRAM.


Full diff: https://github.com/llvm/llvm-project/pull/111798.diff

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/AVR.cpp (+8)
  • (modified) clang/test/Driver/avr-mmcu.c (+2-3)
diff --git a/clang/lib/Driver/ToolChains/AVR.cpp b/clang/lib/Driver/ToolChains/AVR.cpp
index bb5c0e6db9978e..83a756b07056ca 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -400,6 +400,14 @@ void AVRToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 void AVRToolChain::addClangTargetOptions(
     const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
     Action::OffloadKind DeviceOffloadKind) const {
+  // Reject C/C++ compilation for avr1 devices.
+  const Driver &D = getDriver();
+  std::string CPU = getCPUName(D, DriverArgs, getTriple());
+  std::optional<StringRef> FamilyName = GetMCUFamilyName(CPU);
+  if (CPU == "avr1" || (FamilyName && FamilyName->compare("avr1") == 0))
+    D.Diag(diag::err_drv_opt_unsupported_input_type) << "avr1"
+                                                     << "c/c++";
+
   // By default, use `.ctors` (not `.init_array`), as required by libgcc, which
   // runs constructors/destructors on AVR.
   if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
diff --git a/clang/test/Driver/avr-mmcu.c b/clang/test/Driver/avr-mmcu.c
index 6c13ab5b68d3b9..e354917ae9ab1d 100644
--- a/clang/test/Driver/avr-mmcu.c
+++ b/clang/test/Driver/avr-mmcu.c
@@ -1,8 +1,7 @@
 // A test for the propagation of the -mmcu option to -cc1 and -cc1as
 
-// RUN: %clang -### --target=avr -mmcu=attiny11 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
-// CHECK0: "-cc1" {{.*}} "-target-cpu" "attiny11"
-// CHECK0: "-cc1as" {{.*}} "-target-cpu" "attiny11"
+// RUN: not %clang -### --target=avr -mmcu=attiny11 %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
+// CHECK0: error: 'avr1' invalid for input of type c/c++
 
 // RUN: %clang -### --target=avr -mmcu=at90s2313 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK1 %s
 // CHECK1: "-cc1" {{.*}} "-target-cpu" "at90s2313"

@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2024

@llvm/pr-subscribers-clang-driver

Author: Ben Shi (benshi001)

Changes

avr-gcc also rejects since these devices has no SRAM.


Full diff: https://github.com/llvm/llvm-project/pull/111798.diff

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/AVR.cpp (+8)
  • (modified) clang/test/Driver/avr-mmcu.c (+2-3)
diff --git a/clang/lib/Driver/ToolChains/AVR.cpp b/clang/lib/Driver/ToolChains/AVR.cpp
index bb5c0e6db9978e..83a756b07056ca 100644
--- a/clang/lib/Driver/ToolChains/AVR.cpp
+++ b/clang/lib/Driver/ToolChains/AVR.cpp
@@ -400,6 +400,14 @@ void AVRToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 void AVRToolChain::addClangTargetOptions(
     const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
     Action::OffloadKind DeviceOffloadKind) const {
+  // Reject C/C++ compilation for avr1 devices.
+  const Driver &D = getDriver();
+  std::string CPU = getCPUName(D, DriverArgs, getTriple());
+  std::optional<StringRef> FamilyName = GetMCUFamilyName(CPU);
+  if (CPU == "avr1" || (FamilyName && FamilyName->compare("avr1") == 0))
+    D.Diag(diag::err_drv_opt_unsupported_input_type) << "avr1"
+                                                     << "c/c++";
+
   // By default, use `.ctors` (not `.init_array`), as required by libgcc, which
   // runs constructors/destructors on AVR.
   if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
diff --git a/clang/test/Driver/avr-mmcu.c b/clang/test/Driver/avr-mmcu.c
index 6c13ab5b68d3b9..e354917ae9ab1d 100644
--- a/clang/test/Driver/avr-mmcu.c
+++ b/clang/test/Driver/avr-mmcu.c
@@ -1,8 +1,7 @@
 // A test for the propagation of the -mmcu option to -cc1 and -cc1as
 
-// RUN: %clang -### --target=avr -mmcu=attiny11 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
-// CHECK0: "-cc1" {{.*}} "-target-cpu" "attiny11"
-// CHECK0: "-cc1as" {{.*}} "-target-cpu" "attiny11"
+// RUN: not %clang -### --target=avr -mmcu=attiny11 %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
+// CHECK0: error: 'avr1' invalid for input of type c/c++
 
 // RUN: %clang -### --target=avr -mmcu=at90s2313 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK1 %s
 // CHECK1: "-cc1" {{.*}} "-target-cpu" "at90s2313"

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

Thanks for getting to this I had forgotten I raised that issue.

You should move your commit's message into the PR description, as the PR description is what's used for the final commit when we merge this.

@DavidSpickett DavidSpickett changed the title [Driver][AVR] Reject c/c++ compilation for avr1 devices [clang][Driver][AVR] Reject c/c++ compilation for avr1 devices Oct 11, 2024
@DavidSpickett
Copy link
Collaborator

�_bk;t=1728546187607�Failed Tests (1):
�_bk;t=1728546187607�  Clang :: Driver/hip-partial-link.hip

And the CI test failure is this, the reporting is not great because we run check- targets one after another. This test was already failing on main.

If it fails again download the logs and search for "Failed<2-3 spaces>" and you'll see any results summaries where something failed. I plan to work on combining the results so this isn't a problem in the future.

@benshi001 benshi001 force-pushed the avr-driver branch 2 times, most recently from 2592828 to c50c440 Compare October 12, 2024 01:26
@benshi001
Copy link
Member Author

Thanks for getting to this I had forgotten I raised that issue.

You should move your commit's message into the PR description, as the PR description is what's used for the final commit when we merge this.

Thanks. I have update my PR description according to my commit message.

@benshi001
Copy link
Member Author

benshi001 commented Oct 12, 2024

�_bk;t=1728546187607�Failed Tests (1):
�_bk;t=1728546187607�  Clang :: Driver/hip-partial-link.hip

And the CI test failure is this, the reporting is not great because we run check- targets one after another. This test was already failing on main.

If it fails again download the logs and search for "Failed<2-3 spaces>" and you'll see any results summaries where something failed. I plan to work on combining the results so this isn't a problem in the future.

Thanks. I am also thinking the HIP failure is not related to my clang-driver-AVR work.

@DavidSpickett
Copy link
Collaborator

Please add a release note in https://clang.llvm.org/docs/ReleaseNotes.html#avr-support (clang/docs/ReleaseNotes.rst).

Otherwise, this looks good to me.

avr-gcc also rejects since these devices has no SRAM.

Fixes llvm#96881
@benshi001
Copy link
Member Author

Please add a release note in https://clang.llvm.org/docs/ReleaseNotes.html#avr-support (clang/docs/ReleaseNotes.rst).

Otherwise, this looks good to me.

The note is added. Thanks!

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

LGTM with the wording fixed.

@benshi001 benshi001 merged commit 4bf6e83 into llvm:main Oct 14, 2024
6 of 9 checks passed
@benshi001 benshi001 deleted the avr-driver branch October 14, 2024 11:09
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…111798)

avr-gcc also rejects since these devices has no SRAM.

Fixes llvm#96881
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiler crash when compiling for AVR with -mmcu=avr1
4 participants