Re: transaction_safe exceptions prevent libstdc++ building for some targets
On 18/01/2017 19:24, DJ Delorie wrote:
> Joe Seymour <jo...@somniumtech.com> writes:
>>> the msp430 -mlarge multilib failing to build with...
>>>> configure: error: Unknown underlying type for size_t
>>>> make[1]: *** [configure-target-libstdc++-v3] Error 1
>>
>> This is still reproducible.
>
> FYI the underlying type is uint20_t
>
> I think I've complained that libstdc++ has a hard-coded list, rather
> than using a configure-time check, in the past...
>
Thanks!
Here's the patch I'm proposing. I've tested it as follows:
- msp430-elf no longer encounters the error when configuring libstdc++-v3. Note
that libstdc++-v3 doesn't build due to an ICE though.
- Configuring libstdc++-v3 for x86_64-unknown-linux-gnu produces:
include/bits/c++config.h:#define _GLIBCXX_MANGLE_SIZE_T m
Both with and without this patch.
- Configuring libstdc++-v3 for i686-unknown-linux-gnu produces:
include/bits/c++config.h:#define _GLIBCXX_MANGLE_SIZE_T j
Both with and without this patch.
If it's acceptable I would appreciate it if someone would commit it on my
behalf.
Thanks,
2017-01-19 Joe Seymour <jo...@somniumtech.com>
libstdc++-v3/
* acinclude.m4 (GLIBCXX_CHECK_SIZE_T_MANGLING): Support uint20_t.
* configure: Regenerate.
---
libstdc++-v3/acinclude.m4 |8 ++--
libstdc++-v3/configure| 18 ++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 4e04cce..d9859aa 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4460,8 +4460,12 @@ AC_DEFUN([GLIBCXX_CHECK_SIZE_T_MANGLING], [
[glibcxx_cv_size_t_mangling=y], [
AC_TRY_COMPILE([],
[extern __SIZE_TYPE__ x; extern unsigned short x;],
- [glibcxx_cv_size_t_mangling=t],
- [glibcxx_cv_size_t_mangling=x])
+ [glibcxx_cv_size_t_mangling=t], [
+AC_TRY_COMPILE([],
+ [extern __SIZE_TYPE__ x; extern __int20 unsigned
x;],
+ [glibcxx_cv_size_t_mangling=u6uint20],
+ [glibcxx_cv_size_t_mangling=x])
+ ])
])
])
])
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 219a6a3..9bb9862 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -80707,6 +80707,21 @@ _ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
glibcxx_cv_size_t_mangling=t
else
+
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+extern __SIZE_TYPE__ x; extern __int20 unsigned x;
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ glibcxx_cv_size_t_mangling=u6uint20
+else
glibcxx_cv_size_t_mangling=x
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
@@ -80721,6 +80736,9 @@ fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_size_t_mangling"
>&5
$as_echo "$glibcxx_cv_size_t_mangling" >&6; }
if test $glibcxx_cv_size_t_mangling = x; then
--
1.7.1
Re: transaction_safe exceptions prevent libstdc++ building for some targets
On 17/08/2016 12:19, Joe Seymour wrote:
> fail to build with...
>
>> ../../../../../libstdc++-v3/src/c++11/cow-stdexcept.cc:274:3: error: static
>> assertion failed: Pointers must be 32 bits or 64 bits wide
>> static_assert(sizeof(uint64_t) == sizeof(void*)
>
> The assert fails because msp430 has 16-bit or 20-bit pointers. The same error
> can be reproduced for the rl78-elf target, which has 16-bit pointers.
With current master, the rl78-elf build completes successfully and the
msp430-elf build doesn't seem to trigger the assertion, but does later fail
with a presumably unrelated ICE.
> the msp430 -mlarge multilib failing to build with...
>> configure: error: Unknown underlying type for size_t
>> make[1]: *** [configure-target-libstdc++-v3] Error 1
This is still reproducible.
> Disabling the original changes for targets with unsupported pointer sizes
> seems like a reasonable solution to me [...]
> Presumably, this would be achieved by adding an
> --{enable,disable}-transactional-memory argument to configure
I have an initial/partial patch that adds a new configure argument:
--enable-transactional-memory=yes|no|auto
Having said that, since I now only see issues with msp430-elf, I wonder if the
configure argument might be excessive, and if my original patch to
GLIBCXX_CHECK_SIZE_T_MANGLING might be more appropriate?
[PATCH 1/2] [msp430] Remove source files from libmul archives
The msp430 libmul archives currently contain several source files. If you run msp430-elf-nm on these archives it will produce output on stderr: > msp430-elf-nm: enable-execute-stack.c: File format not recognized ... > msp430-elf-nm: libgcc_tm.h: File format not recognized It looks like the source files are added as dependencies of libmul by libgcc/Makefile: > $(EXTRA_PARTS): $(LIBGCC_LINKS) libgcc_tm.h This patch adds filters to t-msp430, to ensure that only object files are added to the archives. Built and tested (no regressions) as follows: Configured with: --target=msp430-elf --enable-languages=c Test variations: msp430-sim/-mcpu=msp430 msp430-sim/-mcpu=msp430x msp430-sim/-mcpu=msp430x/-mlarge/-mdata-region=either/-mcode-region=either msp430-sim/-mhwmult=none msp430-sim/-mhwmult=f5series If this patch is acceptable, I would appreciate it if someone would commit it on my behalf. Thanks, 2017-01-XX Joe Seymour <jo...@somniumtech.com> libgcc/ * config/msp430/t-msp430 (libmul_none.a, libmul_16.a, libmul_32.a) (libmul_f5.a): Filter archived prerequisites. --- libgcc/config/msp430/t-msp430 |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libgcc/config/msp430/t-msp430 b/libgcc/config/msp430/t-msp430 index 2428f54..107eb3d 100644 --- a/libgcc/config/msp430/t-msp430 +++ b/libgcc/config/msp430/t-msp430 @@ -59,16 +59,16 @@ lib2hw_mul_f5.o: $(srcdir)/config/msp430/lib2hw_mul.S $(gcc_compile) $< -c -DMUL_F5 libmul_none.a: lib2_mul_none.o - $(AR_CREATE_FOR_TARGET) $@ $^ + $(AR_CREATE_FOR_TARGET) $@ $(filter %.o,$^) libmul_16.a: lib2hw_mul_16.o lib2_mul_16bit.o - $(AR_CREATE_FOR_TARGET) $@ $^ + $(AR_CREATE_FOR_TARGET) $@ $(filter %.o,$^) libmul_32.a: lib2hw_mul_32.o - $(AR_CREATE_FOR_TARGET) $@ $^ + $(AR_CREATE_FOR_TARGET) $@ $(filter %.o,$^) libmul_f5.a: lib2hw_mul_f5.o - $(AR_CREATE_FOR_TARGET) $@ $^ + $(AR_CREATE_FOR_TARGET) $@ $(filter %.o,$^) # Local Variables: # mode: Makefile -- 1.7.1
[PATCH 2/2] [msp430] Remove mpy.o from libgcc
This patch fixes "multiple definition of `__mspabi_mpyi'" errors
encountered with -mhwmult=f5series, by moving mpy.o out of libgcc and
into libmul_none.a. The other libmul_*.a archives already provide a
definition of _mspabi_mpyi.
mpy.c actually contains __mulhi3, but the msp430 backend renames that to
_mspabi_mpyi.
Built and tested (no regressions) as follows:
Configured with: --target=msp430-elf --enable-languages=c
Test variations:
msp430-sim/-mcpu=msp430
msp430-sim/-mcpu=msp430x
msp430-sim/-mcpu=msp430x/-mlarge/-mdata-region=either/-mcode-region=either
msp430-sim/-mhwmult=none
msp430-sim/-mhwmult=f5series
I've also compared the output from msp430-elf-nm for
lib/gcc/msp430-elf/7.0.0/lib*.a on before/after builds, confirming that
moving _msp430abi_mpyi from libgcc to libmul_none.a is the only change.
Test results actually show several progressions:
# of expected passes +721
# of unexpected failures -396
# of unresolved testcases -26
If this patch is acceptable, I would appreciate it if someone would
commit it on my behalf.
Thanks,
2017-01-XX Joe Seymour <jo...@somniumtech.com>
libgcc/
* config/msp430/t-msp430 (LIB2ADD): Remove mpy.c
(mpy.o): New rule.
(libmul_none.a): Add mpy.o
gcc/testsuite/
* gcc.target/msp430/mul_f5_muldef.c: New test.
---
gcc/testsuite/gcc.target/msp430/mul_f5_muldef.c | 15 +++
libgcc/config/msp430/t-msp430 |6 --
2 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/msp430/mul_f5_muldef.c
diff --git a/gcc/testsuite/gcc.target/msp430/mul_f5_muldef.c
b/gcc/testsuite/gcc.target/msp430/mul_f5_muldef.c
new file mode 100644
index 000..da1b1bb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/mul_f5_muldef.c
@@ -0,0 +1,15 @@
+/* { dg-do link } */
+/* { dg-options "-mhwmult=f5series" } */
+
+/* This program used to result in a multiple definition error:
+
+libmul_f5.a(lib2hw_mul_f5.o): In function `__mulhi2_f5':
+(.text.__mulhi2_f5+0x0): multiple definition of `__mspabi_mpyi'
+libgcc.a(mpy.o):mpy.c:(.text.__mulhi3+0x0): first defined here */
+
+#include
+
+int main (void)
+{
+ printf ("%d", 430);
+}
diff --git a/libgcc/config/msp430/t-msp430 b/libgcc/config/msp430/t-msp430
index 107eb3d..668b943 100644
--- a/libgcc/config/msp430/t-msp430
+++ b/libgcc/config/msp430/t-msp430
@@ -30,7 +30,6 @@ LIB2ADD = \
$(srcdir)/config/msp430/lib2mul.c \
$(srcdir)/config/msp430/lib2shift.c \
$(srcdir)/config/msp430/epilogue.S \
- $(srcdir)/config/msp430/mpy.c \
$(srcdir)/config/msp430/slli.S \
$(srcdir)/config/msp430/srai.S \
$(srcdir)/config/msp430/srli.S \
@@ -43,6 +42,9 @@ LIB2ADD = \
HOST_LIBGCC2_CFLAGS += -Os -ffunction-sections -fdata-sections -mhwmult=none
+mpy.o: $(srcdir)/config/msp430/mpy.c
+ $(gcc_compile) $< -c
+
lib2_mul_none.o: $(srcdir)/config/msp430/lib2mul.c
$(gcc_compile) $< -c -DMUL_NONE
@@ -58,7 +60,7 @@ lib2hw_mul_32.o: $(srcdir)/config/msp430/lib2hw_mul.S
lib2hw_mul_f5.o: $(srcdir)/config/msp430/lib2hw_mul.S
$(gcc_compile) $< -c -DMUL_F5
-libmul_none.a: lib2_mul_none.o
+libmul_none.a: lib2_mul_none.o mpy.o
$(AR_CREATE_FOR_TARGET) $@ $(filter %.o,$^)
libmul_16.a: lib2hw_mul_16.o lib2_mul_16bit.o
--
1.7.1
[PATCH] [msp430] Sync msp430_mcu_data with devices.csv
This patch syncs the generated msp430_mcu_data structure:
- With the latest version of devices.csv released by TI.
- Between msp430.c and driver-msp430.c. The former was updated more
recently than the latter.
- With the copy of the same data structure in binutils, for which a
similar patch has already been approved and committed:
https://sourceware.org/ml/binutils/2016-12/msg00347.html
My understanding is that the devices being removed were "invalid spins",
so can't be being used by anyone, and never will be. Web searches
related to these devices return no relevant results:
msp430 FR5862: No results.
msp430 FR5864: 1 (invalid) result.
msp430 FR5892: No results.
msp430 FR5894: No results.
This patch does not update the structure in t-msp430, which contains
only msp430 devices (as opposed to msp430x devices), none of which are
being added by this patch.
Built and tested (no regressions) as follows:
Configured with: --target=msp430-elf --enable-languages=c
Test variations:
msp430-sim/-mcpu=msp430
msp430-sim/-mcpu=msp430x
msp430-sim/-mcpu=msp430x/-mlarge/-mdata-region=either/-mcode-region=either
msp430-sim/-mhwmult=none
msp430-sim/-mhwmult=f5series
I don't have write access, so if this patch is acceptable I'd appreciate
it if someone would commit it for me.
Thanks,
2017-01-03 Joe Seymour <jo...@somniumtech.com>
* config/msp430/driver-msp430.c (msp430_mcu_data): Sync with data
from TI's devices.csv file as of September 2016.
* config/msp430/msp430.c (msp430_mcu_data): Likewise.
---
gcc/config/msp430/driver-msp430.c | 17 +++--
gcc/config/msp430/msp430.c| 11 +--
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/gcc/config/msp430/driver-msp430.c
b/gcc/config/msp430/driver-msp430.c
index 69b7a73..b6b5676 100644
--- a/gcc/config/msp430/driver-msp430.c
+++ b/gcc/config/msp430/driver-msp430.c
@@ -27,8 +27,8 @@
/* This is a copy of the same data structure found in gas/config/tc-msp430.c
Also another (sort-of) copy can be found in gcc/config/msp430/msp430.c
Keep these three structures in sync.
- The data in this structure has been extracted from the devices.csv file
- released by TI, updated as of 8 October 2015. */
+ The data in this structure has been extracted from version 1.194 of the
+ devices.csv file released by TI in September 2016. */
struct msp430_mcu_data
{
@@ -454,7 +454,15 @@ msp430_mcu_data [] =
{ "msp430fg6626",2,8 },
{ "msp430fr2032",2,0 },
{ "msp430fr2033",2,0 },
+ { "msp430fr2110",2,0 },
+ { "msp430fr2111",2,0 },
+ { "msp430fr2310",2,0 },
+ { "msp430fr2311",2,0 },
{ "msp430fr2433",2,8 },
+ { "msp430fr2532",2,8 },
+ { "msp430fr2533",2,8 },
+ { "msp430fr2632",2,8 },
+ { "msp430fr2633",2,8 },
{ "msp430fr2xx_4xxgeneric",2,8 },
{ "msp430fr4131",2,0 },
{ "msp430fr4132",2,0 },
@@ -507,6 +515,8 @@ msp430_mcu_data [] =
{ "msp430fr5957",2,8 },
{ "msp430fr5958",2,8 },
{ "msp430fr5959",2,8 },
+ { "msp430fr5962",2,8 },
+ { "msp430fr5964",2,8 },
{ "msp430fr5967",2,8 },
{ "msp430fr5968",2,8 },
{ "msp430fr5969",2,8 },
@@ -519,6 +529,9 @@ msp430_mcu_data [] =
{ "msp430fr5988",2,8 },
{ "msp430fr5989",2,8 },
{ "msp430fr59891",2,8 },
+ { "msp430fr5992",2,8 },
+ { "msp430fr5994",2,8 },
+ { "msp430fr59941",2,8 },
{ "msp430fr5xx_6xxgeneric",2,8 },
{ "msp430fr6820",2,8 },
{ "msp430fr6822",2,8 },
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index fb1978b..fe92370 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -93,8 +93,8 @@ msp430_init_machine_status (void)
/* This is a copy of the same data structure found in gas/config/tc-msp430.c
Also another (sort-of) copy can be found in gcc/config/msp430/t-msp430
Keep these three structures in sync.
- The data in this structure has been extracted from the devices.csv file
- released by TI, updated as of March 2016. */
+ The data in this structure has been extracted from version 1.194 of the
+ devices.csv file released by TI in September 2016. */
struct msp430_mcu_data
{
@@ -520,6 +520,8 @@ msp430_mcu_data [] =
{ "msp430fg6626",2,8 },
{ "msp430fr2032",2,0 },
{ "msp430fr2033",2,0 },
+ { "msp430fr2110",2,0 },
+ { "msp430fr2111",2,0 },
{ "msp430fr2310",2,0 },
{ "msp430fr2311",2,0 },
{ "msp430fr2433",2,8 },
@@ -560,8 +562,6 @@ msp430_mcu_data [] =
{ "msp430fr5858",2,8 },
{ "msp430fr5859",2,8 },
{ "msp430fr5867",2,8 },
- { "msp
[PING][PATCH][msp430] Don't output __interrupt_vector sections for weak functions
Is the revised patch acceptable?
If it is, I don't have commit access, so I'd apprecite it if someone
could commit it for me. Similarly, I can comment on the Bugzilla
entry but can't change it's state - assuming that would be appropriate
if the patch is accepted.
Thanks,
Joe
On 01/09/2016 17:18, Joe Seymour wrote:
> On 29/08/2016 22:09, DJ Delorie wrote:
>>
>> Which results in a more user-obvious case, ignoring the interrupt
>> attribute or ignoring the weak attribute? I would think that we never
>> want to compile and link successfully if we can't do what the user
>> wants, and omitting an interrupt handler is... bad.
>>
>> I think this should either be a hard error, so the user must fix it
>> right away, or ignore the weak, which results in a linker error
>> (somehow? maybe?) if the user specifies two handlers for the same
>> interrupt.
>
> Thanks for the review. My original intention was to make it an error, but
> msp430_attr() seemed to set the precedent of emitting warnings for invalid
> attributes, so I tried to follow that convention.
>
> Here's a patch that produces an error instead:
>
> 2016-09-XX Joe Seymour <jo...@somniumtech.com>
>
> gcc/
> PR target/70713
> * config/msp430/msp430.c (msp430_start_function): Emit an error
> if a function is both weak and specifies an interrupt number.
>
> gcc/testsuite/
> PR target/70713
> * gcc.target/msp430/function-attributes-1.c: New test.
> * gcc.target/msp430/function-attributes-2.c: New test.
> * gcc.target/msp430/function-attributes-3.c: New test.
>
> diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
> index dba4d19..c40d2da 100644
> --- a/gcc/config/msp430/msp430.c
> +++ b/gcc/config/msp430/msp430.c
> @@ -2108,6 +2108,13 @@ msp430_start_function (FILE *file, const char *name,
> tree decl)
> {
> char buf[101];
>
> + /* Interrupt vector sections should be unique, but use of weak
> + functions implies multiple definitions. */
> + if (DECL_WEAK (decl))
> + {
> + error ("argument to interrupt attribute is unsupported for weak
> functions");
> + }
> +
> intr_vector = TREE_VALUE (intr_vector);
>
> /* The interrupt attribute has a vector value. Turn this into a
> diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
> b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
> new file mode 100644
> index 000..7a3b7be
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
> @@ -0,0 +1,9 @@
> +void __attribute__((weak, interrupt))
> +weak_interrupt (void) {
> +}
> +
> +void __attribute__((interrupt(11)))
> +interrupt_number (void) {
> +}
> +
> +/* { dg-final { scan-assembler-times "__interrupt_vector_" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
> b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
> new file mode 100644
> index 000..fcb2fb2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
> @@ -0,0 +1,3 @@
> +void __attribute__((weak, interrupt(10)))
> +weak_interrupt_number (void) {
> +} /* { dg-error "argument to interrupt attribute is unsupported for weak
> functions" } */
> diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
> b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
> new file mode 100644
> index 000..b0acf4a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
> @@ -0,0 +1,3 @@
> +void __attribute__((interrupt("nmi"))) __attribute__((weak))
> +interrupt_name_weak (void) {
> +} /* { dg-error "argument to interrupt attribute is unsupported for weak
> functions" } */
>
>
Re: [PATCH][msp430] Don't output __interrupt_vector sections for weak functions
On 29/08/2016 22:09, DJ Delorie wrote:
>
> Which results in a more user-obvious case, ignoring the interrupt
> attribute or ignoring the weak attribute? I would think that we never
> want to compile and link successfully if we can't do what the user
> wants, and omitting an interrupt handler is... bad.
>
> I think this should either be a hard error, so the user must fix it
> right away, or ignore the weak, which results in a linker error
> (somehow? maybe?) if the user specifies two handlers for the same
> interrupt.
Thanks for the review. My original intention was to make it an error, but
msp430_attr() seemed to set the precedent of emitting warnings for invalid
attributes, so I tried to follow that convention.
Here's a patch that produces an error instead:
2016-09-XX Joe Seymour <jo...@somniumtech.com>
gcc/
PR target/70713
* config/msp430/msp430.c (msp430_start_function): Emit an error
if a function is both weak and specifies an interrupt number.
gcc/testsuite/
PR target/70713
* gcc.target/msp430/function-attributes-1.c: New test.
* gcc.target/msp430/function-attributes-2.c: New test.
* gcc.target/msp430/function-attributes-3.c: New test.
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index dba4d19..c40d2da 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2108,6 +2108,13 @@ msp430_start_function (FILE *file, const char *name,
tree decl)
{
char buf[101];
+ /* Interrupt vector sections should be unique, but use of weak
+functions implies multiple definitions. */
+ if (DECL_WEAK (decl))
+ {
+ error ("argument to interrupt attribute is unsupported for weak
functions");
+ }
+
intr_vector = TREE_VALUE (intr_vector);
/* The interrupt attribute has a vector value. Turn this into a
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
new file mode 100644
index 000..7a3b7be
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
@@ -0,0 +1,9 @@
+void __attribute__((weak, interrupt))
+weak_interrupt (void) {
+}
+
+void __attribute__((interrupt(11)))
+interrupt_number (void) {
+}
+
+/* { dg-final { scan-assembler-times "__interrupt_vector_" 1 } } */
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
new file mode 100644
index 000..fcb2fb2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
@@ -0,0 +1,3 @@
+void __attribute__((weak, interrupt(10)))
+weak_interrupt_number (void) {
+} /* { dg-error "argument to interrupt attribute is unsupported for weak
functions" } */
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
new file mode 100644
index 000..b0acf4a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
@@ -0,0 +1,3 @@
+void __attribute__((interrupt("nmi"))) __attribute__((weak))
+interrupt_name_weak (void) {
+} /* { dg-error "argument to interrupt attribute is unsupported for weak
functions" } */
[PATCH][msp430] Don't output __interrupt_vector sections for weak functions
The msp430 target supports an "interrupt" attribute, which can take an optional
argument specifying the interrupt number. If this argument is provided then the
compiler will output an __interrupt_vector_ section that the linker
script can use to create a vector table.
PR target/70713 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70713) reports
that if an __interrupt_vector section is generated for a weak function, this
section won't be discarded when the linker discards the weak function, which
results in multiple __interrupt_vector sections that the linker is unable
to choose between.
This patch seeks to avoid the issue by not generating __interrupt_vector
sections for weak functions, instead emitting a warning.
I considered implementing the warning in msp430_attr(), but that only works if
the weak attribute is specified before the interrupt attribute. Instead I've
implemented the whole patch in msp430_start_function (aside: there are two
functions named msp430_start_function).
I've moved part of msp430_start_function into a new helper function, largely to
keep line length down. I haven't tried to keep the line that calls warning
under 79 characters, as other parts of msp430.c don't.
Built and tested msp430-elf-gcc without regressions. I haven't done any c++
testing because that doesn't build for msp430-elf at the moment:
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01256.html
Note that I don't have commit access. If this patch (or some future version of
it) is OK, I'd appreciate it if someone could commit it for me.
Thanks,
2016-08-XX Joe Seymour <jo...@somniumtech.com>
gcc/
PR target/70713
* config/msp430/msp430.c (msp430_start_function): Don't output
interrupt vectors for weak functions. Issue a warning.
(msp430_output_interrupt_vector): New helper function.
gcc/testsuite/
PR target/70713
* gcc.target/msp430/function-attributes.c: New test.
---
gcc/config/msp430/msp430.c | 58 +---
.../gcc.target/msp430/function-attributes.c| 17 ++
2 files changed, 55 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/msp430/function-attributes.c
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index dba4d19..ee9cdf9 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2094,6 +2094,32 @@ increment_stack (HOST_WIDE_INT amount)
}
}
+static void
+msp430_output_interrupt_vector (FILE *file, const char *name,
+ tree decl, tree intr_vector)
+{
+ char buf[101];
+
+ intr_vector = TREE_VALUE (intr_vector);
+
+ /* The interrupt attribute has a vector value. Turn this into a
+ section name, switch to that section and put the address of
+ the current function into that vector slot. Note msp430_attr()
+ has already verified the vector name for us. */
+ if (TREE_CODE (intr_vector) == STRING_CST)
+sprintf (buf, "__interrupt_vector_%.80s",
+TREE_STRING_POINTER (intr_vector));
+ else /* TREE_CODE (intr_vector) == INTEGER_CST */
+sprintf (buf, "__interrupt_vector_%u",
+(unsigned int) TREE_INT_CST_LOW (intr_vector));
+
+ switch_to_section (get_section (buf, SECTION_CODE, decl));
+ fputs ("\t.word\t", file);
+ assemble_name (file, name);
+ fputc ('\n', file);
+ fputc ('\t', file);
+}
+
void
msp430_start_function (FILE *file, const char *name, tree decl)
{
@@ -2106,26 +2132,18 @@ msp430_start_function (FILE *file, const char *name,
tree decl)
if (intr_vector != NULL_TREE)
{
- char buf[101];
-
- intr_vector = TREE_VALUE (intr_vector);
-
- /* The interrupt attribute has a vector value. Turn this into a
-section name, switch to that section and put the address of
-the current function into that vector slot. Note msp430_attr()
-has already verified the vector name for us. */
- if (TREE_CODE (intr_vector) == STRING_CST)
- sprintf (buf, "__interrupt_vector_%.80s",
-TREE_STRING_POINTER (intr_vector));
- else /* TREE_CODE (intr_vector) == INTEGER_CST */
- sprintf (buf, "__interrupt_vector_%u",
-(unsigned int) TREE_INT_CST_LOW (intr_vector));
-
- switch_to_section (get_section (buf, SECTION_CODE, decl));
- fputs ("\t.word\t", file);
- assemble_name (file, name);
- fputc ('\n', file);
- fputc ('\t', file);
+ /* Interrupt vector sections should be unique, use of weak functions
+implies multiple definitions, so don't generate vector table
+entries for weak functions. */
+ if (DECL_WEAK (decl))
+ {
+ warning (OPT_Wattributes,
+"argument to interrupt attribute is ignored for weak
functions");
+
Re: transaction_safe exceptions prevent libstdc++ building for some targets
On 17/08/2016 12:30, Jonathan Wakely wrote:
> On 17/08/16 12:19 +0100, Joe Seymour wrote:
>> Disabling the original changes for targets with unsupported pointer sizes
>> seems
>> like a reasonable solution to me, but I can't see an existing mechanism to do
>> so? Do others agree?
>
> Yes, the intention was that the transaction-safe exceptions would only
> be defined where it is relatively straightforward to support them. If
> they're causing build failures they can be simply disabled for that
> target. Maybe the configure script should check for 32-bit or 64-bit
> pointers and completely disable the TM exceptions otherwise.
>
>> More generally, it might be useful to have a mechanism to disable
>> transactional
>> memory for some targets. I've observed things like register_tm_clones taking
>> up
>> space in ARM Cortex-M binaries. Presumably, this would be achieved by adding
>> an
>> --{enable,disable}-transactional-memory argument to configure? I'm happy to
>> work on a patch, if this is acceptable in principle.
>
> I think that makes sense, for cases where the target can support the
> TM clones but they aren't wanted.
>
> If we add such an option then the checks for 32/64-bit pointers can go
> in there, so that when the option isn't used the default depends on
> the target.
>
I'll start working on a patch.
Thanks,
Joe
transaction_safe exceptions prevent libstdc++ building for some targets
../configure --target=msp430-elf --enable-languages=c,c++ && make -j4
Results in the msp430 -mlarge multilib failing to build with...
> configure: error: Unknown underlying type for size_t
> make[1]: *** [configure-target-libstdc++-v3] Error 1
This relates to...
> commit 13143e139230dc4d72710a6c4c312105aeddce4f
> Author: torvald <torvald@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date: Fri Jan 15 22:42:41 2016 +
>
>libstdc++: Make certain exceptions transaction_safe.
https://gcc.gnu.org/ml/libstdc++/2016-01/msg00015.html
...which iterates through a list of types at configure time, looking for one
matching __SIZE_TYPE__. The following patch (wip) allows the build to
proceed
(to the next error), by adding __int20 to the list of types to try.
- Is this an acceptable solution?
- There's a comment above GLIBCXX_CHECK_SIZE_T_MANGLING, saying that it was
copied from libitm. Should that be updated too, even if it's
irrelevant for
msp430?
Proceeding past that build error, all multilibs then fail to build with...
> ../../../../../libstdc++-v3/src/c++11/cow-stdexcept.cc:274:3: error:
static assertion failed: Pointers must be 32 bits or 64 bits wide
> static_assert(sizeof(uint64_t) == sizeof(void*)
The assert fails because msp430 has 16-bit or 20-bit pointers. The same
error
can be reproduced for the rl78-elf target, which has 16-bit pointers.
Disabling the original changes for targets with unsupported pointer
sizes seems
like a reasonable solution to me, but I can't see an existing mechanism
to do
so? Do others agree?
More generally, it might be useful to have a mechanism to disable
transactional
memory for some targets. I've observed things like register_tm_clones
taking up
space in ARM Cortex-M binaries. Presumably, this would be achieved by
adding an
--{enable,disable}-transactional-memory argument to configure? I'm happy to
work on a patch, if this is acceptable in principle.
Thanks,
2016-08-XX Joe Seymour <jo...@somniumtech.com>
* acinclude.m4 (GLIBCXX_CHCK_SIZE_T_MANGLING): Try __int20.
* configure: Regenerate.
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index b0f88cb..7332d3e 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4405,9 +4405,13 @@ AC_DEFUN([GLIBCXX_CHECK_SIZE_T_MANGLING], [
[extern __SIZE_TYPE__ x; extern unsigned long
long x;],
[glibcxx_cv_size_t_mangling=y], [
AC_TRY_COMPILE([],
- [extern __SIZE_TYPE__ x; extern unsigned short
x;],
- [glibcxx_cv_size_t_mangling=t],
- [glibcxx_cv_size_t_mangling=x])
+ [extern __SIZE_TYPE__ x; extern unsigned long
long x;],
+ [glibcxx_cv_size_t_mangling=y], [
+AC_TRY_COMPILE([],
+ [extern __SIZE_TYPE__ x; extern __int20
unsigned x;],
+ [glibcxx_cv_size_t_mangling=u6uint20],
+ [glibcxx_cv_size_t_mangling=x])
+ ])
])
])
])
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 41797a9..7ab84eb 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -80475,13 +80475,28 @@ else
int
main ()
{
-extern __SIZE_TYPE__ x; extern unsigned short x;
+extern __SIZE_TYPE__ x; extern unsigned long long x;
;
return 0;
}
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
- glibcxx_cv_size_t_mangling=t
+ glibcxx_cv_size_t_mangling=y
+else
+
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+extern __SIZE_TYPE__ x; extern __int20 unsigned x;
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ glibcxx_cv_size_t_mangling=u6uint20
else
glibcxx_cv_size_t_mangling=x
fi
@@ -80497,6 +80512,9 @@ fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
$glibcxx_cv_size_t_mangling" >&5
$as_echo "$glibcxx_cv_size_t_mangling" >&6; }
if test $glibcxx_cv_size_t_mangling = x; then
[PATCH] Adjust target for vect/pr48765.c
I'm observing vect/pr48765.c fail for non 64-bit PowerPC targets:
gcc/testsuite/gcc.dg/vect/pr48765.c:1:0: error: -m64 not supported in this
configuration
This patch restricts the test to 64-bit PowerPC targets. I don't have commit
access, so if this OK perhaps someone could commit it for me?
Thanks,
2012-10-12 Joe Seymour jseym...@codesourcery.com
* gcc.dg/vect/pr48765.c: Restrict to 64-bit PowerPC targets.
Index: gcc/testsuite/gcc.dg/vect/pr48765.c
===
--- gcc/testsuite/gcc.dg/vect/pr48765.c (revision 192402)
+++ gcc/testsuite/gcc.dg/vect/pr48765.c (working copy)
@@ -1,4 +1,4 @@
-/* { dg-do compile { target powerpc*-*-* } } */
+/* { dg-do compile { target powerpc64 } } */
/* { dg-options -m64 -O3 -mcpu=power6 } */
enum reg_class
[testsuite] Prune irrelevant warning from pr53060.c
The pr53060.c test for excess errors fails on some PowerPC targets with an
unexpected warning: GCC vector returned by reference: non-standard ABI
extension with no compatibility guarantee
This patch prunes the irrelevant warning as in pr34856.c. I don't have commit
access, so perhaps someone could commit this if it's OK?
Thanks,
2012-10-12 Joe Seymour jseym...@codesourcery.com
* gcc.dg/pr53060.c: Prune irrelevant warning.
Index: gcc/testsuite/gcc.dg/pr53060.c
===
--- gcc/testsuite/gcc.dg/pr53060.c (revision 192269)
+++ gcc/testsuite/gcc.dg/pr53060.c (working copy)
@@ -22,3 +22,6 @@ int main()
if (i[0] != 3) abort();
return 0;
}
+
+/* Ignore a warning that is irrelevant to the purpose of this test. */
+/* { dg-prune-output .*GCC vector returned by reference.* } */
Re: [PATCH] Fix inclusion of cxxabi_forced.h in dynamic_bitset
On 10/06/12 01:50, Paolo Carlini wrote: On 10/06/2012 02:33 AM, Joe Seymour wrote: I'm seeing tr2/headers/all.cc fail in the libstdc++ testsuite: In file included from src/gcc-mainline/libstdc++-v3/testsuite/tr2/headers/all.cc:22:0: /scratch/jseymour/mainline/i686-pc-linux-gnu/install/opt/codesourcery/include/c++/4.8.0/tr2/dynamic_bitset:42:27: fatal error: cxxabi_forced.h: No such file or directory #include cxxabi_forced.h ^ compilation terminated. From libstdc++-v3/libsupc++/Makefile.am: bits_HEADERS = \ atomic_lockfree_defines.h cxxabi_forced.h \ exception_defines.h exception_ptr.h hash_bytes.h nested_exception.h Looking at how other headers in that list are treated, I believe it is the include of cxxabi_forced.h in dynamic_bitset at fault. This patch corrects it. I'm pretty sure you are right. Any idea why the test isn't failing for anybody else? I was surprised not to find any other references to this failure as well, especially as I observed the failure with pristine FSF sources. I've had a closer look: * We (CodeSourcery/Mentor) test the installation directory, with something like: g++ -D_GLIBCXX_ASSERT -fmessage-length=0 -DLOCALEDIR=. -I/scratch/jseymour/mainline/i686-pc-linux-gnu/src/gcc-mainline/libstdc++-v3/testsuite/util \ /scratch/jseymour/mainline/i686-pc-linux-gnu/src/gcc-mainline/libstdc++-v3/testsuite/tr2/headers/all.cc -std=gnu++0x -S -o all.s * The standard make check invocation tests the objdir/srcdir with a longer command, passing various paths etc, in particular: -I/scratch/jseymour/mainline/i686-pc-linux-gnu/src/gcc-mainline/libstdc++-v3/libsupc++ Because all the headers in libstdc++-v3 are in that directory cxxabi_forced.h is found successfully. It is the Makefile that places it in a different directory during installation. I suppose to get this test working correctly, we need to move the files listed in bits_HEADERS into a bits/ directory in the source tree, then make appropriate changes to cater for the adjusted directory layout. Joe
[PATCH] Fix inclusion of cxxabi_forced.h in dynamic_bitset
I'm seeing tr2/headers/all.cc fail in the libstdc++ testsuite:
In file included from
src/gcc-mainline/libstdc++-v3/testsuite/tr2/headers/all.cc:22:0:
/scratch/jseymour/mainline/i686-pc-linux-gnu/install/opt/codesourcery/include/c++/4.8.0/tr2/dynamic_bitset:42:27:
fatal error: cxxabi_forced.h: No such file or directory
#include cxxabi_forced.h
^
compilation terminated.
From libstdc++-v3/libsupc++/Makefile.am:
bits_HEADERS = \
atomic_lockfree_defines.h cxxabi_forced.h \
exception_defines.h exception_ptr.h hash_bytes.h nested_exception.h
Looking at how other headers in that list are treated, I believe it is the
include of cxxabi_forced.h in dynamic_bitset at fault. This patch corrects it.
I don't have commit access, so if this is acceptable perhaps someone would be
kind enough to commit it.
Thanks,
2012-10-06 Joe Seymour jseym...@codesourcery.com
* include/tr2/dynamic_bitset (cxxabi_forced.h): Fix include.
Index: libstdc++-v3/include/tr2/dynamic_bitset
===
--- libstdc++-v3/include/tr2/dynamic_bitset (revision 192151)
+++ libstdc++-v3/include/tr2/dynamic_bitset (working copy)
@@ -39,7 +39,7 @@
#include bits/functexcept.h // For invalid_argument, out_of_range,
// overflow_error
#include iosfwd
-#include cxxabi_forced.h
+#include bits/cxxabi_forced.h
namespace std _GLIBCXX_VISIBILITY(default)
{
