Skip to content
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

[MSP430] Default to unsigned char #115964

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

arichardson
Copy link
Member

This matches the ABI document at https://www.ti.com/lit/pdf/slaa534 as
well as the GCC implementation.

Partially fixes #115957.

Created using spr 1.3.6-beta.1
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Nov 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 13, 2024

@llvm/pr-subscribers-backend-msp430
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Alexander Richardson (arichardson)

Changes

This matches the ABI document at https://www.ti.com/lit/pdf/slaa534 as
well as the GCC implementation.

Partially fixes #115957.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+1)
  • (added) clang/test/Driver/msp430-char.c (+5)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 0952262c360185..75379e0a00b981 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1341,6 +1341,7 @@ static bool isSignedCharDefault(const llvm::Triple &Triple) {
     return false;
 
   case llvm::Triple::hexagon:
+  case llvm::Triple::msp430:
   case llvm::Triple::ppcle:
   case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
diff --git a/clang/test/Driver/msp430-char.c b/clang/test/Driver/msp430-char.c
new file mode 100644
index 00000000000000..4f62eb167e1c84
--- /dev/null
+++ b/clang/test/Driver/msp430-char.c
@@ -0,0 +1,5 @@
+/// Check that char is unsigned by default.
+// RUN: %clang -### %s --target=msp430 -c 2>&1
+// RUN: %clang -### %s --target=msp430 -c 2>&1 | FileCheck%s
+// CHECK: "-cc1" "-triple" "msp430"
+// CHECK-SAME: "-fno-signed-char"

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Copy link
Collaborator

@asl asl left a comment

Choose a reason for hiding this comment

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

Thanks!

@asl asl merged commit 7b7ae72 into main Nov 14, 2024
8 checks passed
@asl asl deleted the users/arichardson/spr/msp430-default-to-unsigned-char branch November 14, 2024 20:49
arichardson added a commit that referenced this pull request Dec 2, 2024
This matches GCC.

Partially addresses #115964

Pull Request: #115967
espressif-bot pushed a commit to espressif/llvm-project that referenced this pull request Dec 24, 2024
This matches GCC.

Partially addresses llvm#115964

Pull Request: llvm#115967
espressif-bot pushed a commit to espressif/llvm-project that referenced this pull request Jan 17, 2025
This matches GCC.

Partially addresses llvm#115964

Pull Request: llvm#115967
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 28, 2025
Update comments and sort target_arch in c_char_definition

Follow-up to rust-lang#132975.

- Clang's wrong default on MSP430 has been fixed in llvm/llvm-project#115964, and will be included in LLVM 20, which will be used soon.
- Add a reference on Xtensa's default (from rust-lang#132975 (comment)).
- Fix link for Windows's default.
- Add a link to the discussion on L4Re (rust-lang#132975 (comment))
- Sort `target_arch`. (now match with `target_arch`s in comments)

r? `@tgross35`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 28, 2025
Rollup merge of rust-lang#136173 - taiki-e:c-char, r=tgross35

Update comments and sort target_arch in c_char_definition

Follow-up to rust-lang#132975.

- Clang's wrong default on MSP430 has been fixed in llvm/llvm-project#115964, and will be included in LLVM 20, which will be used soon.
- Add a reference on Xtensa's default (from rust-lang#132975 (comment)).
- Fix link for Windows's default.
- Add a link to the discussion on L4Re (rust-lang#132975 (comment))
- Sort `target_arch`. (now match with `target_arch`s in comments)

r? `@tgross35`
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
Update comments and sort target_arch in c_char_definition

Follow-up to rust-lang#132975.

- Clang's wrong default on MSP430 has been fixed in llvm/llvm-project#115964, and will be included in LLVM 20, which will be used soon.
- Add a reference on Xtensa's default (from rust-lang#132975 (comment)).
- Fix link for Windows's default.
- Add a link to the discussion on L4Re (rust-lang#132975 (comment))
- Sort `target_arch`. (now match with `target_arch`s in comments)

r? `@tgross35`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:MSP430 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.

Clang's isSignedCharDefault() is incorrect for multiple architectures
3 participants