Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
On 25 April 2014 16:31, Ramana Radhakrishnan ramana@googlemail.com wrote: On Fri, Apr 25, 2014 at 4:29 PM, Charles Baylis charles.bay...@linaro.org wrote: OK to backport to 4.8 and 4.7? Ok by me but give 24 working hours for an RM to object. Committed to 4.7 as r210227. Committed to 4.8 as r210226. 2014-05-08 Charles Baylis charles.bay...@linaro.org Backport from mainline 2014-04-07 Charles Baylis charles.bay...@linaro.org PR target/60609 * config/arm/arm.h (ASM_OUTPUT_CASE_END): Remove. (LABEL_ALIGN_AFTER_BARRIER): Align barriers which occur after ADDR_DIFF_VEC.
Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
This doesn't seem to have shown problems on trunk/4.9. I have bootstrapped and checked the patch on 4.8 arm-unknown-linux-gnueabihf (Thumb-2) on qemu. I have checked the patch on 4.7 arm-unknown-linux-gnueabihf (Thumb-2) on qemu. OK to backport to 4.8 and 4.7? On 7 April 2014 16:02, Charles Baylis charles.bay...@linaro.org wrote: On 4 April 2014 15:50, Ramana Radhakrishnan ramana@googlemail.com wrote: Additionally the testing has only considered Thumb2 - since we also do jumptable shortening for Thumb1 and given this late change it's worth also testing this on Thumb1 and making sure there are no regressions. Maybe Joey can help there if you aren't set up to do this. Ok if no regressions and modulo RM objections. I have tested v5t Thumb-1 with no regressions on qemu. One minor Changelog nit. 2014-04-02 Charles Baylis charles.bay...@linaro.org PR target/60609 * config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove. (LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after ADDR_DIFF_VEC. s/)/):/g above. Noted.
Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
On Fri, Apr 25, 2014 at 4:29 PM, Charles Baylis charles.bay...@linaro.org wrote: This doesn't seem to have shown problems on trunk/4.9. I have bootstrapped and checked the patch on 4.8 arm-unknown-linux-gnueabihf (Thumb-2) on qemu. I have checked the patch on 4.7 arm-unknown-linux-gnueabihf (Thumb-2) on qemu. OK to backport to 4.8 and 4.7? Ok by me but give 24 working hours for an RM to object. Ramana On 7 April 2014 16:02, Charles Baylis charles.bay...@linaro.org wrote: On 4 April 2014 15:50, Ramana Radhakrishnan ramana@googlemail.com wrote: Additionally the testing has only considered Thumb2 - since we also do jumptable shortening for Thumb1 and given this late change it's worth also testing this on Thumb1 and making sure there are no regressions. Maybe Joey can help there if you aren't set up to do this. Ok if no regressions and modulo RM objections. I have tested v5t Thumb-1 with no regressions on qemu. One minor Changelog nit. 2014-04-02 Charles Baylis charles.bay...@linaro.org PR target/60609 * config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove. (LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after ADDR_DIFF_VEC. s/)/):/g above. Noted.
Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
On 4 April 2014 15:50, Ramana Radhakrishnan ramana@googlemail.com wrote: Additionally the testing has only considered Thumb2 - since we also do jumptable shortening for Thumb1 and given this late change it's worth also testing this on Thumb1 and making sure there are no regressions. Maybe Joey can help there if you aren't set up to do this. Ok if no regressions and modulo RM objections. I have tested v5t Thumb-1 with no regressions on qemu. One minor Changelog nit. 2014-04-02 Charles Baylis charles.bay...@linaro.org PR target/60609 * config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove. (LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after ADDR_DIFF_VEC. s/)/):/g above. Noted.
Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
On 04/03/14 11:02, Ramana Radhakrishnan wrote: On Thu, Apr 3, 2014 at 2:27 PM, Charles Baylis charles.bay...@linaro.org wrote: Hi This bug causes the compiler to create a Thumb-2 TBB instruction with a jump table containing an out of range value in a .byte field: whatever.s:148: Error: value of 256 too large for field of 1 bytes at 100 This occurs because the jump table is followed with a .align 1 due to ASM_OUTPUT_CASE_END, but the 'shorten' phase does not account for the space taken by this align directive. My first reaction is to wonder why this is this not a bug in the shorten phase. I don't think that code ever expected an alignment directive to be emitted by ASM_OUTPUT_CASE_END :( This patch addresses the issue by removing ASM_OUTPUT_CASE_END from arm.h, and ensuring that the alignment after an ADDR_DIFF_VEC is instead inserted by aligning the label following the barrier which follows it. This is achieved by defining LABEL_ALIGN_AFTER_BARRIER appropriately. On first glance this feels like a blunt hammer, what's the code size bloat with putting out such an alignment after each barrier that the compiler emits rather than tracking this in ASM_OUTPUT_CASE_END. I'd tend to agree that emitting an alignment after each barrier would be a blunt hammer in this case. ISTM we really want a new target hook to define the alignment after a the jump table, independent of the other alignment directives. Then we'd have to teach shorten_branches about that. Perhaps the blunt hammer for 4.9 and the new alignemnt-after-jump-table for the next stage1? jeff
Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
My first reaction is to wonder why this is this not a bug in the shorten phase. I don't think that code ever expected an alignment directive to be emitted by ASM_OUTPUT_CASE_END :( Fair point and it looks like this support came in when the support for Thumb2 was added eons ago. This patch addresses the issue by removing ASM_OUTPUT_CASE_END from arm.h, and ensuring that the alignment after an ADDR_DIFF_VEC is instead inserted by aligning the label following the barrier which follows it. This is achieved by defining LABEL_ALIGN_AFTER_BARRIER appropriately. On first glance this feels like a blunt hammer, what's the code size bloat with putting out such an alignment after each barrier that the compiler emits rather than tracking this in ASM_OUTPUT_CASE_END. I'd tend to agree that emitting an alignment after each barrier would be a blunt hammer in this case. ISTM we really want a new target hook to define the alignment after a the jump table, independent of the other alignment directives. Then we'd have to teach shorten_branches about that. Perhaps the blunt hammer for 4.9 and the new alignemnt-after-jump-table for the next stage1? Given the following below - it may not be that blunt at all. I don't know how I missed this yesterday (thanks to Charlie for pointing this out on IRC) :( +#define LABEL_ALIGN_AFTER_BARRIER(LABEL)\ + (GET_CODE (PATTERN (prev_active_insn (LABEL))) == ADDR_DIFF_VEC \ + ? 1 : 0) I think we should keep this on trunk for a week to check if there is no unintended fallout before backporting to 4.8 Additionally the testing has only considered Thumb2 - since we also do jumptable shortening for Thumb1 and given this late change it's worth also testing this on Thumb1 and making sure there are no regressions. Maybe Joey can help there if you aren't set up to do this. Ok if no regressions and modulo RM objections. One minor Changelog nit. 2014-04-02 Charles Baylis charles.bay...@linaro.org PR target/60609 * config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove. (LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after ADDR_DIFF_VEC. s/)/):/g above. regards Ramana jeff
[PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
Hi
This bug causes the compiler to create a Thumb-2 TBB instruction with
a jump table containing an out of range value in a .byte field:
whatever.s:148: Error: value of 256 too large for field of 1 bytes at 100
This occurs because the jump table is followed with a .align 1 due
to ASM_OUTPUT_CASE_END, but the 'shorten' phase does not account for
the space taken by this align directive.
This patch addresses the issue by removing ASM_OUTPUT_CASE_END from
arm.h, and ensuring that the alignment after an ADDR_DIFF_VEC is
instead inserted by aligning the label following the barrier which
follows it. This is achieved by defining LABEL_ALIGN_AFTER_BARRIER
appropriately.
Bootstrapped/checked on arm-unknown-linux-gnueabihf.
OK for trunk, and backporting to 4.8?
2014-04-02 Charles Baylis charles.bay...@linaro.org
PR target/60609
* config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove.
(LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after
ADDR_DIFF_VEC.
2014-04-02 Charles Baylis charles.bay...@linaro.org
PR target/60609
* g++.dg/torture/pr60609.C: New test.
From 9b0c1ada23e2b210b02ebaee2f599bb5205a91d6 Mon Sep 17 00:00:00 2001
From: Charles Baylis charles.bay...@linaro.org
Date: Thu, 3 Apr 2014 10:57:33 +0100
Subject: [PATCH] fix for PR target/60609
2014-04-02 Charles Baylis charles.bay...@linaro.org
PR target/60609
* config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove.
(LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after
ADDR_DIFF_VEC.
2014-04-02 Charles Baylis charles.bay...@linaro.org
PR target/60609
* g++.dg/torture/pr60609.C: New test.
---
gcc/config/arm/arm.h | 11 +-
gcc/testsuite/g++.dg/torture/pr60609.C | 252 +
2 files changed, 255 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/torture/pr60609.C
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 7ca47a7..a4bbd12 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2194,14 +2194,9 @@ extern int making_const_table;
#undef ASM_OUTPUT_BEFORE_CASE_LABEL
#define ASM_OUTPUT_BEFORE_CASE_LABEL(FILE, PREFIX, NUM, TABLE) /* Empty. */
-/* Make sure subsequent insns are aligned after a TBB. */
-#define ASM_OUTPUT_CASE_END(FILE, NUM, JUMPTABLE) \
- do \
-{ \
- if (GET_MODE (PATTERN (JUMPTABLE)) == QImode) \
- ASM_OUTPUT_ALIGN (FILE, 1); \
-} \
- while (0)
+#define LABEL_ALIGN_AFTER_BARRIER(LABEL)\
+ (GET_CODE (PATTERN (prev_active_insn (LABEL))) == ADDR_DIFF_VEC \
+ ? 1 : 0)
#define ARM_DECLARE_FUNCTION_NAME(STREAM, NAME, DECL) \
do \
diff --git a/gcc/testsuite/g++.dg/torture/pr60609.C b/gcc/testsuite/g++.dg/torture/pr60609.C
new file mode 100644
index 000..9ddec0b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr60609.C
@@ -0,0 +1,252 @@
+/* { dg-do assemble } */
+
+class exception
+{
+};
+class bad_alloc:exception
+{
+};
+class logic_error:exception
+{
+};
+class domain_error:logic_error
+{
+};
+class invalid_argument:logic_error
+{
+};
+class length_error:logic_error
+{
+};
+class overflow_error:exception
+{
+};
+typedef int mpz_t[];
+template class class __gmp_expr;
+template class __gmp_expr mpz_t
+{
+~__gmp_expr ();
+};
+
+class PIP_Solution_Node;
+class internal_exception
+{
+~internal_exception ();
+};
+class not_an_integer:internal_exception
+{
+};
+class not_a_variable:internal_exception
+{
+};
+class not_an_optimization_mode:internal_exception
+{
+};
+class not_a_bounded_integer_type_width:internal_exception
+{
+};
+class not_a_bounded_integer_type_representation:internal_exception
+{
+};
+class not_a_bounded_integer_type_overflow:internal_exception
+{
+};
+class not_a_complexity_class:internal_exception
+{
+};
+class not_a_control_parameter_name:internal_exception
+{
+};
+class not_a_control_parameter_value:internal_exception
+{
+};
+class not_a_pip_problem_control_parameter_name:internal_exception
+{
+};
+class not_a_pip_problem_control_parameter_value:internal_exception
+{
+};
+class not_a_relation:internal_exception
+{
+};
+class ppl_handle_mismatch:internal_exception
+{
+};
+class timeout_exception
+{
+~timeout_exception ();
+};
+class deterministic_timeout_exception:timeout_exception
+{
+};
+void __assert_fail (const char *, const char *, int, int *)
+__attribute__ ((__noreturn__));
+void PL_get_pointer (void *);
+int Prolog_is_address ();
+inline int
+Prolog_get_address (void **p1)
+{
+Prolog_is_address ()? static_cast
+void (0) : __assert_fail (Prolog_is_address, ./swi_cfli.hh, 0, 0);
+PL_get_pointer (p1);
+return 0;
+}
+
+class non_linear:internal_exception
+{
+};
+class not_unsigned_integer:internal_exception
+{
+};
+class not_universe_or_empty:internal_exception
+{
+};
+class not_a_nil_terminated_list:internal_exception
+{
+};
+class PPL_integer_out_of_range
+{
+__gmp_expr mpz_t n;
+};
+void handle_exception ();
+template typename T T * term_to_handle
Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
On Thu, Apr 3, 2014 at 2:27 PM, Charles Baylis charles.bay...@linaro.org wrote: Hi This bug causes the compiler to create a Thumb-2 TBB instruction with a jump table containing an out of range value in a .byte field: whatever.s:148: Error: value of 256 too large for field of 1 bytes at 100 This occurs because the jump table is followed with a .align 1 due to ASM_OUTPUT_CASE_END, but the 'shorten' phase does not account for the space taken by this align directive. My first reaction is to wonder why this is this not a bug in the shorten phase. This patch addresses the issue by removing ASM_OUTPUT_CASE_END from arm.h, and ensuring that the alignment after an ADDR_DIFF_VEC is instead inserted by aligning the label following the barrier which follows it. This is achieved by defining LABEL_ALIGN_AFTER_BARRIER appropriately. On first glance this feels like a blunt hammer, what's the code size bloat with putting out such an alignment after each barrier that the compiler emits rather than tracking this in ASM_OUTPUT_CASE_END. I'll try and have a look at this again tomorrow morning. regards Ramana Bootstrapped/checked on arm-unknown-linux-gnueabihf. OK for trunk, and backporting to 4.8? 2014-04-02 Charles Baylis charles.bay...@linaro.org PR target/60609 * config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove. (LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after ADDR_DIFF_VEC. 2014-04-02 Charles Baylis charles.bay...@linaro.org PR target/60609 * g++.dg/torture/pr60609.C: New test.
