[Patch, avr, PR85624] - Fix ICE when initializing 128-byte aligned array
Hi,
The below patch fixes an ICE for the avr target when the setmemhi
expander is involved.
The setmemhi expander generated RTL ends up as an unrecognized insn
if the alignment of the destination exceeds that of a QI
mode const_int (127), AND the number of bytes to set fits in a QI
mode const_int. The second condition prevents *clrmemhi from matching,
and *clrmemqi does not match because it expects operand 3 (the alignment
const_int rtx) to be QI mode, and a value of 128 or greater does not fit.
The patch fixes this by changing the *clrmemqi pattern to match a HI
mode const_int, and also adds a testcase.
Regression test showed no new failures, ok to commit to trunk?
Regards
Senthil
gcc/ChangeLog:
2018-07-18 Senthil Kumar Selvaraj
PR target/85624
* config/avr/avr.md (*clrmemqi): Change mode of operands[2]
from QI to HI.
gcc/testsuite/ChangeLog:
2018-07-18 Senthil Kumar Selvaraj
PR target/85624
* gcc.target/avr/pr85624.c: New test.
diff --git gcc/config/avr/avr.md gcc/config/avr/avr.md
index e619e695418..644e3cfabc5 100644
--- gcc/config/avr/avr.md
+++ gcc/config/avr/avr.md
@@ -1095,7 +1095,7 @@
[(set (mem:BLK (match_operand:HI 0 "register_operand" "e"))
(const_int 0))
(use (match_operand:QI 1 "register_operand" "r"))
- (use (match_operand:QI 2 "const_int_operand" "n"))
+ (use (match_operand:HI 2 "const_int_operand" "n"))
(clobber (match_scratch:HI 3 "=0"))
(clobber (match_scratch:QI 4 "=&1"))]
""
diff --git gcc/testsuite/gcc.target/avr/pr85624.c
gcc/testsuite/gcc.target/avr/pr85624.c
new file mode 100644
index 000..ede2e80216a
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr85624.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+/* This testcase exposes PR85624. An alignment directive with
+ a value greater than 127 on an array with dimensions that fit
+ QImode causes an 'unrecognizable insn' ICE. Turns out clrmemqi
+ did not match the pattern expanded by setmemhi, because it
+ assumed the alignment val will fit in a QI. */
+
+int foo() {
+ volatile int arr[3] __attribute__((aligned(128))) = {0};
+ return arr[2];
+}
[Patch, avr, PR86635] Fix miscompilation with __memx and libgcc float function __gtsf2
The below patch fixes a miscompilation of function calls with__memx address
space
arguments.
For code like
extern const __memx float a;
extern const float b;
int diff () { return a > b; }
when compiled with -Os, a is never loaded and passed in as an argument
to the __gtsf2 libgcc function.
Turns out that mov for the variable a expands into
(insn 8 7 9 2 (parallel [
(set (reg:SF 22 r22)
(mem/u/c:SF (reg/f:PSI 47) [1 a+0 S4 A8 AS7]))
(clobber (reg:SF 22 r22))
(clobber (reg:QI 21 r21))
(clobber (reg:HI 30 r30))
]) "test.c":4 36 {xloadsf_A}
(expr_list:REG_DEAD (reg/f:PSI 47)
(expr_list:REG_UNUSED (reg:HI 30 r30)
(expr_list:REG_EQUAL (mem/u/c:SF (symbol_ref:PSI ("a") [flags
0xe40] ) [1 a+0 S4 A8 AS7])
(nil)
The ud_dce pass sees this insn and deletes it as reg:SF r22 is both set
and clobbered.
Georg-Johann pointed out a similar issue (PR63633), and that was fixed
by introducing a pseudo as the target of set. This patch does the same -
adds an avr_emit2_fix_outputs for gen functions with 2 operands, that
detects hard reg conflicts with clobbered regs and substitutes pseudos
in their place.
The patch also adds a testcase to verify a is actually read. Reg testing
passed. Ok to commit to trunk?
Regards
Senthil
gcc/ChangeLog:
2018-07-25 Senthil Kumar Selvaraj
* config/avr/avr-protos.h (avr_emit2_fix_outputs): New prototype.
* config/avr/avr.c (avr_emit2_fix_outputs): New function.
* config/avr/avr.md (mov): Wrap gen_xload_A call
with avr_emit2_fix_outputs.
gcc/testsuite/ChangeLog:
2018-07-25 Senthil Kumar Selvaraj
* gcc.target/avr/torture/pr86635.c: New test.
diff --git gcc/config/avr/avr-protos.h gcc/config/avr/avr-protos.h
index 5622e9035a0..f8db418582e 100644
--- gcc/config/avr/avr-protos.h
+++ gcc/config/avr/avr-protos.h
@@ -135,6 +135,7 @@ regmask (machine_mode mode, unsigned regno)
}
extern void avr_fix_inputs (rtx*, unsigned, unsigned);
+extern bool avr_emit2_fix_outputs (rtx (*)(rtx,rtx), rtx*, unsigned, unsigned);
extern bool avr_emit3_fix_outputs (rtx (*)(rtx,rtx,rtx), rtx*, unsigned,
unsigned);
extern rtx lpm_reg_rtx;
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 81c35e7fbc2..996d5187c52 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -13335,6 +13335,34 @@ avr_emit3_fix_outputs (rtx (*gen)(rtx,rtx,rtx), rtx
*op,
return avr_move_fixed_operands (op, hreg, opmask);
}
+/* Same as avr_emit3_fix_outputs, but for 2 operands */
+bool
+avr_emit2_fix_outputs (rtx (*gen)(rtx,rtx), rtx *op,
+ unsigned opmask, unsigned rmask)
+{
+ const int n = 2;
+ rtx hreg[n];
+
+ /* It is letigimate for GEN to call this function, and in order not to
+ get self-recursive we use the following static kludge. This is the
+ only way not to duplicate all expanders and to avoid ugly and
+ hard-to-maintain C-code instead of the much more appreciated RTL
+ representation as supplied by define_expand. */
+ static bool lock = false;
+
+ gcc_assert (opmask < (1u << n));
+
+ if (lock)
+return false;
+
+ avr_fix_operands (op, hreg, opmask, rmask);
+
+ lock = true;
+ emit_insn (gen (op[0], op[1]));
+ lock = false;
+
+ return avr_move_fixed_operands (op, hreg, opmask);
+}
/* Worker function for movmemhi expander.
XOP[0] Destination as MEM:BLK
diff --git gcc/config/avr/avr.md gcc/config/avr/avr.md
index e619e695418..5ff4e490a21 100644
--- gcc/config/avr/avr.md
+++ gcc/config/avr/avr.md
@@ -672,7 +672,11 @@
; insn-emit does not depend on the mode, it's all about operands.
*/
emit_insn (gen_xload8qi_A (dest, src));
else
- emit_insn (gen_xload_A (dest, src));
+ if (!avr_emit2_fix_outputs (gen_xload_A, operands, 1 << 0,
+ regmask (mode, 22)
+ | regmask (QImode, 21)
+ | regmask (HImode, REG_Z)))
+FAIL;
DONE;
}
diff --git gcc/testsuite/gcc.target/avr/torture/pr86635.c
gcc/testsuite/gcc.target/avr/torture/pr86635.c
new file mode 100644
index 000..f91367f7e7a
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/torture/pr86635.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! avr_tiny } } } */
+/* { dg-options "-std=gnu99" } */
+
+extern const __memx float a;
+extern const float b;
+
+unsigned long diff () { return a > b; }
+
+/* { dg-final { scan-assembler "call __xload_4" } } */
Re: [Patch, avr, PR86635] Fix miscompilation with __memx and libgcc float function __gtsf2
Senthil Kumar Selvaraj writes:
> The below patch fixes a miscompilation of function calls with__memx address
> space
> arguments.
>
> For code like
>
> extern const __memx float a;
> extern const float b;
>
> int diff () { return a > b; }
>
> when compiled with -Os, a is never loaded and passed in as an argument
> to the __gtsf2 libgcc function.
>
> Turns out that mov for the variable a expands into
>
> (insn 8 7 9 2 (parallel [
> (set (reg:SF 22 r22)
> (mem/u/c:SF (reg/f:PSI 47) [1 a+0 S4 A8 AS7]))
> (clobber (reg:SF 22 r22))
> (clobber (reg:QI 21 r21))
> (clobber (reg:HI 30 r30))
> ]) "test.c":4 36 {xloadsf_A}
> (expr_list:REG_DEAD (reg/f:PSI 47)
> (expr_list:REG_UNUSED (reg:HI 30 r30)
> (expr_list:REG_EQUAL (mem/u/c:SF (symbol_ref:PSI ("a") [flags
> 0xe40] ) [1 a+0 S4 A8 AS7])
> (nil)
>
> The ud_dce pass sees this insn and deletes it as reg:SF r22 is both set
> and clobbered.
>
> Georg-Johann pointed out a similar issue (PR63633), and that was fixed
> by introducing a pseudo as the target of set. This patch does the same -
> adds an avr_emit2_fix_outputs for gen functions with 2 operands, that
> detects hard reg conflicts with clobbered regs and substitutes pseudos
> in their place.
>
> The patch also adds a testcase to verify a is actually read. Reg testing
> passed. Ok to commit to trunk?
Sent an out-of-date patch. Here's the right one.
>
> Regards
> Senthil
>
gcc/ChangeLog:
2018-07-25 Senthil Kumar Selvaraj
* config/avr/avr-protos.h (avr_emit2_fix_outputs): New prototype.
* config/avr/avr.c (avr_emit2_fix_outputs): New function.
* config/avr/avr.md (mov): Wrap gen_xload_A call
with avr_emit2_fix_outputs.
gcc/testsuite/ChangeLog:
2018-07-25 Senthil Kumar Selvaraj
* gcc.target/avr/torture/pr86635.c: New test.
diff --git gcc/config/avr/avr-protos.h gcc/config/avr/avr-protos.h
index 5622e9035a0..f8db418582e 100644
--- gcc/config/avr/avr-protos.h
+++ gcc/config/avr/avr-protos.h
@@ -135,6 +135,7 @@ regmask (machine_mode mode, unsigned regno)
}
extern void avr_fix_inputs (rtx*, unsigned, unsigned);
+extern bool avr_emit2_fix_outputs (rtx (*)(rtx,rtx), rtx*, unsigned, unsigned);
extern bool avr_emit3_fix_outputs (rtx (*)(rtx,rtx,rtx), rtx*, unsigned,
unsigned);
extern rtx lpm_reg_rtx;
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 81c35e7fbc2..996d5187c52 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -13335,6 +13335,34 @@ avr_emit3_fix_outputs (rtx (*gen)(rtx,rtx,rtx), rtx
*op,
return avr_move_fixed_operands (op, hreg, opmask);
}
+/* Same as avr_emit3_fix_outputs, but for 2 operands */
+bool
+avr_emit2_fix_outputs (rtx (*gen)(rtx,rtx), rtx *op,
+ unsigned opmask, unsigned rmask)
+{
+ const int n = 2;
+ rtx hreg[n];
+
+ /* It is letigimate for GEN to call this function, and in order not to
+ get self-recursive we use the following static kludge. This is the
+ only way not to duplicate all expanders and to avoid ugly and
+ hard-to-maintain C-code instead of the much more appreciated RTL
+ representation as supplied by define_expand. */
+ static bool lock = false;
+
+ gcc_assert (opmask < (1u << n));
+
+ if (lock)
+return false;
+
+ avr_fix_operands (op, hreg, opmask, rmask);
+
+ lock = true;
+ emit_insn (gen (op[0], op[1]));
+ lock = false;
+
+ return avr_move_fixed_operands (op, hreg, opmask);
+}
/* Worker function for movmemhi expander.
XOP[0] Destination as MEM:BLK
diff --git gcc/config/avr/avr.md gcc/config/avr/avr.md
index e619e695418..033a428e9f3 100644
--- gcc/config/avr/avr.md
+++ gcc/config/avr/avr.md
@@ -672,7 +672,14 @@
; insn-emit does not depend on the mode, it's all about operands.
*/
emit_insn (gen_xload8qi_A (dest, src));
else
- emit_insn (gen_xload_A (dest, src));
+ {
+operands[0] = dest; operands[1] = src;
+if (!avr_emit2_fix_outputs (gen_xload_A, operands, 1 << 0,
+ regmask (mode, 22)
+ | regmask (QImode, 21)
+ | regmask (HImode, REG_Z)))
+ FAIL;
+ }
DONE;
}
diff --git gcc/testsuite/gcc.target/avr/torture/pr86635.c
gcc/testsuite/gcc.target/avr/torture/pr86635.c
new file mode 100644
index 000..f91367f7e7a
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/torture/pr86635.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! avr_tiny } } } */
+/* { dg-options "-std=gnu99" } */
+
+extern const __memx float a;
+extern const float b;
+
+unsigned long diff () { return a > b; }
+
+/* { dg-final { scan-assembler "call __xload_4" } } */
Re: [Patch] Fix infinite loop/crash if array initializer index equals max value
On Fri, Aug 23, 2013 at 09:49:55PM +, Joseph S. Myers wrote:
> On Thu, 22 Aug 2013, Selvaraj, Senthil_Kumar wrote:
>
> > 2013-08-23 Senthil Kumar Selvaraj
> > * c-typeck.c (output_pending_init_elements): Handle overflow of
> > constructor_unfilled_index.
>
> This patch needs to add include a testcase to the testsuite that fails
> before and passes after the patch. (I realise such a test may only be
> able to run for a subset of targets.)
Reattaching the patch with a testcase for the AVR target. I'm not sure
how to generalize the testcase for other targets - the constant is the
max value (unsigned) of the mode used to represent initialized array
indices.
The attached test fails with a timeout before applying the patch, and
passes after applying it.
Regards
Senthil
gcc/c/ChangeLog
2013-09-04 Senthil Kumar Selvaraj
* c-typeck.c (output_pending_init_elements): Handle overflow of
constructor_unfilled_index.
gcc/testsuite/ChangeLog
2013-09-04 Senthil Kumar Selvaraj
* gcc.dg/large-size-array-7.c: New test to verify overflow handling
of constructor_unfilled_index.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 30871db..ed2e37a 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -7953,8 +7953,9 @@ output_pending_init_elements (int all, struct obstack *
braced_init_obstack)
TREE_TYPE (constructor_type),
constructor_unfilled_index, 0, false,
braced_init_obstack);
- else if (tree_int_cst_lt (constructor_unfilled_index,
- elt->purpose))
+ else if (!TREE_OVERFLOW_P (constructor_unfilled_index)
+&& tree_int_cst_lt (constructor_unfilled_index,
+ elt->purpose))
{
/* Advance to the next smaller node. */
if (elt->left)
@@ -7979,7 +7980,8 @@ output_pending_init_elements (int all, struct obstack *
braced_init_obstack)
while (elt->parent && elt->parent->right == elt)
elt = elt->parent;
elt = elt->parent;
- if (elt && tree_int_cst_lt (constructor_unfilled_index,
+ if (elt && !TREE_OVERFLOW_P (constructor_unfilled_index)
+ && tree_int_cst_lt (constructor_unfilled_index,
elt->purpose))
{
next = elt->purpose;
diff --git gcc/testsuite/gcc.dg/large-size-array-7.c
gcc/testsuite/gcc.dg/large-size-array-7.c
new file mode 100644
index 000..196767d
--- /dev/null
+++ gcc/testsuite/gcc.dg/large-size-array-7.c
@@ -0,0 +1,5 @@
+/* { dg-do compile { target "avr-*-*" } } */
+/* { dg-options "-O2" } */
+static char * name[] = {
+[0xFF] = "bar"
+ }; /* { dg-error "too large" } */
[Patch, avr] Emit diagnostics if -f{pic,PIC,pie,PIE} or -shared is passed
The AVR backend does not generate position independent code, yet it
happily accepts -fpic, -fPIC, -fpie and -fPIE. The generated code
doesn't change at all. Also, it accepts the -shared option to generate a
shared library, without really doing anything with it.
This causes one of the regression tests
(gcc.dg/lto/pr54709 c_lto_pr54709_0.o-c_lto_pr54709_1.o link) to fail with
an 'undefined reference to main' error, when the test is trying to build
a shared object.
The attached patch generates a warning if one of the -f{pic,PIC,pie,PIE}
options is provided, and an error if -shared is provided (
config/mep/mep.c and config/s390/tpf.h already do something very similar).
Regression tested with no new failures.Tests which exercise PIC now report as
unsupported.
If ok, could someone commit please? I don't have commit access.
Regards
Senthil
gcc/ChangeLog
2013-11-04 Senthil Kumar Selvaraj
* config/avr/avr.c (avr_option_override): Warn if asked to generate
position independent code.
* config/avr/avr.h: Modify LINK_SPEC to reject -shared.
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index e7e1c2f..cf4b8ca 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -310,6 +310,15 @@ avr_option_override (void)
flag_omit_frame_pointer = 0;
}
+ if (flag_pic == 1)
+warning (OPT_fpic, "-fpic is not supported");
+ if (flag_pic == 2)
+warning (OPT_fPIC, "-fPIC is not supported");
+ if (flag_pie == 1)
+warning (OPT_fpie, "-fpie is not supported");
+ if (flag_pie == 2)
+warning (OPT_fPIE, "-fPIE is not supported");
+
avr_current_device = &avr_mcu_types[avr_mcu_index];
avr_current_arch = &avr_arch_types[avr_current_device->arch];
diff --git gcc/config/avr/avr.h gcc/config/avr/avr.h
index f223a61..1eff5be 100644
--- gcc/config/avr/avr.h
+++ gcc/config/avr/avr.h
@@ -522,7 +522,8 @@ extern const char *avr_device_to_sp8 (int argc, const char
**argv);
mmcu=at90can64*|\
mmcu=at90usb64*:--pmem-wrap-around=64k}}}\
%:device_to_ld(%{mmcu=*:%*})\
-%:device_to_data_start(%{mmcu=*:%*})"
+%:device_to_data_start(%{mmcu=*:%*})\
+%{shared:%eshared is not supported}"
#define LIB_SPEC \
"%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28:
-lc }"
[Patch, testsuite] Require scheduling support in gcc.dg/superblock.c
Hi,
gcc.dg/superblock.c uses the -fdump-rtl-sched2 option and then scans the
resulting dump. This fails for the AVR target, as it doesn't have any
instruction scheduling support.
The attached patch makes this test require scheduling support in the
target. If ok, could someone commit please? I do not have commit access.
Regards
Senthil
gcc/testsuite
2013-11-04 Senthil Kumar Selvaraj
* gcc.dg/superblock.c: Require scheduling support
diff --git gcc/testsuite/gcc.dg/superblock.c gcc/testsuite/gcc.dg/superblock.c
index 2b9aedf..272d161 100644
--- gcc/testsuite/gcc.dg/superblock.c
+++ gcc/testsuite/gcc.dg/superblock.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fno-asynchronous-unwind-tables -fsched2-use-superblocks
-fdump-rtl-sched2 -fdump-rtl-bbro" } */
+/* { dg-require-effective-target scheduling } */
typedef int aligned __attribute__ ((aligned (64)));
extern void abort (void);
[Patch, avr] Restore default value of PARAM_ALLOW_STORE_DATA_RACES to 1
Hi,
This patch sets PARAM_ALLOW_STORE_DATA_RACES to 1 (the default until
a year back), to avoid code size regressions in trunk (and probably
5.x )for the AVR target.
Consider the following piece of code
volatile int z;
void foo(int x)
{
static char i;
for (i=0; i< 4; ++i)
{
if (x > 2)
z = 1;
else
z = 2;
}
}
Unmodified gcc trunk generates this
movw r20,r24
sts i.1495,__zero_reg__
ldi r25,0
ldi r18,0
ldi r22,lo8(2)
ldi r23,0
ldi r30,lo8(1)
ldi r31,0
.L2:
cpi r25,lo8(4)
brne .L5
cpse r18,__zero_reg__
sts i.1495,r25
.L1:
ret
.L5:
cpi r20,3
cpc r21,__zero_reg__
brlt .L3
sts z+1,r31
sts z,r30
.L4:
subi r25,lo8(-(1))
ldi r18,lo8(1)
rjmp .L2
.L3:
sts z+1,r23
sts z,r22
rjmp .L4
.size foo, .-foo
.local i.1495
.comm i.1495,1,1
.comm z,2,1
.ident "GCC: (GNU) 6.0.0 20160201 (experimental)"
Note the usage of an extra reg (r18) that is used as a flag to
record loop entry (in .L4), and the conditional store of r25 to i in .L2.
In 4.x, there is no extra reg usage - only a single unconditional set of
i to r25 at the end of the loop.
Digging into the code, I found that LIM checks
PARAM_ALLOW_STORE_DATA_RACES and introduces the flag to avoid store data
races - see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52558. The
default value of the param was set to zero a year and a half back - see
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01548.html.
For AVR, I guess assuming any store can cause a data race is too
pessimistic for the general case. Globals shared with interrupts will
need special handling for atomic access anyway, so I thought we should
revert the default back to allow store data races.
If this is ok, could someone commit please? I don't have commit access.
Regards
Senthil
gcc/ChangeLog
2016-02-01 Senthil Kumar Selvaraj
* config/avr/avr.c (avr_option_override): Set
PARAM_ALLOW_STORE_DATA_RACES to 1.
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index e557772..a7728e3 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -43,6 +43,7 @@
#include "expr.h"
#include "langhooks.h"
#include "cfgrtl.h"
+#include "params.h"
#include "builtins.h"
#include "context.h"
#include "tree-pass.h"
@@ -410,6 +411,15 @@ avr_option_override (void)
if (avr_strict_X)
flag_caller_saves = 0;
+ /* Allow optimizer to introduce store data races. This used to be the
+ default - it was changed because bigger targets did not see any
+ performance decrease. For the AVR though, disallowing data races
+ introduces additional code in LIM and increases reg pressure. */
+
+ maybe_set_param_value (PARAM_ALLOW_STORE_DATA_RACES, 1,
+ global_options.x_param_values,
+ global_options_set.x_param_values);
+
/* Unwind tables currently require a frame pointer for correctness,
see toplev.c:process_options(). */
[Patch, testsuite] Require int32 target support in sso tests
Hi,
When running the regression testsuite for the AVR target, I noticed a
bunch of sso tests failing - turns out they assume sizeof(int) == 4.
This patch marks them UNSUPPORTED for targets that don't match that
assumption, by requiring effective-target int32.
A few tests still run and fail because they expect IO support to be
available on test execution - is there an effective-target option
to specify lack of IO? I checked lib/target-supports.exp, and the
closest I could find was check_effective_target_hw, but it's possible
some simulators support printing to the host machine.
If this patch is ok, could someone commit please? I don't have commit
access.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-02-04 Senthil Kumar Selvaraj
* gcc/testsuite/gcc.dg/sso/p1.c: Add dg-require-effective-target int32.
* gcc/testsuite/gcc.dg/sso/p2.c: Likewise.
* gcc/testsuite/gcc.dg/sso/p3.c: Likewise.
* gcc/testsuite/gcc.dg/sso/p5.c: Likewise.
* gcc/testsuite/gcc.dg/sso/p6.c: Likewise.
* gcc/testsuite/gcc.dg/sso/p7.c: Likewise.
* gcc/testsuite/gcc.dg/sso/p8.c: Likewise.
* gcc/testsuite/gcc.dg/sso/q1.c: Likewise.
* gcc/testsuite/gcc.dg/sso/q2.c: Likewise.
* gcc/testsuite/gcc.dg/sso/q3.c: Likewise.
* gcc/testsuite/gcc.dg/sso/q5.c: Likewise.
* gcc/testsuite/gcc.dg/sso/q6.c: Likewise.
* gcc/testsuite/gcc.dg/sso/q7.c: Likewise.
* gcc/testsuite/gcc.dg/sso/q8.c: Likewise.
* gcc/testsuite/gcc.dg/sso/r3.c: Likewise.
* gcc/testsuite/gcc.dg/sso/r5.c: Likewise.
* gcc/testsuite/gcc.dg/sso/r6.c: Likewise.
* gcc/testsuite/gcc.dg/sso/r7.c: Likewise.
* gcc/testsuite/gcc.dg/sso/r8.c: Likewise.
* gcc/testsuite/gcc.dg/sso/s3.c: Likewise.
* gcc/testsuite/gcc.dg/sso/s5.c: Likewise.
* gcc/testsuite/gcc.dg/sso/s6.c: Likewise.
* gcc/testsuite/gcc.dg/sso/s7.c: Likewise.
* gcc/testsuite/gcc.dg/sso/s8.c: Likewise.
* gcc/testsuite/gcc.dg/sso/t1.c: Likewise.
* gcc/testsuite/gcc.dg/sso/t2.c: Likewise.
* gcc/testsuite/gcc.dg/sso/t3.c: Likewise.
* gcc/testsuite/gcc.dg/sso/t5.c: Likewise.
* gcc/testsuite/gcc.dg/sso/t6.c: Likewise.
* gcc/testsuite/gcc.dg/sso/t7.c: Likewise.
* gcc/testsuite/gcc.dg/sso/t8.c: Likewise.
* gcc/testsuite/gcc.dg/sso/u5.c: Likewise.
* gcc/testsuite/gcc.dg/sso/u6.c: Likewise.
diff --git gcc/testsuite/gcc.dg/sso/p1.c gcc/testsuite/gcc.dg/sso/p1.c
index bf14ffe..ca2f363 100644
--- gcc/testsuite/gcc.dg/sso/p1.c
+++ gcc/testsuite/gcc.dg/sso/p1.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/p2.c gcc/testsuite/gcc.dg/sso/p2.c
index 2d92946..906d60b 100644
--- gcc/testsuite/gcc.dg/sso/p2.c
+++ gcc/testsuite/gcc.dg/sso/p2.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/p3.c gcc/testsuite/gcc.dg/sso/p3.c
index fb71596..2ee8ab0 100644
--- gcc/testsuite/gcc.dg/sso/p3.c
+++ gcc/testsuite/gcc.dg/sso/p3.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/p5.c gcc/testsuite/gcc.dg/sso/p5.c
index fc8b7e1..b13dc0a 100644
--- gcc/testsuite/gcc.dg/sso/p5.c
+++ gcc/testsuite/gcc.dg/sso/p5.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/p6.c gcc/testsuite/gcc.dg/sso/p6.c
index 90805a7..94b5f30 100644
--- gcc/testsuite/gcc.dg/sso/p6.c
+++ gcc/testsuite/gcc.dg/sso/p6.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/p7.c gcc/testsuite/gcc.dg/sso/p7.c
index e347ecf..afc3167 100644
--- gcc/testsuite/gcc.dg/sso/p7.c
+++ gcc/testsuite/gcc.dg/sso/p7.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/p8.c gcc/testsuite/gcc.dg/sso/p8.c
index d66c7da..8c1ed2c 100644
--- gcc/testsuite/gcc.dg/sso/p8.c
+++ gcc/testsuite/gcc.dg/sso/p8.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/q1.c gcc/testsuite/gcc.dg/sso/q1.c
index f61398b..08efa63 100644
--- gcc/testsuite/gcc.dg/sso/q1.c
+++ gcc/testsuite/gcc.dg/sso/q1.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/q2.c gcc/testsuite/gcc.dg/sso/q2.c
index 35e2839..9e2de92 100644
--- gcc/testsuite/gcc.dg/sso/q2.c
+++ gcc/testsuite/gcc.dg/sso/q2.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/q3.c gcc/testsuite/gcc.dg/sso/q3.c
index 31d395a..c4fd1d0 100644
--- gcc/testsuite/gcc.dg/sso/q3.c
+++ gcc/testsuite/gcc.dg/sso/q3.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32 } */
#include
diff --git gcc/testsuite/gcc.dg/sso/q5.
Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer
Hurugalawadi, Naveen writes: > Hi, > > Please find attached the patch that performs optimization on unsigned values. > > Original fold-const part implemented in match.pd. > > Please review the patch and let us know if it's OK? > > Regression Tested on X86_64 with no regressions. > > Thanks, > Naveen > > ChangeLog: > * match.pd (cmp (convert (bit_not @0)) INTEGER_CST@1): New Simplifier. > > Testsuite/ChangeLog: > * gcc.dg/pr31531.c : New testcase. Could you please add dg-require-effective-target int32 to the test to avoid breaking targets with smaller ints (right shift by 31 bits)? Regards Senthil
Re: [Patch, testsuite] Require int32 target support in sso tests
Mike Stump okayed the patch
(https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00321.html).
Could someone commit it for me please? I don't have commit access.
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Hi,
>
> When running the regression testsuite for the AVR target, I noticed a
> bunch of sso tests failing - turns out they assume sizeof(int) == 4.
> This patch marks them UNSUPPORTED for targets that don't match that
> assumption, by requiring effective-target int32.
>
> A few tests still run and fail because they expect IO support to be
> available on test execution - is there an effective-target option
> to specify lack of IO? I checked lib/target-supports.exp, and the
> closest I could find was check_effective_target_hw, but it's possible
> some simulators support printing to the host machine.
>
> If this patch is ok, could someone commit please? I don't have commit
> access.
>
> Regards
> Senthil
>
> gcc/testsuite/ChangeLog
>
> 2016-02-04 Senthil Kumar Selvaraj
>
> * gcc/testsuite/gcc.dg/sso/p1.c: Add dg-require-effective-target int32.
> * gcc/testsuite/gcc.dg/sso/p2.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/p3.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/p5.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/p6.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/p7.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/p8.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/q1.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/q2.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/q3.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/q5.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/q6.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/q7.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/q8.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/r3.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/r5.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/r6.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/r7.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/r8.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/s3.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/s5.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/s6.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/s7.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/s8.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/t1.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/t2.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/t3.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/t5.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/t6.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/t7.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/t8.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/u5.c: Likewise.
> * gcc/testsuite/gcc.dg/sso/u6.c: Likewise.
>
> diff --git gcc/testsuite/gcc.dg/sso/p1.c gcc/testsuite/gcc.dg/sso/p1.c
> index bf14ffe..ca2f363 100644
> --- gcc/testsuite/gcc.dg/sso/p1.c
> +++ gcc/testsuite/gcc.dg/sso/p1.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32 } */
>
> #include
>
> diff --git gcc/testsuite/gcc.dg/sso/p2.c gcc/testsuite/gcc.dg/sso/p2.c
> index 2d92946..906d60b 100644
> --- gcc/testsuite/gcc.dg/sso/p2.c
> +++ gcc/testsuite/gcc.dg/sso/p2.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32 } */
>
> #include
>
> diff --git gcc/testsuite/gcc.dg/sso/p3.c gcc/testsuite/gcc.dg/sso/p3.c
> index fb71596..2ee8ab0 100644
> --- gcc/testsuite/gcc.dg/sso/p3.c
> +++ gcc/testsuite/gcc.dg/sso/p3.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32 } */
>
> #include
>
> diff --git gcc/testsuite/gcc.dg/sso/p5.c gcc/testsuite/gcc.dg/sso/p5.c
> index fc8b7e1..b13dc0a 100644
> --- gcc/testsuite/gcc.dg/sso/p5.c
> +++ gcc/testsuite/gcc.dg/sso/p5.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32 } */
>
> #include
>
> diff --git gcc/testsuite/gcc.dg/sso/p6.c gcc/testsuite/gcc.dg/sso/p6.c
> index 90805a7..94b5f30 100644
> --- gcc/testsuite/gcc.dg/sso/p6.c
> +++ gcc/testsuite/gcc.dg/sso/p6.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32 } */
>
> #include
>
> diff --git gcc/testsuite/gcc.dg/sso/p7.c gcc/testsuite/gcc.dg/sso/p7.c
> index e347ecf..afc3167 100644
> --- gcc/testsuite/gcc.dg/sso/p7.c
> +++ gcc/testsuite/gcc.dg/sso/p7.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32 } */
>
> #include
>
> diff --git gcc/testsuite/gcc.dg/sso/p8.c gcc/testsuite/gcc.dg/sso/p8.c
> index d66c7da..8c1ed2c 100644
> --- gcc/testsuite/gcc.dg/sso/p8.c
> +++ gcc/testsuite/gcc.dg/sso/p8.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32 } */
>
> #include
>
Re: [Patch, testsuite] Require int32 target support in sso tests
Mike Stump writes:
> On Feb 22, 2016, at 12:41 AM, Senthil Kumar Selvaraj
> wrote:
>> Could someone commit it for me please? I don't have commit access.
>
> Could you test out:
>
> Index: sso.exp
> ===
> --- sso.exp (revision 233611)
> +++ sso.exp (working copy)
> @@ -32,6 +32,10 @@ set SSO_TORTURE_OPTIONS [list \
>
> set-torture-options $SSO_TORTURE_OPTIONS
>
> +if { ![check_effective_target_int32] } {
> +return
> +}
> +
> # Main loop.
> gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.c]] "" “”
>
> and let us know if it works for you. I think this should address Eric’s
> concerns and yours.
Yes that works - just that it also skips a few testcases that don't
depend on integers being 32 bits (see ). Doesn't matter much for AVR until we
get I/O working on the simulator though.
Regards
Senthil
[Patch, testsuite] Skip addr_equal-1 if target keeps null pointer checks
Hi,
The below patch skips gcc.dg/addr_equal-1.c if the target keeps null
pointer checks.
The test fails for such targets (avr, in my case) because the address
comparison in the below code does not resolve to a constant, causing
builtin_constant_p to return false and fail the test.
/* Variables and functions do not share same memory locations otherwise. */
if (!__builtin_constant_p ((void *)undef_fn0 == (void *)&undef_var0))
abort ();
For targets that delete null pointer checks, the equality comparison
expression
is optimized away to 0, as the code in match.pd knows they can only be
equal if they are both NULL, which cannot be true since
flag-delete-null-pointer-checks is on.
For targets that keep null pointer checks, 0 is a valid address and the
comparison expression is left as is, and that causes a later pass to
fold the builtin_constant_p to a false value, resulting in the test
failure.
If this is ok, could someone commit please? I don't have commit
access.
Regards
Senthil
gcc/testsuite/ChangeLog
2015-09-28 Senthil Kumar Selvaraj
* gcc.dg/addr_equal-1.c: Skip test if target keeps
null pointer checks.
diff --git gcc/testsuite/gcc.dg/addr_equal-1.c
gcc/testsuite/gcc.dg/addr_equal-1.c
index 94499f0..957b03a 100644
--- gcc/testsuite/gcc.dg/addr_equal-1.c
+++ gcc/testsuite/gcc.dg/addr_equal-1.c
@@ -3,6 +3,7 @@
/* { dg-require-weak "" } */
/* { dg-require-alias "" } */
/* { dg-options "-O2" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
void abort (void);
extern int undef_var0, undef_var1;
extern __attribute__ ((weak)) int weak_undef_var0;
Re: [Patch, testsuite] Skip addr_equal-1 if target keeps null pointer checks
On Mon, Sep 28, 2015 at 01:38:18PM -0600, Jeff Law wrote: > On 09/28/2015 02:15 AM, Senthil Kumar Selvaraj wrote: > >Hi, > > > > The below patch skips gcc.dg/addr_equal-1.c if the target keeps null > > pointer checks. > > > > The test fails for such targets (avr, in my case) because the address > > comparison in the below code does not resolve to a constant, causing > > builtin_constant_p to return false and fail the test. > > > > /* Variables and functions do not share same memory locations otherwise. > > */ > > if (!__builtin_constant_p ((void *)undef_fn0 == (void *)&undef_var0)) > > abort (); > > > > For targets that delete null pointer checks, the equality comparison > > expression > > is optimized away to 0, as the code in match.pd knows they can only be > > equal if they are both NULL, which cannot be true since > > flag-delete-null-pointer-checks is on. > > > > For targets that keep null pointer checks, 0 is a valid address and the > > comparison expression is left as is, and that causes a later pass to > > fold the builtin_constant_p to a false value, resulting in the test > > failure. > This sounds like a failing in the compiler itself, not a testsuite issue. > > Even on a target where objects can be at address 0, you can't have a > variable and a function at the same address. Hmm, symtab_node::equal_address_to, which is where the address equality check happens, has a comment that contradicts your statement, and the function variable overlap check is done after the NULL possibility check. The current code looks like this /* If both symbols may resolve to NULL, we can not really prove them different. */ if (!nonzero_address () && !s2->nonzero_address ()) return 2; /* Except for NULL, functions and variables never overlap. */ if (TREE_CODE (decl) != TREE_CODE (s2->decl)) return 0; Does anyone know why? Regards Senthil
[Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses
Hi,
As part of support for io and io_low attributes, the upper bound of
the range check for low IO and IO addresses was changed from hardcoded
values to hardcoded_range_end + 1 - GET_MODE_SIZE(mode).
GCC passes VOID as the mode from genrecog, and GET_MODE_SIZE returns
0, resulting in the range getting incorrectly extended by a byte.
Not sure why it was done, as the mode of the operand shouldn't really
matter when computing the upper bound. In any case, the insns that use
the predicate already have a mem:QI wrapping it, and all the bit
addressable instructions operate on a single IO register only.
This patch reverts the check back to a hardcoded value, and adds a
test to prevent regressions.
No new regression failures. If ok, could someone commit please? I
don't have commit access.
Regards
Senthil
gcc/ChangeLog
2015-10-05 Senthil Kumar Selvaraj
PR target/67839
* config/avr/predicates.md (low_io_address_operand): Don't
consider MODE when computing upper bound.
(io_address_operand): Likewise.
gcc/testsuite/ChangeLog
2015-10-05 Senthil Kumar Selvaraj
PR target/67839
* gcc.target/avr/pr67839.c: New test.
diff --git gcc/config/avr/predicates.md gcc/config/avr/predicates.md
index 2d12bc6..622bc0b 100644
--- gcc/config/avr/predicates.md
+++ gcc/config/avr/predicates.md
@@ -46,7 +46,7 @@
(define_special_predicate "low_io_address_operand"
(ior (and (match_code "const_int")
(match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
- 0, 0x20 - GET_MODE_SIZE (mode))"))
+ 0, 0x1F)"))
(and (match_code "symbol_ref")
(match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO_LOW"
@@ -60,7 +60,7 @@
(define_special_predicate "io_address_operand"
(ior (and (match_code "const_int")
(match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
- 0, 0x40 - GET_MODE_SIZE (mode))"))
+ 0, 0x3F)"))
(and (match_code "symbol_ref")
(match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO"
diff --git gcc/testsuite/gcc.target/avr/pr67839.c
gcc/testsuite/gcc.target/avr/pr67839.c
new file mode 100644
index 000..604ab4b
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr67839.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+/* { dg-final { scan-assembler "sbi 0x1f,0" } } */
+/* { dg-final { scan-assembler "cbi 0x1f,0" } } */
+/* { dg-final { scan-assembler-not "sbi 0x20,0" } } */
+/* { dg-final { scan-assembler-not "cbi 0x20,0" } } */
+/* { dg-final { scan-assembler "in r\\d+,__SREG__" } } */
+/* { dg-final { scan-assembler "out __SREG__,r\\d+" } } */
+/* { dg-final { scan-assembler-not "in r\\d+,0x40" } } */
+/* { dg-final { scan-assembler-not "out 0x40, r\\d+" } } */
+
+/* This testcase verifies that SBI/CBI/SBIS/SBIC
+ and IN/OUT instructions are not generated for
+ an IO addresses outside the valid range.
+*/
+#define IO_ADDR(x) (*((volatile char *)x + __AVR_SFR_OFFSET__))
+int main ()
+{
+ IO_ADDR(0x1f) |= 1;
+ IO_ADDR(0x1f) &= 0xFE;
+
+ IO_ADDR(0x20) |= 1;
+ IO_ADDR(0x20) &= 0xFE;
+
+ IO_ADDR(0x3f) = IO_ADDR(0x3f);
+
+ IO_ADDR(0x40) = IO_ADDR(0x40);
+ return 0;
+}
Re: [Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses
Ping!
Regards
Senthil
On Mon, Oct 05, 2015 at 02:30:58PM +0530, Senthil Kumar Selvaraj wrote:
> Hi,
>
> As part of support for io and io_low attributes, the upper bound of
> the range check for low IO and IO addresses was changed from hardcoded
> values to hardcoded_range_end + 1 - GET_MODE_SIZE(mode).
>
> GCC passes VOID as the mode from genrecog, and GET_MODE_SIZE returns
> 0, resulting in the range getting incorrectly extended by a byte.
>
> Not sure why it was done, as the mode of the operand shouldn't really
> matter when computing the upper bound. In any case, the insns that use
> the predicate already have a mem:QI wrapping it, and all the bit
> addressable instructions operate on a single IO register only.
>
> This patch reverts the check back to a hardcoded value, and adds a
> test to prevent regressions.
>
> No new regression failures. If ok, could someone commit please? I
> don't have commit access.
>
>
> Regards
> Senthil
>
> gcc/ChangeLog
>
> 2015-10-05 Senthil Kumar Selvaraj
>
> PR target/67839
> * config/avr/predicates.md (low_io_address_operand): Don't
> consider MODE when computing upper bound.
> (io_address_operand): Likewise.
>
>
> gcc/testsuite/ChangeLog
>
> 2015-10-05 Senthil Kumar Selvaraj
>
> PR target/67839
> * gcc.target/avr/pr67839.c: New test.
>
>
>
> diff --git gcc/config/avr/predicates.md gcc/config/avr/predicates.md
> index 2d12bc6..622bc0b 100644
> --- gcc/config/avr/predicates.md
> +++ gcc/config/avr/predicates.md
> @@ -46,7 +46,7 @@
> (define_special_predicate "low_io_address_operand"
>(ior (and (match_code "const_int")
> (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> -0, 0x20 - GET_MODE_SIZE (mode))"))
> +0, 0x1F)"))
> (and (match_code "symbol_ref")
> (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO_LOW"
>
> @@ -60,7 +60,7 @@
> (define_special_predicate "io_address_operand"
>(ior (and (match_code "const_int")
> (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> -0, 0x40 - GET_MODE_SIZE (mode))"))
> +0, 0x3F)"))
> (and (match_code "symbol_ref")
> (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO"
>
> diff --git gcc/testsuite/gcc.target/avr/pr67839.c
> gcc/testsuite/gcc.target/avr/pr67839.c
> new file mode 100644
> index 000..604ab4b
> --- /dev/null
> +++ gcc/testsuite/gcc.target/avr/pr67839.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Os" } */
> +/* { dg-final { scan-assembler "sbi 0x1f,0" } } */
> +/* { dg-final { scan-assembler "cbi 0x1f,0" } } */
> +/* { dg-final { scan-assembler-not "sbi 0x20,0" } } */
> +/* { dg-final { scan-assembler-not "cbi 0x20,0" } } */
> +/* { dg-final { scan-assembler "in r\\d+,__SREG__" } } */
> +/* { dg-final { scan-assembler "out __SREG__,r\\d+" } } */
> +/* { dg-final { scan-assembler-not "in r\\d+,0x40" } } */
> +/* { dg-final { scan-assembler-not "out 0x40, r\\d+" } } */
> +
> +/* This testcase verifies that SBI/CBI/SBIS/SBIC
> + and IN/OUT instructions are not generated for
> + an IO addresses outside the valid range.
> +*/
> +#define IO_ADDR(x) (*((volatile char *)x + __AVR_SFR_OFFSET__))
> +int main ()
> +{
> + IO_ADDR(0x1f) |= 1;
> + IO_ADDR(0x1f) &= 0xFE;
> +
> + IO_ADDR(0x20) |= 1;
> + IO_ADDR(0x20) &= 0xFE;
> +
> + IO_ADDR(0x3f) = IO_ADDR(0x3f);
> +
> + IO_ADDR(0x40) = IO_ADDR(0x40);
> + return 0;
> +}
Re: [Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses
Ping!
Regards
Senthil
On Fri, Oct 16, 2015 at 03:17:17PM +0530, Senthil Kumar Selvaraj wrote:
> Ping!
>
> Regards
> Senthil
>
> On Mon, Oct 05, 2015 at 02:30:58PM +0530, Senthil Kumar Selvaraj wrote:
> > Hi,
> >
> > As part of support for io and io_low attributes, the upper bound of
> > the range check for low IO and IO addresses was changed from hardcoded
> > values to hardcoded_range_end + 1 - GET_MODE_SIZE(mode).
> >
> > GCC passes VOID as the mode from genrecog, and GET_MODE_SIZE returns
> > 0, resulting in the range getting incorrectly extended by a byte.
> >
> > Not sure why it was done, as the mode of the operand shouldn't really
> > matter when computing the upper bound. In any case, the insns that use
> > the predicate already have a mem:QI wrapping it, and all the bit
> > addressable instructions operate on a single IO register only.
> >
> > This patch reverts the check back to a hardcoded value, and adds a
> > test to prevent regressions.
> >
> > No new regression failures. If ok, could someone commit please? I
> > don't have commit access.
> >
> >
> > Regards
> > Senthil
> >
> > gcc/ChangeLog
> >
> > 2015-10-05 Senthil Kumar Selvaraj
> >
> > PR target/67839
> > * config/avr/predicates.md (low_io_address_operand): Don't
> > consider MODE when computing upper bound.
> > (io_address_operand): Likewise.
> >
> >
> > gcc/testsuite/ChangeLog
> >
> > 2015-10-05 Senthil Kumar Selvaraj
> >
> > PR target/67839
> > * gcc.target/avr/pr67839.c: New test.
> >
> >
> >
> > diff --git gcc/config/avr/predicates.md gcc/config/avr/predicates.md
> > index 2d12bc6..622bc0b 100644
> > --- gcc/config/avr/predicates.md
> > +++ gcc/config/avr/predicates.md
> > @@ -46,7 +46,7 @@
> > (define_special_predicate "low_io_address_operand"
> >(ior (and (match_code "const_int")
> > (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> > - 0, 0x20 - GET_MODE_SIZE (mode))"))
> > + 0, 0x1F)"))
> > (and (match_code "symbol_ref")
> > (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO_LOW"
> >
> > @@ -60,7 +60,7 @@
> > (define_special_predicate "io_address_operand"
> >(ior (and (match_code "const_int")
> > (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> > - 0, 0x40 - GET_MODE_SIZE (mode))"))
> > + 0, 0x3F)"))
> > (and (match_code "symbol_ref")
> > (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO"
> >
> > diff --git gcc/testsuite/gcc.target/avr/pr67839.c
> > gcc/testsuite/gcc.target/avr/pr67839.c
> > new file mode 100644
> > index 000..604ab4b
> > --- /dev/null
> > +++ gcc/testsuite/gcc.target/avr/pr67839.c
> > @@ -0,0 +1,29 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-Os" } */
> > +/* { dg-final { scan-assembler "sbi 0x1f,0" } } */
> > +/* { dg-final { scan-assembler "cbi 0x1f,0" } } */
> > +/* { dg-final { scan-assembler-not "sbi 0x20,0" } } */
> > +/* { dg-final { scan-assembler-not "cbi 0x20,0" } } */
> > +/* { dg-final { scan-assembler "in r\\d+,__SREG__" } } */
> > +/* { dg-final { scan-assembler "out __SREG__,r\\d+" } } */
> > +/* { dg-final { scan-assembler-not "in r\\d+,0x40" } } */
> > +/* { dg-final { scan-assembler-not "out 0x40, r\\d+" } } */
> > +
> > +/* This testcase verifies that SBI/CBI/SBIS/SBIC
> > + and IN/OUT instructions are not generated for
> > + an IO addresses outside the valid range.
> > +*/
> > +#define IO_ADDR(x) (*((volatile char *)x + __AVR_SFR_OFFSET__))
> > +int main ()
> > +{
> > + IO_ADDR(0x1f) |= 1;
> > + IO_ADDR(0x1f) &= 0xFE;
> > +
> > + IO_ADDR(0x20) |= 1;
> > + IO_ADDR(0x20) &= 0xFE;
> > +
> > + IO_ADDR(0x3f) = IO_ADDR(0x3f);
> > +
> > + IO_ADDR(0x40) = IO_ADDR(0x40);
> > + return 0;
> > +}
[Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode
This patch came out of a discussion held in the gcc mailing list
(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html).
The patch restricts folding of conditional exprs with lhs previously
set by a type conversion to occur only if the source of the type
conversion's mode is word mode or smaller.
Bootstrapped and reg tested on x86_64 (with --enable-languages=c,c++).
If ok, could you commit please? I don't have commit access.
Regards
Senthil
gcc/ChangeLog
2015-11-11 Senthil Kumar Selvaraj
* tree-vrp.c (simplify_cond_using_ranges): Fold only
if innerop's mode is word_mode or smaller.
diff --git gcc/tree-vrp.c gcc/tree-vrp.c
index e2393e4..c139bc6 100644
--- gcc/tree-vrp.c
+++ gcc/tree-vrp.c
@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt)
innerop = gimple_assign_rhs1 (def_stmt);
if (TREE_CODE (innerop) == SSA_NAME
+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop)))
+ <= GET_MODE_SIZE(word_mode))
&& !POINTER_TYPE_P (TREE_TYPE (innerop)))
{
value_range *vr = get_value_range (innerop);
Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode
On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote:
> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
>
> >This patch came out of a discussion held in the gcc mailing list
> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html).
> >
> >The patch restricts folding of conditional exprs with lhs previously
> >set by a type conversion to occur only if the source of the type
> >conversion's mode is word mode or smaller.
> >
> >Bootstrapped and reg tested on x86_64 (with --enable-languages=c,c++).
> >
> >If ok, could you commit please? I don't have commit access.
> >
> >Regards
> >Senthil
> >
> >gcc/ChangeLog
> >
> >2015-11-11 Senthil Kumar Selvaraj
> >
> > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> > if innerop's mode is word_mode or smaller.
> >
> >
> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c
> >index e2393e4..c139bc6 100644
> >--- gcc/tree-vrp.c
> >+++ gcc/tree-vrp.c
> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt)
> > innerop = gimple_assign_rhs1 (def_stmt);
> >
> > if (TREE_CODE (innerop) == SSA_NAME
> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop)))
> >+ <= GET_MODE_SIZE(word_mode))
> > && !POINTER_TYPE_P (TREE_TYPE (innerop)))
> > {
> > value_range *vr = get_value_range (innerop);
>
> I thought the result of the discussion was that the transformation is ok if
> either it is narrowing or it widens but to something no bigger than
> word_mode. So you should have 2 comparisons, or 1 with a max.
Hmm, I came to the opposite conclusion - I thought Richard only okayed
"widening upto word-mode", not the narrowing.
Richard?
Regards
Senthil
>
> --
> Marc Glisse
Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode
On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote:
> On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj
> wrote:
> >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote:
> >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
> >>
> >> >This patch came out of a discussion held in the gcc mailing list
> >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html).
> >> >
> >> >The patch restricts folding of conditional exprs with lhs previously
> >> >set by a type conversion to occur only if the source of the type
> >> >conversion's mode is word mode or smaller.
> >> >
> >> >Bootstrapped and reg tested on x86_64 (with
> >--enable-languages=c,c++).
> >> >
> >> >If ok, could you commit please? I don't have commit access.
> >> >
> >> >Regards
> >> >Senthil
> >> >
> >> >gcc/ChangeLog
> >> >
> >> >2015-11-11 Senthil Kumar Selvaraj
> >
> >> >
> >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> >> > if innerop's mode is word_mode or smaller.
> >> >
> >> >
> >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c
> >> >index e2393e4..c139bc6 100644
> >> >--- gcc/tree-vrp.c
> >> >+++ gcc/tree-vrp.c
> >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt)
> >> > innerop = gimple_assign_rhs1 (def_stmt);
> >> >
> >> > if (TREE_CODE (innerop) == SSA_NAME
> >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop)))
> >> >+ <= GET_MODE_SIZE(word_mode))
> >> >&& !POINTER_TYPE_P (TREE_TYPE (innerop)))
> >> > {
> >> >value_range *vr = get_value_range (innerop);
> >>
> >> I thought the result of the discussion was that the transformation is
> >ok if
> >> either it is narrowing or it widens but to something no bigger than
> >> word_mode. So you should have 2 comparisons, or 1 with a max.
> >
> >Hmm, I came to the opposite conclusion - I thought Richard only okayed
> >"widening upto word-mode", not the narrowing.
>
> I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK.
My bad. Here's a revised patch that checks for both conditions, using
max as Marc suggested to limit to word_mode or narrowing conversions.
Bootstrapped and regtested for x86_64 with c and c++.
Is this ok? If yes, would you commit it
for me please? I don't have commit access.
gcc/ChangeLog
2015-11-14 Senthil Kumar Selvaraj
* tree-vrp.c (simplify_cond_using_ranges): Fold only
if innerop's mode smaller or equal to word_mode or op0's mode.
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index e2393e4..cfd90e7 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt)
innerop = gimple_assign_rhs1 (def_stmt);
if (TREE_CODE (innerop) == SSA_NAME
- && !POINTER_TYPE_P (TREE_TYPE (innerop)))
+ && !POINTER_TYPE_P (TREE_TYPE (innerop))
+ && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop)))
+ <= std::max (GET_MODE_SIZE (word_mode),
+GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0))
{
value_range *vr = get_value_range (innerop);
Regards
Senthil
>
> Richard.
>
> >Richard?
> >
> >Regards
> >Senthil
> >>
> >> --
> >> Marc Glisse
>
>
Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode
On Mon, Nov 16, 2015 at 10:02:15AM +0100, Richard Biener wrote:
> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
>
> > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote:
> > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj
> > > wrote:
> > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote:
> > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
> > > >>
> > > >> >This patch came out of a discussion held in the gcc mailing list
> > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html).
> > > >> >
> > > >> >The patch restricts folding of conditional exprs with lhs previously
> > > >> >set by a type conversion to occur only if the source of the type
> > > >> >conversion's mode is word mode or smaller.
> > > >> >
> > > >> >Bootstrapped and reg tested on x86_64 (with
> > > >--enable-languages=c,c++).
> > > >> >
> > > >> >If ok, could you commit please? I don't have commit access.
> > > >> >
> > > >> >Regards
> > > >> >Senthil
> > > >> >
> > > >> >gcc/ChangeLog
> > > >> >
> > > >> >2015-11-11 Senthil Kumar Selvaraj
> > > >
> > > >> >
> > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> > > >> > if innerop's mode is word_mode or smaller.
> > > >> >
> > > >> >
> > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c
> > > >> >index e2393e4..c139bc6 100644
> > > >> >--- gcc/tree-vrp.c
> > > >> >+++ gcc/tree-vrp.c
> > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt)
> > > >> > innerop = gimple_assign_rhs1 (def_stmt);
> > > >> >
> > > >> > if (TREE_CODE (innerop) == SSA_NAME
> > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop)))
> > > >> >+ <= GET_MODE_SIZE(word_mode))
> > > >> >&& !POINTER_TYPE_P (TREE_TYPE (innerop)))
> > > >> > {
> > > >> >value_range *vr = get_value_range (innerop);
> > > >>
> > > >> I thought the result of the discussion was that the transformation is
> > > >ok if
> > > >> either it is narrowing or it widens but to something no bigger than
> > > >> word_mode. So you should have 2 comparisons, or 1 with a max.
> > > >
> > > >Hmm, I came to the opposite conclusion - I thought Richard only okayed
> > > >"widening upto word-mode", not the narrowing.
> > >
> > > I didn't mean to suggest narrowing is not OK. In fact narrowing is
> > > always OK.
> >
> > My bad. Here's a revised patch that checks for both conditions, using
> > max as Marc suggested to limit to word_mode or narrowing conversions.
> >
> > Bootstrapped and regtested for x86_64 with c and c++.
> >
> > Is this ok? If yes, would you commit it
> > for me please? I don't have commit access.
> >
> > gcc/ChangeLog
> > 2015-11-14 Senthil Kumar Selvaraj
> >
> > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> > if innerop's mode smaller or equal to word_mode or op0's mode.
> >
> >
> > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> > index e2393e4..cfd90e7 100644
> > --- a/gcc/tree-vrp.c
> > +++ b/gcc/tree-vrp.c
> > @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt)
> >innerop = gimple_assign_rhs1 (def_stmt);
> >
> >if (TREE_CODE (innerop) == SSA_NAME
> > - && !POINTER_TYPE_P (TREE_TYPE (innerop)))
> > + && !POINTER_TYPE_P (TREE_TYPE (innerop))
> > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop)))
> > + <= std::max (GET_MODE_SIZE (word_mode),
> > +GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0))
>
> Please use TYPE_PRECISION (...) and GET_MODE_PRECISION (word_mode) and
> add a comment as to what we are testing here and why.
>
> Btw, ideally we'd factor out a
>
> bool
> desired_pro_or_demotion_p (tree to_type, tree from_type) {}
>
> function somewhere as we have similar tests throughout the compiler
> that we might want to
Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode
On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote:
> On Wed, 18 Nov 2015, Senthil Kumar Selvaraj wrote:
>
> > On Mon, Nov 16, 2015 at 10:02:15AM +0100, Richard Biener wrote:
> > > On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
> > >
> > > > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote:
> > > > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj
> > > > > wrote:
> > > > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote:
> > > > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
> > > > > >>
> > > > > >> >This patch came out of a discussion held in the gcc mailing list
> > > > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html).
> > > > > >> >
> > > > > >> >The patch restricts folding of conditional exprs with lhs
> > > > > >> >previously
> > > > > >> >set by a type conversion to occur only if the source of the type
> > > > > >> >conversion's mode is word mode or smaller.
> > > > > >> >
> > > > > >> >Bootstrapped and reg tested on x86_64 (with
> > > > > >--enable-languages=c,c++).
> > > > > >> >
> > > > > >> >If ok, could you commit please? I don't have commit access.
> > > > > >> >
> > > > > >> >Regards
> > > > > >> >Senthil
> > > > > >> >
> > > > > >> >gcc/ChangeLog
> > > > > >> >
> > > > > >> >2015-11-11 Senthil Kumar Selvaraj
> > > > > >
> > > > > >> >
> > > > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> > > > > >> > if innerop's mode is word_mode or smaller.
> > > > > >> >
> > > > > >> >
> > > > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c
> > > > > >> >index e2393e4..c139bc6 100644
> > > > > >> >--- gcc/tree-vrp.c
> > > > > >> >+++ gcc/tree-vrp.c
> > > > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt)
> > > > > >> > innerop = gimple_assign_rhs1 (def_stmt);
> > > > > >> >
> > > > > >> > if (TREE_CODE (innerop) == SSA_NAME
> > > > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop)))
> > > > > >> >+ <= GET_MODE_SIZE(word_mode))
> > > > > >> >&& !POINTER_TYPE_P (TREE_TYPE (innerop)))
> > > > > >> > {
> > > > > >> >value_range *vr = get_value_range (innerop);
> > > > > >>
> > > > > >> I thought the result of the discussion was that the transformation
> > > > > >> is
> > > > > >ok if
> > > > > >> either it is narrowing or it widens but to something no bigger than
> > > > > >> word_mode. So you should have 2 comparisons, or 1 with a max.
> > > > > >
> > > > > >Hmm, I came to the opposite conclusion - I thought Richard only
> > > > > >okayed
> > > > > >"widening upto word-mode", not the narrowing.
> > > > >
> > > > > I didn't mean to suggest narrowing is not OK. In fact narrowing is
> > > > > always OK.
> > > >
> > > > My bad. Here's a revised patch that checks for both conditions, using
> > > > max as Marc suggested to limit to word_mode or narrowing conversions.
> > > >
> > > > Bootstrapped and regtested for x86_64 with c and c++.
> > > >
> > > > Is this ok? If yes, would you commit it
> > > > for me please? I don't have commit access.
> > > >
> > > > gcc/ChangeLog
> > > > 2015-11-14 Senthil Kumar Selvaraj
> > > >
> > > > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> > > > if innerop's mode smaller or equal to word_mode or op0's mode.
> > > >
> > > >
> > > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> > > > index e2393e4..cfd90e7 100644
> > > > --- a/gcc/tree-
Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode
On Thu, Nov 19, 2015 at 10:31:41AM -0700, Jeff Law wrote: > On 11/18/2015 11:20 PM, Senthil Kumar Selvaraj wrote: > >On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote: > >> > >>Otherwise ok. > > > >See modified patch below. If you think vrp98.c is unnecessary, feel free > >to dump it :). > > > >If ok, could you commit it for me please? I don't have commit access. > > > >Regards > >Senthil > > > >gcc/ChangeLog > >2015-11-19 Senthil Kumar Selvaraj > > > > * tree.h (desired_pro_or_demotion_p): New function. > > * tree-vrp.c (simplify_cond_using_ranges): Call it. > > > >gcc/testsuite/ChangeLog > >2015-11-19 Senthil Kumar Selvaraj > > > > * gcc.dg/tree-ssa/vrp98.c: New testcase. > > * gcc.target/avr/uint8-single-reg.c: New testcase. > I went ahead and committed this as-is. > > I do think the vrp98 testcase is useful as it verifies that VRP is doing > what we want in a target independent way. It's a good complement to the AVR > specific testcase. I see the same problem on gcc-5-branch as well. Would it be ok to backport the fix to that branch as well? Regards Senthil > > Thanks, > Jeff >
Re: [Patch, avr, PR85624] - Fix ICE when initializing 128-byte aligned array
Ping!
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Hi,
>
> The below patch fixes an ICE for the avr target when the setmemhi
> expander is involved.
>
> The setmemhi expander generated RTL ends up as an unrecognized insn
> if the alignment of the destination exceeds that of a QI
> mode const_int (127), AND the number of bytes to set fits in a QI
> mode const_int. The second condition prevents *clrmemhi from matching,
> and *clrmemqi does not match because it expects operand 3 (the alignment
> const_int rtx) to be QI mode, and a value of 128 or greater does not fit.
>
> The patch fixes this by changing the *clrmemqi pattern to match a HI
> mode const_int, and also adds a testcase.
>
> Regression test showed no new failures, ok to commit to trunk?
>
> Regards
> Senthil
>
> gcc/ChangeLog:
>
> 2018-07-18 Senthil Kumar Selvaraj
>
> PR target/85624
> * config/avr/avr.md (*clrmemqi): Change mode of operands[2]
> from QI to HI.
>
> gcc/testsuite/ChangeLog:
>
> 2018-07-18 Senthil Kumar Selvaraj
>
> PR target/85624
> * gcc.target/avr/pr85624.c: New test.
>
> diff --git gcc/config/avr/avr.md gcc/config/avr/avr.md
> index e619e695418..644e3cfabc5 100644
> --- gcc/config/avr/avr.md
> +++ gcc/config/avr/avr.md
> @@ -1095,7 +1095,7 @@
>[(set (mem:BLK (match_operand:HI 0 "register_operand" "e"))
> (const_int 0))
> (use (match_operand:QI 1 "register_operand" "r"))
> - (use (match_operand:QI 2 "const_int_operand" "n"))
> + (use (match_operand:HI 2 "const_int_operand" "n"))
> (clobber (match_scratch:HI 3 "=0"))
> (clobber (match_scratch:QI 4 "=&1"))]
>""
> diff --git gcc/testsuite/gcc.target/avr/pr85624.c
> gcc/testsuite/gcc.target/avr/pr85624.c
> new file mode 100644
> index 000..ede2e80216a
> --- /dev/null
> +++ gcc/testsuite/gcc.target/avr/pr85624.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +/* This testcase exposes PR85624. An alignment directive with
> + a value greater than 127 on an array with dimensions that fit
> + QImode causes an 'unrecognizable insn' ICE. Turns out clrmemqi
> + did not match the pattern expanded by setmemhi, because it
> + assumed the alignment val will fit in a QI. */
> +
> +int foo() {
> + volatile int arr[3] __attribute__((aligned(128))) = {0};
> + return arr[2];
> +}
Re: [Patch, avr, PR85624] - Fix ICE when initializing 128-byte aligned array
Jeff Law writes:
> On 08/06/2018 04:53 AM, Senthil Kumar Selvaraj wrote:
>> Ping!
>>
>> Regards
>> Senthil
>>
>> Senthil Kumar Selvaraj writes:
>>
>>> Hi,
>>>
>>> The below patch fixes an ICE for the avr target when the setmemhi
>>> expander is involved.
>>>
>>> The setmemhi expander generated RTL ends up as an unrecognized insn
>>> if the alignment of the destination exceeds that of a QI
>>> mode const_int (127), AND the number of bytes to set fits in a QI
>>> mode const_int. The second condition prevents *clrmemhi from matching,
>>> and *clrmemqi does not match because it expects operand 3 (the alignment
>>> const_int rtx) to be QI mode, and a value of 128 or greater does not fit.
>>>
>>> The patch fixes this by changing the *clrmemqi pattern to match a HI
>>> mode const_int, and also adds a testcase.
>>>
>>> Regression test showed no new failures, ok to commit to trunk?
>>>
>>> Regards
>>> Senthil
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-07-18 Senthil Kumar Selvaraj
>>>
>>> PR target/85624
>>> * config/avr/avr.md (*clrmemqi): Change mode of operands[2]
>>> from QI to HI.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2018-07-18 Senthil Kumar Selvaraj
>>>
>>> PR target/85624
>>> * gcc.target/avr/pr85624.c: New test.
> Given there's also pattern clrmemhi it feels like you're papering over a
> bug elsewhere, possibly in the expanders.
clrmemhi and clrmemqi differ on the mode of the register used to hold
the number of bytes to clear. The setmemhi expander picks a QI or HI
mode reg depending on the width of the size operand, and the
clrmem{qi,hi} match on that.
The operand whose mode I modified represents the alignment of the
destination, and isn't actually used in the assembler template.
Regards
Senthil
>
> Ultimately I'll leave the decision here to the AVR maintainers through.
>
> jeff
[Ping, avr] Emit diagnostics if -f{pic,PIC,pie,PIE} or -shared is passed
Ping!
Regards
Senthil
On Mon, Nov 04, 2013 at 06:45:19PM +0530, Senthil Kumar Selvaraj wrote:
> The AVR backend does not generate position independent code, yet it
> happily accepts -fpic, -fPIC, -fpie and -fPIE. The generated code
> doesn't change at all. Also, it accepts the -shared option to generate a
> shared library, without really doing anything with it.
>
> This causes one of the regression tests
> (gcc.dg/lto/pr54709 c_lto_pr54709_0.o-c_lto_pr54709_1.o link) to fail with
> an 'undefined reference to main' error, when the test is trying to build
> a shared object.
>
> The attached patch generates a warning if one of the -f{pic,PIC,pie,PIE}
> options is provided, and an error if -shared is provided (
> config/mep/mep.c and config/s390/tpf.h already do something very similar).
>
> Regression tested with no new failures.Tests which exercise PIC now report as
> unsupported.
>
> If ok, could someone commit please? I don't have commit access.
>
> Regards
> Senthil
>
> gcc/ChangeLog
> 2013-11-04 Senthil Kumar Selvaraj
>
> * config/avr/avr.c (avr_option_override): Warn if asked to generate
> position independent code.
> * config/avr/avr.h: Modify LINK_SPEC to reject -shared.
>
>
> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
> index e7e1c2f..cf4b8ca 100644
> --- gcc/config/avr/avr.c
> +++ gcc/config/avr/avr.c
> @@ -310,6 +310,15 @@ avr_option_override (void)
>flag_omit_frame_pointer = 0;
> }
>
> + if (flag_pic == 1)
> +warning (OPT_fpic, "-fpic is not supported");
> + if (flag_pic == 2)
> +warning (OPT_fPIC, "-fPIC is not supported");
> + if (flag_pie == 1)
> +warning (OPT_fpie, "-fpie is not supported");
> + if (flag_pie == 2)
> +warning (OPT_fPIE, "-fPIE is not supported");
> +
>avr_current_device = &avr_mcu_types[avr_mcu_index];
>avr_current_arch = &avr_arch_types[avr_current_device->arch];
>
> diff --git gcc/config/avr/avr.h gcc/config/avr/avr.h
> index f223a61..1eff5be 100644
> --- gcc/config/avr/avr.h
> +++ gcc/config/avr/avr.h
> @@ -522,7 +522,8 @@ extern const char *avr_device_to_sp8 (int argc, const
> char **argv);
> mmcu=at90can64*|\
> mmcu=at90usb64*:--pmem-wrap-around=64k}}}\
> %:device_to_ld(%{mmcu=*:%*})\
> -%:device_to_data_start(%{mmcu=*:%*})"
> +%:device_to_data_start(%{mmcu=*:%*})\
> +%{shared:%eshared is not supported}"
>
> #define LIB_SPEC \
>
> "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28:
> -lc }"
Fix address space computation in expand_debug_expr
For the AVR target, assertions in convert_debug_memory_address cause a couple of ICEs (see PR 52472). Jakub suggested returning a NULL rtx, which works, but on debugging further, I found that expand_debug_expr appears to incorrectly compute the address space for ADDR_EXPR and MEM_REFs. For ADDR_EXPR, TREE_TYPE(exp) is a POINTER_TYPE (always?), but in the generic address space, even if the object whose address is taken is in a different address space. expand_debug_expr takes TYPE_ADDR_SPACE(TREE_TYPE(exp)) and therefore computes the address space as generic. convert_debug_memory_address then asserts that the mode is a valid pointer mode in the address space and fails. Similarly, for MEM_REFs, TREE_TYPE(exp) is the type of the dereferenced value, and therefore checking if it is a POINTER_TYPE doesn't help for a single pointer dereference. The address space gets computed as generic even if the pointer points to a different address space. The address mode for the generic address space is passed to convert_debug_memory_address, and the assertion that that mode must match the mode of the rtx then fails. The below patch attempts to fix this by picking the right TREE_TYPE to pass to TYPE_ADDR_SPACE for MEM_REF (use type of arg 0) and ADDR_EXPR (check for pointer type and look at nested addr space). Does this look reasonable or did I get it all wrong? Regards Senthil diff --git gcc/cfgexpand.c gcc/cfgexpand.c index 8b0e466..ca78953 100644 --- gcc/cfgexpand.c +++ gcc/cfgexpand.c @@ -3941,8 +3941,8 @@ expand_debug_expr (tree exp) op0 = plus_constant (inner_mode, op0, INTVAL (op1)); } - if (POINTER_TYPE_P (TREE_TYPE (exp))) - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); + if (POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0 + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0; else as = ADDR_SPACE_GENERIC; @@ -4467,7 +4467,11 @@ expand_debug_expr (tree exp) return NULL; } - as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); + if (POINTER_TYPE_P (TREE_TYPE (exp))) +as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); + else +as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); + op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as); return op0;
Re: Fix address space computation in expand_debug_expr
On Thu, Jun 05, 2014 at 09:46:25AM +0200, Richard Biener wrote: > On Wed, Jun 4, 2014 at 10:06 PM, Senthil Kumar Selvaraj > wrote: > > For the AVR target, assertions in convert_debug_memory_address cause a > > couple of ICEs (see PR 52472). Jakub suggested returning a NULL rtx, > > which works, but on debugging further, I found that expand_debug_expr > > appears to incorrectly compute the address space for ADDR_EXPR and > > MEM_REFs. > > > > For ADDR_EXPR, TREE_TYPE(exp) is a POINTER_TYPE (always?), but in > > the generic address space, even if the object whose address is taken > > is in a different address space. expand_debug_expr takes > > TYPE_ADDR_SPACE(TREE_TYPE(exp)) and therefore computes the address > > space as generic. convert_debug_memory_address then asserts that the > > mode is a valid pointer mode in the address space and fails. > > > > Similarly, for MEM_REFs, TREE_TYPE(exp) is the type of the > > dereferenced value, and therefore checking if it is a POINTER_TYPE > > doesn't help for a single pointer dereference. The address space > > gets computed as generic even if the pointer points to a different > > address space. The address mode for the generic address space is > > passed to convert_debug_memory_address, and the assertion that that mode > > must match the mode of the rtx then fails. > > > > The below patch attempts to fix this by picking the right TREE_TYPE to > > pass to TYPE_ADDR_SPACE for MEM_REF (use type of arg 0) and > > ADDR_EXPR (check for pointer type and look at nested addr space). > > > > Does this look reasonable or did I get it all wrong? > > > > Regards > > Senthil > > > > diff --git gcc/cfgexpand.c gcc/cfgexpand.c > > index 8b0e466..ca78953 100644 > > --- gcc/cfgexpand.c > > +++ gcc/cfgexpand.c > > @@ -3941,8 +3941,8 @@ expand_debug_expr (tree exp) > > op0 = plus_constant (inner_mode, op0, INTVAL (op1)); > > } > > > > - if (POINTER_TYPE_P (TREE_TYPE (exp))) > > - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); > > + if (POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0 > > + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, > > 0; > >else > > as = ADDR_SPACE_GENERIC; > > TREE_OPERAND (exp, 0) always has pointer type so I'd change this to > > as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0; > > > @@ -4467,7 +4467,11 @@ expand_debug_expr (tree exp) > > return NULL; > > } > > > > - as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); > > + if (POINTER_TYPE_P (TREE_TYPE (exp))) > > +as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); > > + else > > +as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); > > + > > Likewise - TREE_TYPE (exp) is always a pointer type. Otherwise the > patch looks ok to me. > > Richard. > > >op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as); > > > > return op0; Modified patch attached. This fixes PR 52472 (the ADDR_EXPR case) as well as a couple of ICEs in gcc.target/avr/torture/addr-space-2-x.c (MEM_REF case). I've also added a new testcase for the PR. I don't have commit access - could someone apply this for me please? It'd be great if this was backported to 4.9 and 4.8 branches as well. Regards Senthil gcc/ChangeLog 2014-06-05 Senthil Kumar Selvaraj PR target/52472 * cfgexpand.c (expand_debug_expr): Use address space of nested TREE_TYPE for ADDR_EXPR and MEM_REF. gcc/testsuite/ChangeLog 2014-06-05 Senthil Kumar Selvaraj PR target/52472 * gcc.target/avr/pr52472.c: New test. diff --git gcc/cfgexpand.c gcc/cfgexpand.c index 8b0e466..e161cb7 100644 --- gcc/cfgexpand.c +++ gcc/cfgexpand.c @@ -3941,10 +3941,7 @@ expand_debug_expr (tree exp) op0 = plus_constant (inner_mode, op0, INTVAL (op1)); } - if (POINTER_TYPE_P (TREE_TYPE (exp))) - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); - else - as = ADDR_SPACE_GENERIC; + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0; op0 = convert_debug_memory_address (targetm.addr_space.address_mode (as), op0, as); @@ -4467,7 +4464,7 @@ expand_debug_expr (tree exp) return NULL; } - as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as); return op0; diff --git gcc/testsuite/gcc.target/avr/pr52472.c gcc/testsuite/gcc.target/avr/pr
Re: [dwarf, RFC] Emitting per-function dwarf info
Ping!
Regards
Senthil
On Fri, Apr 10, 2015 at 12:19:36PM +0530, Senthil Kumar Selvaraj wrote:
> Hi,
>
> This (rather big) patch is an attempt to generate per function DWARF
> information for functions that go into their own sections (through
> -ffunction-section or otherwise). This is so that the GNU linker's
> garbage collection mechanism can then remove all DWARF information when it
> removes an unmarked function section.
>
> Most of the code for splitting off the debug information was adapted from
> Jason's comdat-debug branch, with a few changes to move to the C++
> collection types.
>
> We started out with section groups, aiming to generate one section group
> per function containing DWARF info AND code for the function, so that when
> the linker gc's a function, it removes the whole section group.
> But it turned out to be difficult to split all DWARF information - data that
> goes into debug_ranges, for example, is maintained in a flat list, and we
> found it hard to split it per function.
>
> If not all debug information is in the section group, references from the
> "global" DWARF info into the section group results in the section group
> itself not getting garbage collected - effectively breaking gc-sections.
>
> We then figured that the GNU binutils linker removes sections with the
> SEC_DEBUGGING flag that are named with the function's text section as the
> suffix (see https://www.sourceware.org/ml/binutils/2015-03/msg00326.html),
> so
> we currently generate sections named to match that, and then let the
> linker work its magic.
>
> At a high level, this is what we do
>
> 1. In gen_subprogram_die, we see if the fde should go into a separate CU and
> if yes, record it in a hash_table.
> 2. In dwarf2_out_finish, we check if each subprogram die is in the table,
> and
> if yes, create a new CU for the die, along with a DW_TAG_imported_unit
> die referring to the main CU. The newly created CU die is also
> recorded in the hash table.
> 3. In output_comp_unit, if the CU is in the hash table, we switch to
>.debug_info instead of debug_info_section.
> 4. We do the same thing when generating aranges, pubnames and debug_loc.
>
> Bootstrapping x86_64-linux for c and c++ works, but there are quite a
> few regression test failures. We'll analyze and follow-up on those.
>
> The formatting style is inconsistent, we'll fix those as well.
>
> We wanted to put it out before going ahead further. How does this look?
> Are there are any major problems with this approach?
>
> For code like this,
>
> volatile int x;
> void foo() { x--; }
> int main() { return x; }
>
> $ ~/native/install/bin/gcc -ffunction-sections -g3 -Wa,-gdwarf-sections
> -fdwarf-sections test.c -c
> $ ~/native/install/bin/objdump -h test.o
>
> 5 .text.foo 0016 0050 2**0
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> 6 .text.main001a 0066 2**0
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> 7 .debug_info.text.main 0044
> 0080 2**0
> CONTENTS, RELOC, READONLY, DEBUGGING
> 8 .debug_info.text.foo 0040
> 00b1 2**0
> CONTENTS, RELOC, READONLY, DEBUGGING
> 9 .debug_info 004d 00e5 2**0
> CONTENTS, RELOC, READONLY, DEBUGGING
> 10 .debug_abbrev 007f 011e 2**0
> CONTENTS, READONLY, DEBUGGING
> 11 .debug_aranges 0020 0194
> 2**0
> CONTENTS, RELOC, READONLY, DEBUGGING
> 12 .debug_aranges.text.main 0030
> 01b3 2**0
> CONTENTS, RELOC, READONLY, DEBUGGING
> 13 .debug_aranges.text.foo 0030
> 01d6 2**0
> CONTENTS, RELOC, READONLY, DEBUGGING
> 14 .debug_ranges 0070 01f9 2**0
> CONTENTS, RELOC, READONLY, DEBUGGING
>
> Linking and dumping debug info shows that there is no debug info for
> foo (which would have been gc'ed by the linker)
>
> $ ~/native/install/bin/gcc -Wl,--gc-sections test.o -Wl,-Map=test.map
> $ cat test.map
>
> Discarded input sections
>
>
> .text.foo 0x 0x16 test.o
> .debug_info.text.foo
> 0x000
Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
On Thu, Apr 16, 2015 at 11:02:05AM +0200, Georg-Johann Lay wrote:
> Am 04/16/2015 um 08:43 AM schrieb Senthil Kumar Selvaraj:
> >This patch fixes PR 65657.
>
> The following artifact appears to be PR63633.
>
I did see that one - unfortunately, that fix won't help here. IIUC, you
check if input/output operand hard regs are in the clobber list,
and if yes, you generate pseudos to save and restore clobbered hard
regs.
In this case, the reg is actually clobbered by a different insn (one
that loads the next argument to the function). So unless I blindly generate
pseudos for
all regs in the clobber list, the clobbering will still happen.
FWIW, I did try saving/restoring all clobbered regs, and it did fix the
problem - just that it appeared like a (worse) hack to me. Aren't we
manually replicating what RA/reload should be doing?
What do you think?
> >When cfgexpand.c expands a function call, it first figures out the
> >registers in which the arguments will go, followed by expansion of the
> >arguments themselves (right to left). It later emits mov insns to set
> >the precomputed registers with the expanded RTXes.
> >
> >If one of the arguments is a __memx char pointer dereference, the mov
> >eventually expands to gen_xload_A (for certain cases), which
> >clobbers R22, R21 and Z. This causes problems if one of those
> >clobbered registers was precomputed to hold another argument.
> >
> >In general, call expansion does not appear to take clobbers into account -
> >when it checks for argument overlap, the RTX (args[i].value) is only a MEM
> >in QImode for the memx deref - the clobber shows up when it eventually
> >calls emit_move_insn, at which point, it is too late.
> >
> >This does not happen for a int pointer dereference - turns out that
> >precompute_register_parameters does a copy_to_mode_reg if the
> >cost of args[i].value is more than COSTS_N_INSNS(1) i.e., it creates a
> >pseudo and later assigns the pseudo to the arg register. This is done
> >before any moves to arg registers is done, so other arguments are not
> >overwritten.
> >
> >Doing the same thing - providing a better cost estimate for a MEM rtx in
> >the non-default address space, makes this problem go away, and that is
> >what this patch does. Regression testing does not show any new failures.
> >
> >The fix does seem tangential to the core problem - not sure if there is
> >a better way to fix this?
> >
> >If not, could someone commit this please? I don't have commit access.
> >
> >Regards
> >Senthil
> >
> >gcc/ChangeLog
> >
> >2015-04-16 Senthil Kumar Selvaraj
> >
> > * config/avr/avr.c (avr_rtx_costs_1): Increase cost for
> > MEM rtx in non-default address space.
> >
> >gcc/testsuite/ChangeLog
> >
> >2015-04-16 Senthil Kumar Selvaraj
> >
> > * gcc.target/avr/pr65657.c: New.
> >
> >diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
> >index 68d5ddc..c9b8678 100644
> >--- gcc/config/avr/avr.c
> >+++ gcc/config/avr/avr.c
> >@@ -9959,7 +9959,11 @@ avr_rtx_costs_1 (rtx x, int codearg, int outer_code
> >ATTRIBUTE_UNUSED,
> >return true;
> >
> > case MEM:
> >- *total = COSTS_N_INSNS (GET_MODE_SIZE (mode));
> >+ /* MEM rtx with non-default address space is more
> >+ expensive. Not expressing that results in reg
> >+ clobber during expand (PR 65657). */
> >+ *total = COSTS_N_INSNS (GET_MODE_SIZE (mode)
> >+ + (MEM_ADDR_SPACE(x) == ADDR_SPACE_RAM ? 0 : 5));
> >return true;
>
> IMO costs should not be used to fix wrong code or ICEs. Presumably the fix
> is similar to the one used by PR63633.
Yes, fixing it by adjusting costs is fragile, but it appeared cleaner to me.
Regards
Senthil
>
> Johann
>
> > case NEG:
> >diff --git gcc/testsuite/gcc.target/avr/pr65657.c
> >gcc/testsuite/gcc.target/avr/pr65657.c
> >new file mode 100644
> >index 000..d908fe3
> >--- /dev/null
> >+++ gcc/testsuite/gcc.target/avr/pr65657.c
> >@@ -0,0 +1,39 @@
> >+/* { dg-do run } */
> >+/* { dg-options "-Os" } */
> >+
> >+/* When cfgexpand expands the call to foo, it
> >+ assigns registers R22:R23 to 0xABCD and R24
> >+ to *x. Because x is a __memx address space
> >+ pointer, the dereference results in an RTL
> >+ insn that clobbers R22 among other
> >+ registers (xload_A).
> >+
> >+ Call expansion does not take this into account
> >+ and ends up clobbering R22 set previously to
> >+ hold part of the second argument.
> >+*/
> >+
> >+#include
> >+
> >+void __attribute__((noinline))
> >+__attribute__((noclone))
> >+foo (char c, volatile unsigned int d)
> >+{
> >+if (d != 0xABCD)
> >+ abort();
> >+if (c != 'A')
> >+abort();
> >+}
> >+
> >+void __attribute__((noinline))
> >+__attribute__((noclone))
> >+readx(const char __memx *x)
> >+{
> >+foo (*x, 0xABCD);
> >+}
> >+
> >+const char __memx arr[] = { 'A' };
> >+int main()
> >+{
> >+ readx (arr);
> >+}
> >
>
Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
On Thu, Apr 16, 2015 at 06:47:26PM +0200, Georg-Johann Lay wrote: > Am 04/16/2015 um 11:28 AM schrieb Senthil Kumar Selvaraj: > >On Thu, Apr 16, 2015 at 11:02:05AM +0200, Georg-Johann Lay wrote: > >>Am 04/16/2015 um 08:43 AM schrieb Senthil Kumar Selvaraj: > >>>This patch fixes PR 65657. > >> > >>The following artifact appears to be PR63633. > >> > > > >I did see that one - unfortunately, that fix won't help here. IIUC, you > >check if input/output operand hard regs are in the clobber list, > >and if yes, you generate pseudos to save and restore clobbered hard > >regs. > > > >In this case, the reg is actually clobbered by a different insn (one > > Arrgh, yes... > > >that loads the next argument to the function). So unless I blindly generate > >pseudos for > >all regs in the clobber list, the clobbering will still happen. > > > >FWIW, I did try saving/restoring all clobbered regs, and it did fix the > >problem - just that it appeared like a (worse) hack to me. Aren't we > >manually replicating what RA/reload should be doing? > > As it appears, we'll have to do it by hand. The attaches patch is just a > sketch that indicates how the problem could be approached. Notice the new > assertions in the split expanders; they will throw ICE until the fix is > actually installed. > > The critical insn are generated in movMM expander and are changed to have no > clobbers (SCRATCHes actually). An a later pass, when appropriate life info > can be made available, run yet another avr pass that > > 1a) Save-restore needed hard regs around the insn. > > 2a) Kick out hard regs overlapping the clobbers, e.g. in set_src, into new > pseudos. Maybe that could happen due to some hard regs progagation, or we > can use a new predicate similar combine_pseudo_register_operand. > > 3) Replace scratch -> hard regs for all scratch_operands. > > 2b) Restore respective hard regs from their pseudos. > > 1b) Restore respective hard regs from their pseudos. > > > And maybe we can get better code by allocating things like address register > by hand and get better code then. > > When I implemented some of the libgcc insns I tried to express the operand > by means of constraints, e.h. for (reg:HI 22) and let register allocator do > the job. > > The resulting code was functional but *horrific*. > > The register allocator is not yet ready to generate efficient code in such > demanding situations... > > > > >What do you think? > > > > IMO sooner or later we'll need such an infrastructure; maybe also for > non-mov insn that are implemented by transparent libcalls like divmod, mul, > etc. I'm curious how other embedded targets handle this though - arm, for example? Surely they should have some libcalls/builtins which need specific hard regs? I should check that out. > > >>>When cfgexpand.c expands a function call, it first figures out the > >>>registers in which the arguments will go, followed by expansion of the > >>>arguments themselves (right to left). It later emits mov insns to set > >>>the precomputed registers with the expanded RTXes. > >>> > >>>If one of the arguments is a __memx char pointer dereference, the mov > >>>eventually expands to gen_xload_A (for certain cases), which > >>>clobbers R22, R21 and Z. This causes problems if one of those > >>>clobbered registers was precomputed to hold another argument. > >>> > >>>In general, call expansion does not appear to take clobbers into account - > > We had been warned that using hard regs is evil... But without that > technique the code quality would decrease way too much. Oh ok - do you know some place where this is documented or was discussed? > > >>>when it checks for argument overlap, the RTX (args[i].value) is only a MEM > >>>in QImode for the memx deref - the clobber shows up when it eventually > >>>calls emit_move_insn, at which point, it is too late. > > Such situations could only be handled by a target hook which allowed to > expand specific trees by hand... Such a hook could cater for insn that must > use hard registers. Yes, I guess so :( > > >>>This does not happen for a int pointer dereference - turns out that > >>>precompute_register_parameters does a copy_to_mode_reg if the > >>>cost of args[i].value is more than COSTS_N_INSNS(1) i.e., it creates a > >>>pseudo and later assigns the pseudo to the arg register. This is done > >>>before any moves to arg registers is done, so other
Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
On Fri, Apr 17, 2015 at 01:16:58PM +0530, Senthil Kumar Selvaraj wrote: > On Thu, Apr 16, 2015 at 06:47:26PM +0200, Georg-Johann Lay wrote: > > Am 04/16/2015 um 11:28 AM schrieb Senthil Kumar Selvaraj: > > >On Thu, Apr 16, 2015 at 11:02:05AM +0200, Georg-Johann Lay wrote: > > >>Am 04/16/2015 um 08:43 AM schrieb Senthil Kumar Selvaraj: > > >>>This patch fixes PR 65657. > > >> > > >>The following artifact appears to be PR63633. > > >> > > > > > >I did see that one - unfortunately, that fix won't help here. IIUC, you > > >check if input/output operand hard regs are in the clobber list, > > >and if yes, you generate pseudos to save and restore clobbered hard > > >regs. > > > > > >In this case, the reg is actually clobbered by a different insn (one > > > > Arrgh, yes... > > > > >that loads the next argument to the function). So unless I blindly > > >generate pseudos for > > >all regs in the clobber list, the clobbering will still happen. > > > > > >FWIW, I did try saving/restoring all clobbered regs, and it did fix the > > >problem - just that it appeared like a (worse) hack to me. Aren't we > > >manually replicating what RA/reload should be doing? > > > > As it appears, we'll have to do it by hand. The attaches patch is just a > > sketch that indicates how the problem could be approached. Notice the new > > assertions in the split expanders; they will throw ICE until the fix is > > actually installed. > > > > The critical insn are generated in movMM expander and are changed to have no > > clobbers (SCRATCHes actually). An a later pass, when appropriate life info > > can be made available, run yet another avr pass that > > > > 1a) Save-restore needed hard regs around the insn. > > > > 2a) Kick out hard regs overlapping the clobbers, e.g. in set_src, into new > > pseudos. Maybe that could happen due to some hard regs progagation, or we > > can use a new predicate similar combine_pseudo_register_operand. > > > > 3) Replace scratch -> hard regs for all scratch_operands. > > > > 2b) Restore respective hard regs from their pseudos. > > > > 1b) Restore respective hard regs from their pseudos. > > > > > > And maybe we can get better code by allocating things like address register > > by hand and get better code then. > > > > When I implemented some of the libgcc insns I tried to express the operand > > by means of constraints, e.h. for (reg:HI 22) and let register allocator do > > the job. > > > > The resulting code was functional but *horrific*. > > > > The register allocator is not yet ready to generate efficient code in such > > demanding situations... > > > > > > > >What do you think? > > > > > > > IMO sooner or later we'll need such an infrastructure; maybe also for > > non-mov insn that are implemented by transparent libcalls like divmod, mul, > > etc. > > I'm curious how other embedded targets handle this though - arm, for > example? Surely they should have some libcalls/builtins which need > specific hard regs? I should check that out. > > > > > >>>When cfgexpand.c expands a function call, it first figures out the > > >>>registers in which the arguments will go, followed by expansion of the > > >>>arguments themselves (right to left). It later emits mov insns to set > > >>>the precomputed registers with the expanded RTXes. > > >>> > > >>>If one of the arguments is a __memx char pointer dereference, the mov > > >>>eventually expands to gen_xload_A (for certain cases), which > > >>>clobbers R22, R21 and Z. This causes problems if one of those > > >>>clobbered registers was precomputed to hold another argument. > > >>> > > >>>In general, call expansion does not appear to take clobbers into account > > >>>- > > > > We had been warned that using hard regs is evil... But without that > > technique the code quality would decrease way too much. > > Oh ok - do you know some place where this is documented or was > discussed? > > > > >>>when it checks for argument overlap, the RTX (args[i].value) is only a > > >>>MEM > > >>>in QImode for the memx deref - the clobber shows up when it eventually > > >>>calls emit_move_insn, at which point, it is too late. > > > > Such situations c
Re: [patch,avr]: Fix various problems with specs and specs file generation.
On Mon, Mar 02, 2015 at 08:40:17PM +0100, Georg-Johann Lay wrote:
> The new specs file generation introduces several problems. This patch
>
> - Fix build warnings
>
> - Clean up unused code and the old, now dead specs definitions.
>
> - Removes unused files and adjust build scripts / rules.
>
> - Issues with avr-libc awareness:
>
> -- Makes specs-generation aware of avr-libc (include tm.h in build script
> and depend on WITH_AVRLIBC).
>
> -- avr-libc implements functions in libm.a which usually live in libgcc,
> hence add -lm to libgcc_spec, cf. PR54461.
>
> -- The new libdev.a is a feature of avr-libc and not available with, e.g.,
> newlib. Hence remove it from lib_spec if the compiler is not configured for
> avr-libc (--with-avrlibc=no).
>
> - Many minor issues with option propagation.
>
> - -march had been added to multilib generation some time ago, but
> driver_self_spec was not aware of that option.
>
>
> Ok for trunk?
>
>
> BTW, anyone knows what -march= is good for? It allows all kinds of silly
> option combinations like "-march=avrtiny -mmcu=atmega8" without any
> complaints.
IIRC, -march was added because replacing -mmcu= to
-mmcu= in the driver's self specs broke multilib selection - the
driver always acted as if no -mmcu was specified. Adding
a new option (-march), translating mmcu= to march= and then
basing t-multilib on that worked ok.
>
> What's wrong with -mmcu=avr* ? All architecture names start with "avr", and
> /no/ device name starts /with/ "avr", hence -mmcu= should be fine and work
> as smooth as in the last 10 years.
>
> Why that -march= option? If there is not a good reason for -march=, I'd
> propose to clean it up and use -mmcu=avr* instead.
>
>
> Johann
>
>
> gcc/
> Fix various problems with specs file generation.
>
> * config.gcc (extra_gcc_objs) [avr]: Remove.
>
> * config/avr/driver-avr.c: Remove file.
> * config/avr/t-avr (driver-avr.o): Remove rule.
> (gen-avr-mmcu-specs): Use CXX_FOR_BUILD, CXXFLAGS_FOR_BUILD and
> INCLUDES to build. Depend on TM_H.
> * config/avr/gen-avr-mmcu-specs.c: Tidy up code. Fix various
> build warnings. Fix non-matching types and non-existing %-codes.
> (tm.h): Include.
> (*lib) [!WITH_AVRLIBC]: Don't link libdev.a.
> (*libgcc) [WITH_AVRLIBC]: Add "-lm".
> * config/avr/avrlibc.h (LIBGCC_SPEC): Remove definition.
> * config/avr/avr.h (DRIVER_SELF_SPECS): Fix handling of -march=.
> (CPP_SPEC, CC1PLUS_SPEC, ASM_SPEC, LINK_SPEC, LIB_SPEC)
> (LIBGCC_SPEC): Remove definitions.
>
> Index: config.gcc
> ===
> --- config.gcc(revision 220854)
> +++ config.gcc(working copy)
> @@ -1103,7 +1103,6 @@ avr-*-*)
> fi
> tmake_file="${tmake_file} avr/t-avr avr/t-multilib"
> use_gcc_stdint=wrap
> - extra_gcc_objs="driver-avr.o avr-devices.o"
> extra_objs="avr-devices.o avr-log.o"
> ;;
> bfin*-elf*)
> Index: config/avr/t-avr
> ===
> --- config/avr/t-avr (revision 221028)
> +++ config/avr/t-avr (working copy)
> @@ -16,10 +16,6 @@
> # along with GCC; see the file COPYING3. If not see
> #
[patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
This patch fixes PR 65657.
When cfgexpand.c expands a function call, it first figures out the
registers in which the arguments will go, followed by expansion of the
arguments themselves (right to left). It later emits mov insns to set
the precomputed registers with the expanded RTXes.
If one of the arguments is a __memx char pointer dereference, the mov
eventually expands to gen_xload_A (for certain cases), which
clobbers R22, R21 and Z. This causes problems if one of those
clobbered registers was precomputed to hold another argument.
In general, call expansion does not appear to take clobbers into account -
when it checks for argument overlap, the RTX (args[i].value) is only a MEM
in QImode for the memx deref - the clobber shows up when it eventually
calls emit_move_insn, at which point, it is too late.
This does not happen for a int pointer dereference - turns out that
precompute_register_parameters does a copy_to_mode_reg if the
cost of args[i].value is more than COSTS_N_INSNS(1) i.e., it creates a
pseudo and later assigns the pseudo to the arg register. This is done
before any moves to arg registers is done, so other arguments are not
overwritten.
Doing the same thing - providing a better cost estimate for a MEM rtx in
the non-default address space, makes this problem go away, and that is
what this patch does. Regression testing does not show any new failures.
The fix does seem tangential to the core problem - not sure if there is
a better way to fix this?
If not, could someone commit this please? I don't have commit access.
Regards
Senthil
gcc/ChangeLog
2015-04-16 Senthil Kumar Selvaraj
* config/avr/avr.c (avr_rtx_costs_1): Increase cost for
MEM rtx in non-default address space.
gcc/testsuite/ChangeLog
2015-04-16 Senthil Kumar Selvaraj
* gcc.target/avr/pr65657.c: New.
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 68d5ddc..c9b8678 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -9959,7 +9959,11 @@ avr_rtx_costs_1 (rtx x, int codearg, int outer_code
ATTRIBUTE_UNUSED,
return true;
case MEM:
- *total = COSTS_N_INSNS (GET_MODE_SIZE (mode));
+ /* MEM rtx with non-default address space is more
+ expensive. Not expressing that results in reg
+ clobber during expand (PR 65657). */
+ *total = COSTS_N_INSNS (GET_MODE_SIZE (mode)
+ + (MEM_ADDR_SPACE(x) == ADDR_SPACE_RAM ? 0 : 5));
return true;
case NEG:
diff --git gcc/testsuite/gcc.target/avr/pr65657.c
gcc/testsuite/gcc.target/avr/pr65657.c
new file mode 100644
index 000..d908fe3
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr65657.c
@@ -0,0 +1,39 @@
+/* { dg-do run } */
+/* { dg-options "-Os" } */
+
+/* When cfgexpand expands the call to foo, it
+ assigns registers R22:R23 to 0xABCD and R24
+ to *x. Because x is a __memx address space
+ pointer, the dereference results in an RTL
+ insn that clobbers R22 among other
+ registers (xload_A).
+
+ Call expansion does not take this into account
+ and ends up clobbering R22 set previously to
+ hold part of the second argument.
+*/
+
+#include
+
+void __attribute__((noinline))
+__attribute__((noclone))
+foo (char c, volatile unsigned int d)
+{
+if (d != 0xABCD)
+ abort();
+if (c != 'A')
+abort();
+}
+
+void __attribute__((noinline))
+__attribute__((noclone))
+readx(const char __memx *x)
+{
+foo (*x, 0xABCD);
+}
+
+const char __memx arr[] = { 'A' };
+int main()
+{
+ readx (arr);
+}
Re: [patch, avr] Fix PR64331: insn output and insn length computation rely on REG_DEAD notes.
On Mon, Feb 23, 2015 at 11:53:52AM +0100, Georg-Johann Lay wrote:
> This patch fixes PR64331 which produced wrong code because of outdated (too
> many) REG_DEAD notes. These notes are not (re)computed per default, hence
> do the computation by hand each time avr.c:reg_unused_after is called in a
> different pass.
>
> Ok to apply?
>
I was testing a similar patch a while back. Mine had a call to
df_finish_pass after calling df_analyze, and I didn't "cache" the pass
number. I remember running into ICEs
building libgcc after that - from df_verify, IIRC.
Do you know why df_finish_pass isn't needed in this case? AFAICT, all it
appears to do is to clean up some stuff and enable a verification flag.
Regards
Senthil
> Johann
>
>
> gcc/
> PR target/64331
> * config/avr/avr.c (reg_unused_after): Recompute insn notes if the
> pass changed since the last (re)computation.
> * config/avr/avr.h (machine_function.dead_notes_pass_number): New
> component.
>
> gcc/testsuite/
> PR target/64331
> * gcc.target/avr/torture/pr64331.c: New run test.
>
> Index: config/avr/avr.c
> ===
> --- config/avr/avr.c (revision 220738)
> +++ config/avr/avr.c (working copy)
> @@ -51,6 +51,7 @@
> #include "target-def.h"
> #include "params.h"
> #include "df.h"
> +#include "tree-pass.h" /* for current_pass */
>
> /* Maximal allowed offset for an address in the LD command */
> #define MAX_LD_OFFSET(MODE) (64 - (signed)GET_MODE_SIZE (MODE))
> @@ -7862,8 +7863,22 @@ avr_adjust_insn_length (rtx insn, int le
> int
> reg_unused_after (rtx insn, rtx reg)
> {
> + if (cfun->machine->dead_notes_pass_number
> + != current_pass->static_pass_number)
> +{
> + /* `dead_or_set_p' needs correct REG_DEAD notes, cf. PR64331. Hence
> + recompute them each time we find ourselves in a different pass.
> + `reg_unused_after' is used during final for insn output, and in
> + some earlier passes it is involved in insn length computations. */
> +
> + df_note_add_problem ();
> + df_analyze ();
> +
> + cfun->machine->dead_notes_pass_number =
> current_pass->static_pass_number;
> +}
> +
>return (dead_or_set_p (insn, reg)
> - || (REG_P(reg) && _reg_unused_after (insn, reg)));
> + || (REG_P (reg) && _reg_unused_after (insn, reg)));
> }
>
> /* Return nonzero if REG is not used after INSN.
> Index: config/avr/avr.h
> ===
> --- config/avr/avr.h (revision 220738)
> +++ config/avr/avr.h (working copy)
> @@ -592,6 +592,10 @@ struct GTY(()) machine_function
>/* 'true' if the above is_foo predicates are sanity-checked to avoid
> multiple diagnose for the same function. */
>int attributes_checked_p;
> +
> + /* The latest static pass number for which REG_DEAD notes have been
> + computed, cf. PR64331. */
> + int dead_notes_pass_number;
> };
>
> /* AVR does not round pushes, but the existence of this macro is
> Index: testsuite/gcc.target/avr/torture/pr64331.c
> ===
> --- testsuite/gcc.target/avr/torture/pr64331.c(revision 0)
> +++ testsuite/gcc.target/avr/torture/pr64331.c(revision 0)
> @@ -0,0 +1,38 @@
> +/* { dg-do run } */
> +
> +typedef struct
> +{
> + unsigned a, b;
> +} T2;
> +
> +
> +__attribute__((__noinline__, __noclone__))
> +void foo2 (T2 *t, int x)
> +{
> + if (x != t->a)
> +{
> + t->a = x;
> +
> + if (x && x == t->b)
> + t->a = 20;
> +}
> +}
> +
> +
> +T2 t;
> +
> +int main (void)
> +{
> + t.a = 1;
> + t.b = 1234;
> +
> + foo2 (&t, 1234);
> +
> + if (t.a != 20)
> +__builtin_abort();
> +
> + __builtin_exit (0);
> +
> + return 0;
> +}
> +
[Patch, testsuite] Fix some more bogus failures for avr
Hi,
Committed below patch to trunk as obvious.
Regards
Senthil
2016-08-03 Senthil Kumar Selvaraj
* gcc.dg/init-excess-2.c: Require int32plus.
* gcc.dg/pr44024.c: Skip if target keeps null pointer checks.
* gcc.dg/pr59963-2.c: Require int32plus.
* gcc.dg/pr71084.c: Cast pointer to intprt_t.
* gcc.dg/unroll-7.c: Require int32plus.
Index: gcc.dg/init-excess-2.c
===
--- gcc.dg/init-excess-2.c (revision 239064)
+++ gcc.dg/init-excess-2.c (working copy)
@@ -3,6 +3,7 @@
c/71115 - Missing warning: excess elements in struct initializer. */
/* { dg-do compile } */
/* { dg-options "" } */
+/* { dg-require-effective-target int32plus } */
#include
Index: gcc.dg/pr44024.c
===
--- gcc.dg/pr44024.c(revision 239064)
+++ gcc.dg/pr44024.c(working copy)
@@ -1,5 +1,6 @@
/* { dg-do link } */
/* { dg-options "-O1 -fdelete-null-pointer-checks -fdump-tree-ccp1" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
void foo();
void link_error (void);
Index: gcc.dg/pr59963-2.c
===
--- gcc.dg/pr59963-2.c (revision 239064)
+++ gcc.dg/pr59963-2.c (working copy)
@@ -1,6 +1,7 @@
/* PR c/59963 */
/* { dg-do compile } */
/* { dg-options "-Woverflow -Wconversion" } */
+/* { dg-require-effective-target int32plus } */
extern void bar (unsigned char);
extern void bar8 (unsigned char, unsigned char, unsigned char, unsigned char,
Index: gcc.dg/pr71084.c
===
--- gcc.dg/pr71084.c(revision 239064)
+++ gcc.dg/pr71084.c(working copy)
@@ -2,6 +2,8 @@
/* { dg-do compile } */
/* { dg-options "-O2" } */
+__extension__ typedef __INTPTR_TYPE__ intptr_t;
+
void babl_format (void);
void gimp_drawable_get_format (void);
int _setjmp (void);
@@ -32,7 +34,7 @@
gimp_drawable_get_format();
}
for (; run_height;)
-for (; run_i < (long)fn1; ++run_i)
+for (; run_i < (long)(intptr_t)fn1; ++run_i)
for (; width;)
;
}
Index: gcc.dg/unroll-7.c
===
--- gcc.dg/unroll-7.c (revision 239064)
+++ gcc.dg/unroll-7.c (working copy)
@@ -1,5 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-rtl-loop2_unroll -funroll-loops" } */
+/* { dg-require-effective-target int32plus } */
+
int t(int *a)
{
int i;
[Patch, wwwdocs] Add caveat for AVR port
Hi, This doc patch informs the user that a specific (or higher) version of binutils is a prerequisite for the fix for a rather vexing bug (PR 71151) that was fixed for 6.2. I've added it to the Caveats section; is there a better place? If not, ok to commit? Regards Senthil Index: changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/changes.html,v retrieving revision 1.86 diff -u -r1.86 changes.html --- changes.html20 Jun 2016 10:34:00 - 1.86 +++ changes.html8 Aug 2016 08:17:13 - @@ -40,6 +40,10 @@ +The AVR port requires binutils version 2.26.1 or later for the fix +for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151";>PR71151 +to work. +
Re: [Patch, wwwdocs] Add caveat for AVR port
Georg-Johann Lay writes: > On 08.08.2016 10:24, Senthil Kumar Selvaraj wrote: >> Hi, >> >> This doc patch informs the user that a specific (or higher) version of >> binutils is a prerequisite for the fix for a rather vexing bug (PR >> 71151) that was fixed for 6.2. >> >> I've added it to the Caveats section; is there a better place? If not, ok >> to >> commit? >> >> Regards >> Senthil >> >> Index: changes.html >> === >> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/changes.html,v >> retrieving revision 1.86 >> diff -u -r1.86 changes.html >> --- changes.html 20 Jun 2016 10:34:00 - 1.86 >> +++ changes.html 8 Aug 2016 08:17:13 - >> @@ -40,6 +40,10 @@ >> >> >> >> +The AVR port requires binutils version 2.26.1 or later for the >> fix >> +for > href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71151";>PR71151 >> +to work. >> + >> >> >> > > > Maybe mention that also GCC 6.2 or higher is needed? After all, it's a bug > in > the compiler and using respective Binutils without a fixed compiler is > pointless... Ah, I thought the existing page would stay unmodified and my changes would show up only for 6.2+ :(. I'll say "With GCC 6.2 and later, the AVR port requires...". Sounds good to you? Regards Senthil
Re: [Patch, tentative, reload] Make push_reload work with more types of subregs?
Bernd Schmidt writes:
> On 07/28/2016 09:33 AM, Senthil Kumar Selvaraj wrote:
>>
>> Is there a reason why only REG and SYMBOL_REFs get valid
>> subreg_in_class? I tried extending it handle constants and PLUS
>> expressions, and it fixes PR 71873. It also fixes a another
>> bug that was a work around for the reload failure (PR 64452) - that
>> had a plus expression instead of the const.
>>
>> Reg testing on avr and x86_64 did not show any new failures. Is this
>> the right way to fix this?
>
> I think it looks quite plausible. Note that testing x86_64 on trunk will
> not do anything - it is no longer using reload. Could you go back to an
> older branch (4.7 I think is the last one using reload) and retest
> x86_64 with that, for better test coverage?
I picked out the commit where you'd added SYMBOL_REF handling (rev
#190252), and patched that with the below code. Bootstrapped and
regtested on x86_64-pc-linux - the results were identical with and
without the patch. Is this good enough for trunk?
Regards
Senthil
gcc/ChangeLog:
2016-08-08 Senthil Kumar Selvaraj
PR reload/71873
* reload.c (push_reload): Compute subreg_in_class for
subregs of constants and plus expressions.
gcc/testsuite/ChangeLog:
2016-08-08 Senthil Kumar Selvaraj
* gcc.target/avr/pr71873.c: New test.
Index: gcc/reload.c
===
--- gcc/reload.c(revision 239239)
+++ gcc/reload.c(working copy)
@@ -1141,7 +1141,9 @@
SUBREG_BYTE (in),
GET_MODE (in)),
REGNO (SUBREG_REG (in)));
- else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
+ else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF
+ || CONSTANT_P (SUBREG_REG (in))
+ || GET_CODE (SUBREG_REG (in)) == PLUS)
subreg_in_class = find_valid_class_1 (inmode,
GET_MODE (SUBREG_REG (in)),
rclass);
Index: gcc/testsuite/gcc.target/avr/pr71873.c
===
--- gcc/testsuite/gcc.target/avr/pr71873.c (nonexistent)
+++ gcc/testsuite/gcc.target/avr/pr71873.c (working copy)
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -fcaller-saves" } */
+
+#include
+
+typedef struct {
+ uint8_t x;
+ uint32_t y;
+} A;
+
+A a;
+
+extern int bar(int);
+extern int foo (char *s, ...);
+
+extern uint8_t param;
+extern uint8_t h,m,s,ld,lm;
+extern uint16_t d;
+
+void gps_parse_string(int z)
+{
+ while (bar(z))
+ {
+switch (param)
+{
+ case 0: foo("a", &h, &m, &s, &d); break;
+ case 1: foo("d", &ld, &lm, &a.y); break;
+}
+ }
+}
Re: [Patch, tentative, reload] Make push_reload work with more types of subregs?
Bernd Schmidt writes:
> On 08/08/2016 05:26 PM, Senthil Kumar Selvaraj wrote:
>
>> I picked out the commit where you'd added SYMBOL_REF handling (rev
>> #190252), and patched that with the below code. Bootstrapped and
>> regtested on x86_64-pc-linux - the results were identical with and
>> without the patch. Is this good enough for trunk?
>
>> - else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
>> + else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF
>> + || CONSTANT_P (SUBREG_REG (in))
>> + || GET_CODE (SUBREG_REG (in)) == PLUS)
>> subreg_in_class = find_valid_class_1 (inmode,
>>GET_MODE (SUBREG_REG (in)),
>>rclass);
>
> Actually SYMBOL_REF should also be CONSTANT_P. For integers it looks
> like it'll pass VOIDmode to find_valid_class_1 and just return NO_REGS.
> which I suppose is OK.
>
> Would you mind removing the SYMBOL_REF test and retesting? Ok with that
> change.
Bootstrapped and reg tested below patch with same setup as above - no
regressions showed up.
Committed patch to trunk. Ok for backport to 6.x and 5.x branches as
well?
Regards
Senthil
gcc/ChangeLog
2016-08-10 Senthil Kumar Selvaraj
PR target/71873
* reload.c (push_reload): Compute subreg_in_class for
subregs of constants and plus expressions. Remove special
handling of SYMBOL_REFs.
gcc/testsuite/ChangeLog
2016-08-10 Senthil Kumar Selvaraj
PR target/71873
* gcc.target/avr/pr71873.c: New test.
Index: gcc/reload.c
===
--- gcc/reload.c(revision 239318)
+++ gcc/reload.c(working copy)
@@ -1141,7 +1141,8 @@
SUBREG_BYTE (in),
GET_MODE (in)),
REGNO (SUBREG_REG (in)));
- else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
+ else if (CONSTANT_P (SUBREG_REG (in))
+ || GET_CODE (SUBREG_REG (in)) == PLUS)
subreg_in_class = find_valid_class_1 (inmode,
GET_MODE (SUBREG_REG (in)),
rclass);
Index: gcc/testsuite/gcc.target/avr/pr71873.c
===
--- gcc/testsuite/gcc.target/avr/pr71873.c (nonexistent)
+++ gcc/testsuite/gcc.target/avr/pr71873.c (working copy)
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -fcaller-saves" } */
+
+#include
+
+typedef struct {
+ uint8_t x;
+ uint32_t y;
+} A;
+
+A a;
+
+extern int bar(int);
+extern int foo (char *s, ...);
+
+extern uint8_t param;
+extern uint8_t h,m,s,ld,lm;
+extern uint16_t d;
+
+void gps_parse_string(int z)
+{
+ while (bar(z))
+ {
+switch (param)
+{
+ case 0: foo("a", &h, &m, &s, &d); break;
+ case 1: foo("d", &ld, &lm, &a.y); break;
+}
+ }
+}
[Patch, testsuite] Skip tests that expect 4 byte alignment for avr
Hi,
The below patch adds the AVR target to the list of targets that don't
have natural_alignment_32. It also skips ipa/propalign-*.c
tests (which expect 4 byte alignment), if both
natural_alignment_32 and natural_alignment_64 are false.
Is this the right way to fix this? Ok to commit?
Regards
Senthil
gcc/testsuite/ChangeLog:
2016-08-11 Senthil Kumar Selvaraj
* gcc.dg/ipa/propalign-1.c: Skip for targets with !natural_alignment_32
and !natural_alignment_64.
* gcc.dg/ipa/propalign-2.c: Likewise.
* gcc.dg/ipa/propalign-3.c: Likewise.
* gcc.dg/ipa/propalign-4.c: Likewise.
* gcc.dg/ipa/propalign-5.c: Likewise.
* lib/target-supports.exp
(check_effective_target_natural_alignment_32): Add avr-*-*.
Index: gcc/testsuite/gcc.dg/ipa/propalign-1.c
===
--- gcc/testsuite/gcc.dg/ipa/propalign-1.c (revision 239318)
+++ gcc/testsuite/gcc.dg/ipa/propalign-1.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fno-early-inlining -fdump-ipa-cp -fdump-tree-optimized"
} */
+/* { dg-skip-if "No alignment restrictions" { { ! natural_alignment_32 } && {
! natural_alignment_64 } } } */
#include
Index: gcc/testsuite/gcc.dg/ipa/propalign-2.c
===
--- gcc/testsuite/gcc.dg/ipa/propalign-2.c (revision 239318)
+++ gcc/testsuite/gcc.dg/ipa/propalign-2.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fno-early-inlining -fdump-ipa-cp -fdump-tree-optimized"
} */
+/* { dg-skip-if "No alignment restrictions" { { ! natural_alignment_32 } && {
! natural_alignment_64 } } } */
#include
Index: gcc/testsuite/gcc.dg/ipa/propalign-3.c
===
--- gcc/testsuite/gcc.dg/ipa/propalign-3.c (revision 239318)
+++ gcc/testsuite/gcc.dg/ipa/propalign-3.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fno-ipa-cp-alignment -fno-early-inlining -fdump-ipa-cp
-fdump-tree-optimized" } */
+/* { dg-skip-if "No alignment restrictions" { { ! natural_alignment_32 } && {
! natural_alignment_64 } } } */
#include
Index: gcc/testsuite/gcc.dg/ipa/propalign-4.c
===
--- gcc/testsuite/gcc.dg/ipa/propalign-4.c (revision 239318)
+++ gcc/testsuite/gcc.dg/ipa/propalign-4.c (working copy)
@@ -1,5 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-ipa-cp" } */
+/* { dg-skip-if "No alignment restrictions" { { ! natural_alignment_32 } && {
! natural_alignment_64 } } } */
+
int n;
static void
Index: gcc/testsuite/gcc.dg/ipa/propalign-5.c
===
--- gcc/testsuite/gcc.dg/ipa/propalign-5.c (revision 239318)
+++ gcc/testsuite/gcc.dg/ipa/propalign-5.c (working copy)
@@ -1,5 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-ipa-cp" } */
+/* { dg-skip-if "No alignment restrictions" { { ! natural_alignment_32 } && {
! natural_alignment_64 } } } */
+
int n;
static void
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp (revision 239318)
+++ gcc/testsuite/lib/target-supports.exp (working copy)
@@ -5221,7 +5221,8 @@
} else {
# FIXME: 32bit powerpc: guaranteed only if MASK_ALIGN_NATURAL/POWER.
set et_natural_alignment_32_saved 1
-if { ([istarget *-*-darwin*] && [is-effective-target lp64]) } {
+if { ([istarget *-*-darwin*] && [is-effective-target lp64])
+ || [istarget avr-*-*] } {
set et_natural_alignment_32_saved 0
}
}
Re: [Patch, testsuite] Skip tests that expect 4 byte alignment for avr
Jeff Law writes: > On 08/11/2016 01:40 AM, Senthil Kumar Selvaraj wrote: >> Hi, >> >> The below patch adds the AVR target to the list of targets that don't >> have natural_alignment_32. It also skips ipa/propalign-*.c >> tests (which expect 4 byte alignment), if both >> natural_alignment_32 and natural_alignment_64 are false. >> >> Is this the right way to fix this? Ok to commit? >> >> Regards >> Senthil >> >> gcc/testsuite/ChangeLog: >> >> 2016-08-11 Senthil Kumar Selvaraj >> >> * gcc.dg/ipa/propalign-1.c: Skip for targets with !natural_alignment_32 >> and !natural_alignment_64. >> * gcc.dg/ipa/propalign-2.c: Likewise. >> * gcc.dg/ipa/propalign-3.c: Likewise. >> * gcc.dg/ipa/propalign-4.c: Likewise. >> * gcc.dg/ipa/propalign-5.c: Likewise. >> * lib/target-supports.exp >> (check_effective_target_natural_alignment_32): Add avr-*-*. > Do you need to add an avr case to > check_effective_target_natural_alignment_64 as well? The 64 bit version is written to return true only for lp64 and spu-*-*, so I didn't have to do anything there. > > Have you tested this on anything other than avr? > Yes, x86_64-pc-linux passes these tests, like it did before my patch. Regards Senthil
[Patch, testsuite] Fix more bogus test failures for avr
Hi,
This patch fixes a couple of testsuite failures for the avr target by
requiring int32plus support for zero_sign_ext_test.c, and explicitly
using __UINT32_TYPE__ instead of assuming 32 bit ints for pr71083.c
Regtested with avr and x86_64-pc-linux. Committed to trunk.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-08-24 Senthil Kumar Selvaraj
* gcc.c-torture/execute/pr71083.c: Use UINT32_TYPE instead
of unsigned int.
* gcc.dg/zero_sign_ext_test.c: Require int32plus.
Index: gcc/testsuite/gcc.c-torture/execute/pr71083.c
===
--- gcc/testsuite/gcc.c-torture/execute/pr71083.c (revision 239731)
+++ gcc/testsuite/gcc.c-torture/execute/pr71083.c (working copy)
@@ -1,5 +1,7 @@
+__extension__ typedef __UINT32_TYPE__ uint32_t;
+
struct lock_chain {
- unsigned int irq_context: 2,
+ uint32_t irq_context: 2,
depth: 6,
base: 24;
};
Index: gcc/testsuite/gcc.dg/zero_sign_ext_test.c
===
--- gcc/testsuite/gcc.dg/zero_sign_ext_test.c (revision 239731)
+++ gcc/testsuite/gcc.dg/zero_sign_ext_test.c (working copy)
@@ -2,6 +2,7 @@
/* { dg-options "-O2" } */
/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
#define TYPE_MAX(type, sign) \
((!sign) ? ((1 << (sizeof (type) * 8 - 1)) - 1) :\
[Patch, testsuite] Fix more bogus failures for avr
Hi,
The below patch fixes some bogus testsuite failures for the avr
target. The first three expect 32 bit ints, and the last one uses too
much RAM for a typical avr device.
Regtested with avr and x86_64-pc-linux, and committed to trunk.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-09-01 Senthil Kumar Selvaraj
* gcc.dg/pr64252.c: Require int32plus.
* gcc.dg/pr66299-1.c: Likewise.
* gcc.dg/pr66299-2.c: Likewise.
* gcc.dg/torture/20131115-1.c: Skip for avr.
Index: gcc/testsuite/gcc.dg/pr64252.c
===
--- gcc/testsuite/gcc.dg/pr64252.c (revision 239920)
+++ gcc/testsuite/gcc.dg/pr64252.c (working copy)
@@ -1,6 +1,7 @@
/* PR target/64252 */
/* { dg-do run } */
/* { dg-options "-O2" } */
+/* { dg-require-effective-target int32plus } */
typedef unsigned int V __attribute__((vector_size (32)));
Index: gcc/testsuite/gcc.dg/pr66299-1.c
===
--- gcc/testsuite/gcc.dg/pr66299-1.c(revision 239920)
+++ gcc/testsuite/gcc.dg/pr66299-1.c(working copy)
@@ -1,6 +1,7 @@
/* PR tree-optimization/66299 */
/* { dg-do run } */
/* { dg-options "-fdump-tree-original" } */
+/* { dg-require-effective-target int32plus } */
void
test1 (int x)
Index: gcc/testsuite/gcc.dg/pr66299-2.c
===
--- gcc/testsuite/gcc.dg/pr66299-2.c(revision 239920)
+++ gcc/testsuite/gcc.dg/pr66299-2.c(working copy)
@@ -1,6 +1,7 @@
/* PR tree-optimization/66299 */
/* { dg-do run } */
/* { dg-options "-fdump-tree-optimized -O" } */
+/* { dg-require-effective-target int32plus } */
void
test1 (int x, unsigned u)
Index: gcc/testsuite/gcc.dg/torture/20131115-1.c
===
--- gcc/testsuite/gcc.dg/torture/20131115-1.c (revision 239920)
+++ gcc/testsuite/gcc.dg/torture/20131115-1.c (working copy)
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-skip-if "RAM usage too large" { "avr-*-*" } } */
struct S { int i; };
__attribute__((const, noinline, noclone))
[Patch, testsuite, avr] Skip gcc.dg/Wno-frame-address.c for avr
Hi,
The below patch adds avr to the list of targets excluded for
Wno-frame-address.c. Like the other excluded targets, the avr backend
only supports the builtin for the current stack frame.
Committed to trunk.
Regards
Senthil
gcc/testsuite/ChangeLog:
2016-09-06 Senthil Kumar Selvaraj
* gcc.dg/Wno-frame-address.c: Skip for avr-*-*.
Index: gcc/testsuite/gcc.dg/Wno-frame-address.c
===
--- gcc/testsuite/gcc.dg/Wno-frame-address.c(revision 240004)
+++ gcc/testsuite/gcc.dg/Wno-frame-address.c(working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-skip-if "Cannot access arbitrary stack frames" { arm*-*-* hppa*-*-*
ia64-*-* visium-*-* } } */
+/* { dg-skip-if "Cannot access arbitrary stack frames" { arm*-*-* avr-*-*
hppa*-*-* ia64-*-* visium-*-* } } */
/* { dg-options "-Werror" } */
/* { dg-additional-options "-mbackchain" { target { s390*-*-* } } } */
[Patch, testsuite] Make some more tests xfail/pass/unsupported for avr
Hi,
This patch attempts to fix some bogus testsuite failures by either
slightly modifying the test, adding xfail for int16 targets, or by
using the appropriate dg-require-effective-target/dg-skip-if directive.
The only tricky changes I see are
* changing usage of unsigned long to __SIZE_TYPE__ aka size_t for a
couple of tests (arg type of malloc in Warray-bounds-11.c
and in a typedef for size_t in pr62090-2.c). This makes the tests pass
for avr and x86_64.
* Skipping pr70169.c for avr, as the test doesn't appear valid for a
modified Harvard architecture. The compiler warns that the pointer is
attempting to access data memory with a program memory address, and the
extra warning causes the test to fail.
Regtesting with x86_64 showed no new regressions. The avr target of course
shows a bunch of new PASS/UNSUPPORTED/XFAILs.
If ok, could someone commit please? I don't have commit access.
Regards
Senthil
2016-05-25 Senthil Kumar Selvaraj
* c-c++-common/Wduplicated-cond-1.c: Use smaller const literal.
* c-c++-common/pr60226.c: Require int32plus.
* gcc.c-torture/execute/pr70602.c: Likewise.
* gcc.dg/Warray-bounds-11.c: Use __SIZE_TYPE__ instead of
unsigned long for malloc arg type.
* gcc.dg/asr_div1.c: Require int32plus.
* gcc.dg/enum-mode-1.c: XFAIL for int16.
* gcc.dg/pie-1.c: Require pie.
* gcc.dg/pie-2.c: Likewise.
* gcc.dg/pr59471.c: Require int32plus.
* gcc.dg/pr59963-2.c: XFAIL for int16.
* gcc.dg/pr60114.c: Require int32plus.
* gcc.dg/pr62090-2.c: Use __SIZE_TYPE__ instead of
unsigned long for typedef of size_t.
* gcc.dg/pr63914.c: Require int32plus.
* gcc.dg/pr64536.c: Require pt32plus.
* gcc.dg/pr65658.c: Likewise.
* gcc.dg/pr67271.c: Require int32plus.
* gcc.dg/pr68112.c: Likewise.
* gcc.dg/pr69071.c: Skip for avr target.
* gcc.dg/pr69973.c: Require int32plus.
* gcc.dg/pr70169.c: Skip for avr target.
* gcc.dg/sso-6.c: Require int32plus.
* gcc.dg/sso-7.c: Likewise.
* gcc.dg/sso-8.c: Likewise.
* gcc.dg/vrp-min-max-2.c: Likewise.
diff --git gcc/testsuite/c-c++-common/Wduplicated-cond-1.c
gcc/testsuite/c-c++-common/Wduplicated-cond-1.c
index 4763a84..179c419 100644
--- gcc/testsuite/c-c++-common/Wduplicated-cond-1.c
+++ gcc/testsuite/c-c++-common/Wduplicated-cond-1.c
@@ -183,7 +183,7 @@ int
fn10 (void)
{
if (foo ())
-return 1732984;
+return 17329;
else if (foo ())
return 18409;
return 0;
diff --git gcc/testsuite/c-c++-common/pr60226.c
gcc/testsuite/c-c++-common/pr60226.c
index 6a686d7..aeb132b8 100644
--- gcc/testsuite/c-c++-common/pr60226.c
+++ gcc/testsuite/c-c++-common/pr60226.c
@@ -1,6 +1,7 @@
/* PR c/60226 */
/* { dg-do compile } */
/* { dg-options "-Wno-c++-compat" { target c } } */
+/* { dg-require-effective-target int32plus } */
typedef int __attribute__ ((aligned (1 << 28))) int28;
int28 foo[4] = {}; /* { dg-error "alignment of array elements is greater than
element size" } */
diff --git gcc/testsuite/gcc.c-torture/execute/pr70602.c
gcc/testsuite/gcc.c-torture/execute/pr70602.c
index c6628b0..4a931fc 100644
--- gcc/testsuite/gcc.c-torture/execute/pr70602.c
+++ gcc/testsuite/gcc.c-torture/execute/pr70602.c
@@ -1,4 +1,5 @@
/* PR tree-optimization/70602 */
+/* { dg-require-effective-target int32plus } */
struct __attribute__((packed)) S
{
diff --git gcc/testsuite/gcc.dg/Warray-bounds-11.c
gcc/testsuite/gcc.dg/Warray-bounds-11.c
index 92e92c0..089fa00 100644
--- gcc/testsuite/gcc.dg/Warray-bounds-11.c
+++ gcc/testsuite/gcc.dg/Warray-bounds-11.c
@@ -1,7 +1,8 @@
/* { dg-do compile } */
/* { dg-options "-O3 -Warray-bounds=2" } */
-extern void* malloc(unsigned long x);
+typedef __SIZE_TYPE__ size_t;
+extern void* malloc(size_t x);
int e[3];
diff --git gcc/testsuite/gcc.dg/asr_div1.c gcc/testsuite/gcc.dg/asr_div1.c
index 11b5246..bf374b8 100644
--- gcc/testsuite/gcc.dg/asr_div1.c
+++ gcc/testsuite/gcc.dg/asr_div1.c
@@ -1,6 +1,7 @@
/* Test division by const int generates only one shift. */
/* { dg-do run } */
/* { dg-options "-O2 -fdump-rtl-combine-all" } */
+/* { dg-require-effective-target int32plus } */
extern void abort (void);
diff --git gcc/testsuite/gcc.dg/enum-mode-1.c gcc/testsuite/gcc.dg/enum-mode-1.c
index a701123..09276b7 100644
--- gcc/testsuite/gcc.dg/enum-mode-1.c
+++ gcc/testsuite/gcc.dg/enum-mode-1.c
@@ -6,5 +6,5 @@ enum e2 { B = 256 } __attribute__((__packed__,
__mode__(__byte__))); /* { dg-err
enum e3 { C = __INT_MAX__ } __attribute__((__mode__(__QI__))); /* { dg-error
"specified mode too small for enumeral values" } */
enum e4 { D = __INT_MAX__ } __attribute__((__packed__, __mode__(__QI__))); /*
{ dg-error "specified mode too small for enumeral values" } */
-enum e5 { E = __INT_MAX__ } __attribute__((__mode__(__HI__))); /*
[Patch, avr] Fix PR 71151
Hi,
This patch fixes PR 71151 by eliminating the
TARGET_ASM_FUNCTION_RODATA_SECTION hook and setting
JUMP_TABLES_IN_TEXT_SECTION to 1.
As described in the bugzilla entry, this hook assumed it will get
called only for jumptable rodata for functions. This was true until
6.1, when a commit in varasm.c started calling the hook for mergeable
string/constant data as well.
This resulted in string constants ending up in a section intended for
jumptables (flash), and broke code using those constants, which
expects them to be present in rodata (SRAM).
Given that the original reason for placing jumptables in a section was
fixed by Johann in PR 63323, this patch restores the original
behavior. Reg testing on both gcc-6-branch and trunk showed no regressions.
As pointed out by Johann, this may end up increasing code
size if there are lots of branches that cross the jump tables. I
intend to propose a separate patch that gives additional information
to the target hook (SECCAT_RODATA_{STRING,JUMPTABLE}) so it can know
what type of function rodata is coming on. Johann also suggested
handling jump table generation ourselves - I'll experiment with that
some more.
If ok, could someone commit please? Could you also backport to
gcc-6-branch?
Regards
Senthil
gcc/ChangeLog
2016-06-03 Senthil Kumar Selvaraj
* config/avr/avr.c (avr_asm_function_rodata_section): Remove.
* config/avr/avr.c (TARGET_ASM_FUNCTION_RODATA_SECTION): Remove.
gcc/testsuite/ChangeLog
2016-06-03 Senthil Kumar Selvaraj
* gcc/testsuite/gcc.target/avr/pr71151-1.c: New.
* gcc/testsuite/gcc.target/avr/pr71151-2.c: New.
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index ba5cd91..3cb8cb7 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -9488,65 +9488,6 @@ avr_asm_init_sections (void)
}
-/* Implement `TARGET_ASM_FUNCTION_RODATA_SECTION'. */
-
-static section*
-avr_asm_function_rodata_section (tree decl)
-{
- /* If a function is unused and optimized out by -ffunction-sections
- and --gc-sections, ensure that the same will happen for its jump
- tables by putting them into individual sections. */
-
- unsigned int flags;
- section * frodata;
-
- /* Get the frodata section from the default function in varasm.c
- but treat function-associated data-like jump tables as code
- rather than as user defined data. AVR has no constant pools. */
- {
-int fdata = flag_data_sections;
-
-flag_data_sections = flag_function_sections;
-frodata = default_function_rodata_section (decl);
-flag_data_sections = fdata;
-flags = frodata->common.flags;
- }
-
- if (frodata != readonly_data_section
- && flags & SECTION_NAMED)
-{
- /* Adjust section flags and replace section name prefix. */
-
- unsigned int i;
-
- static const char* const prefix[] =
-{
- ".rodata", ".progmem.gcc_sw_table",
- ".gnu.linkonce.r.", ".gnu.linkonce.t."
-};
-
- for (i = 0; i < sizeof (prefix) / sizeof (*prefix); i += 2)
-{
- const char * old_prefix = prefix[i];
- const char * new_prefix = prefix[i+1];
- const char * name = frodata->named.name;
-
- if (STR_PREFIX_P (name, old_prefix))
-{
- const char *rname = ACONCAT ((new_prefix,
-name + strlen (old_prefix), NULL));
- flags &= ~SECTION_CODE;
- flags |= AVR_HAVE_JMP_CALL ? 0 : SECTION_CODE;
-
- return get_section (rname, flags, frodata->named.decl);
-}
-}
-}
-
- return progmem_swtable_section;
-}
-
-
/* Implement `TARGET_ASM_NAMED_SECTION'. */
/* Track need of __do_clear_bss, __do_copy_data for named sections. */
@@ -13747,9 +13688,6 @@ avr_fold_builtin (tree fndecl, int n_args
ATTRIBUTE_UNUSED, tree *arg,
#undef TARGET_FOLD_BUILTIN
#define TARGET_FOLD_BUILTIN avr_fold_builtin
-#undef TARGET_ASM_FUNCTION_RODATA_SECTION
-#define TARGET_ASM_FUNCTION_RODATA_SECTION avr_asm_function_rodata_section
-
#undef TARGET_SCALAR_MODE_SUPPORTED_P
#define TARGET_SCALAR_MODE_SUPPORTED_P avr_scalar_mode_supported_p
diff --git gcc/config/avr/avr.h gcc/config/avr/avr.h
index 01da708..ab5e465 100644
--- gcc/config/avr/avr.h
+++ gcc/config/avr/avr.h
@@ -391,7 +391,7 @@ typedef struct avr_args
#define SUPPORTS_INIT_PRIORITY 0
-#define JUMP_TABLES_IN_TEXT_SECTION 0
+#define JUMP_TABLES_IN_TEXT_SECTION 1
#define ASM_COMMENT_START " ; "
diff --git gcc/testsuite/gcc.target/avr/pr71151-1.c
gcc/testsuite/gcc.target/avr/pr71151-1.c
new file mode 100644
index 000..615dce8
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr71151-1.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -ffunction-sections -fdata-sections" } */
+
+/* { dg-final { s
Re: [Patch, avr] Fix PR 71151
Georg-Johann Lay writes:
> Senthil Kumar Selvaraj schrieb:
>> Hi,
>>
>> This patch fixes PR 71151 by eliminating the
>> TARGET_ASM_FUNCTION_RODATA_SECTION hook and setting
>> JUMP_TABLES_IN_TEXT_SECTION to 1.
>>
>> As described in the bugzilla entry, this hook assumed it will get
>> called only for jumptable rodata for functions. This was true until
>> 6.1, when a commit in varasm.c started calling the hook for mergeable
>> string/constant data as well.
>>
>> This resulted in string constants ending up in a section intended for
>> jumptables (flash), and broke code using those constants, which
>> expects them to be present in rodata (SRAM).
>>
>> Given that the original reason for placing jumptables in a section was
>> fixed by Johann in PR 63323, this patch restores the original
>> behavior. Reg testing on both gcc-6-branch and trunk showed no regressions.
>
> Just for the record:
>
> The intention for jump-tables in function-rodata-section was to get
> fine-grained section for the tables so that --gc-sections and
> -ffunction-sections not only gc unused functions but also unused
> jump-tables. As these tables had to reside in the lowest 64KiB of flash
> (.progmem section) neither .rodata nor .text was a correct placement,
> hence the hacking in TARGET_ASM_FUNCTION_RODATA_SECTION.
>
> Before using TARGET_ASM_FUNCTION_RODATA_SECTION, all jump tables were
> put into .progmem.gcc_sw_table by ASM_OUTPUT_BEFORE_CASE_LABEL switching
> to that section.
>
> We actually never had fump-tables in .text before...
JUMP_TABLES_IN_TEXT_SECTION was 1 before r37465 - that was when the
progmem.gcc_sw_table section was introduced. But yes, I understand that
the target hook for FUNCTION_RODATA_SECTION was done to get them gc'ed
along with the code.
>
> The purpose of PR63323 was to have more generic jump-table
> implementation that also works if the table does NOT reside in the lower
> 64KiB. This happens when moving whole whole TEXT section around like
> for a bootloader.
Understood.
>
>> As pointed out by Johann, this may end up increasing code
>> size if there are lots of branches that cross the jump tables. I
>> intend to propose a separate patch that gives additional information
>> to the target hook (SECCAT_RODATA_{STRING,JUMPTABLE}) so it can know
>> what type of function rodata is coming on. Johann also suggested
>> handling jump table generation ourselves - I'll experiment with that
>> some more.
>>
>> If ok, could someone commit please? Could you also backport to
>> gcc-6-branch?
>>
>> Regards
>> Senthil
>>
>> gcc/ChangeLog
>>
>> 2016-06-03 Senthil Kumar Selvaraj
>>
>> * config/avr/avr.c (avr_asm_function_rodata_section): Remove.
>> * config/avr/avr.c (TARGET_ASM_FUNCTION_RODATA_SECTION): Remove.
>>
>> gcc/testsuite/ChangeLog
>>
>> 2016-06-03 Senthil Kumar Selvaraj
>>
>> * gcc/testsuite/gcc.target/avr/pr71151-1.c: New.
>> * gcc/testsuite/gcc.target/avr/pr71151-2.c: New.
>>
>> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
>> index ba5cd91..3cb8cb7 100644
>> --- gcc/config/avr/avr.c
>> +++ gcc/config/avr/avr.c
>> @@ -9488,65 +9488,6 @@ avr_asm_init_sections (void)
>> }
>>
>>
>> -/* Implement `TARGET_ASM_FUNCTION_RODATA_SECTION'. */
>> -
>> -static section*
>> -avr_asm_function_rodata_section (tree decl)
>> -{
>> - /* If a function is unused and optimized out by -ffunction-sections
>> - and --gc-sections, ensure that the same will happen for its jump
>> - tables by putting them into individual sections. */
>> -
>> - unsigned int flags;
>> - section * frodata;
>> -
>> - /* Get the frodata section from the default function in varasm.c
>> - but treat function-associated data-like jump tables as code
>> - rather than as user defined data. AVR has no constant pools. */
>> - {
>> -int fdata = flag_data_sections;
>> -
>> -flag_data_sections = flag_function_sections;
>> -frodata = default_function_rodata_section (decl);
>> -flag_data_sections = fdata;
>> -flags = frodata->common.flags;
>> - }
>> -
>> - if (frodata != readonly_data_section
>> - && flags & SECTION_NAMED)
>> -{
>> - /* Adjust section flags and replace section name prefix. */
>> -
>> - unsigned int i;
>> -
>> - static const char* const prefix[] =
>> -{
&g
[Patch, avr] Fix broken stack-usage-1.c test
Hi, A recent patch I submitted fixed broken -fstack-usage for the avr target, by including the size of the return address pushed to the stack (https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01715.html). I forgot to send this testcase modification with that patch - here's the fix for making gcc.dg/stack-usage-1.c pass again for avr. If this is ok, could someone commit please? I don't have commit access. Regards Senthil gcc/testsuite/ChangeLog 2016-06-08 Senthil Kumar Selvaraj * gcc.dg/stack-usage-1.c (SIZE): Consider return address when setting SIZE. diff --git gcc/testsuite/gcc.dg/stack-usage-1.c gcc/testsuite/gcc.dg/stack-usage-1.c index 7864c6a..bdc5656 100644 --- gcc/testsuite/gcc.dg/stack-usage-1.c +++ gcc/testsuite/gcc.dg/stack-usage-1.c @@ -64,7 +64,11 @@ #define SIZE 240 # endif #elif defined (__AVR__) -# define SIZE 254 +#if defined (__AVR_3_BYTE_PC__ ) +# define SIZE 251 /* 256 - 2 bytes for Y - 3 bytes for return address */ +#else +# define SIZE 252 /* 256 - 2 bytes for Y - 2 bytes for return address */ +#endif #elif defined (__s390x__) # define SIZE 96 /* 256 - 160 bytes for register save area */ #elif defined (__s390__)
[Patch, testsuite] Skip some more tests for targets with int size < 32
Hi,
This patch requires int32plus support for a few more tests - these
were failing for the avr target.
bswap-2.c uses left shifts wider than 16 bits on a char, and
pr68067-{1,2} use an out of range negative number (INT_MIN for 32 bit int).
If this is ok, could someone commit please? I don't have commit access.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-06-08 Senthil Kumar Selvaraj
* gcc.c-torture/execute/bswap-2.c: Require int32plus.
* gcc.dg/torture/pr68067-1.c: Likewise.
* gcc.dg/torture/pr68067-2.c: Likewise.
diff --git gcc/testsuite/gcc.c-torture/execute/bswap-2.c
gcc/testsuite/gcc.c-torture/execute/bswap-2.c
index 88132fe..63e7807 100644
--- gcc/testsuite/gcc.c-torture/execute/bswap-2.c
+++ gcc/testsuite/gcc.c-torture/execute/bswap-2.c
@@ -1,3 +1,5 @@
+/* { dg-require-effective-target int32plus } */
+
#ifdef __UINT32_TYPE__
typedef __UINT32_TYPE__ uint32_t;
#else
diff --git gcc/testsuite/gcc.dg/torture/pr68067-1.c
gcc/testsuite/gcc.dg/torture/pr68067-1.c
index a7b6aa0..f8ad3ca 100644
--- gcc/testsuite/gcc.dg/torture/pr68067-1.c
+++ gcc/testsuite/gcc.dg/torture/pr68067-1.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
int main()
{
diff --git gcc/testsuite/gcc.dg/torture/pr68067-2.c
gcc/testsuite/gcc.dg/torture/pr68067-2.c
index 38a459b..e03bf22 100644
--- gcc/testsuite/gcc.dg/torture/pr68067-2.c
+++ gcc/testsuite/gcc.dg/torture/pr68067-2.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
int main()
{
--
2.7.4
Re: [Patch, avr] Fix PR 71151
Senthil Kumar Selvaraj writes:
> Georg-Johann Lay writes:
>
>> Senthil Kumar Selvaraj schrieb:
>>> Hi,
>>>
>>> This patch fixes PR 71151 by eliminating the
>>> TARGET_ASM_FUNCTION_RODATA_SECTION hook and setting
>>> JUMP_TABLES_IN_TEXT_SECTION to 1.
>>>
>>> As described in the bugzilla entry, this hook assumed it will get
>>> called only for jumptable rodata for functions. This was true until
>>> 6.1, when a commit in varasm.c started calling the hook for mergeable
>>> string/constant data as well.
>>>
>>> This resulted in string constants ending up in a section intended for
>>> jumptables (flash), and broke code using those constants, which
>>> expects them to be present in rodata (SRAM).
>>>
>>> Given that the original reason for placing jumptables in a section was
>>> fixed by Johann in PR 63323, this patch restores the original
>>> behavior. Reg testing on both gcc-6-branch and trunk showed no
>>> regressions.
>>
>> Just for the record:
>>
>> The intention for jump-tables in function-rodata-section was to get
>> fine-grained section for the tables so that --gc-sections and
>> -ffunction-sections not only gc unused functions but also unused
>> jump-tables. As these tables had to reside in the lowest 64KiB of flash
>> (.progmem section) neither .rodata nor .text was a correct placement,
>> hence the hacking in TARGET_ASM_FUNCTION_RODATA_SECTION.
>>
>> Before using TARGET_ASM_FUNCTION_RODATA_SECTION, all jump tables were
>> put into .progmem.gcc_sw_table by ASM_OUTPUT_BEFORE_CASE_LABEL switching
>> to that section.
>>
>> We actually never had fump-tables in .text before...
>
> JUMP_TABLES_IN_TEXT_SECTION was 1 before r37465 - that was when the
> progmem.gcc_sw_table section was introduced. But yes, I understand that
> the target hook for FUNCTION_RODATA_SECTION was done to get them gc'ed
> along with the code.
>
>>
>> The purpose of PR63323 was to have more generic jump-table
>> implementation that also works if the table does NOT reside in the lower
>> 64KiB. This happens when moving whole whole TEXT section around like
>> for a bootloader.
>
> Understood.
>>
>>> As pointed out by Johann, this may end up increasing code
>>> size if there are lots of branches that cross the jump tables. I
>>> intend to propose a separate patch that gives additional information
>>> to the target hook (SECCAT_RODATA_{STRING,JUMPTABLE}) so it can know
>>> what type of function rodata is coming on. Johann also suggested
>>> handling jump table generation ourselves - I'll experiment with that
>>> some more.
>>>
>>> If ok, could someone commit please? Could you also backport to
>>> gcc-6-branch?
>>>
>>> Regards
>>> Senthil
>>>
>>> gcc/ChangeLog
>>>
>>> 2016-06-03 Senthil Kumar Selvaraj
>>>
>>> * config/avr/avr.c (avr_asm_function_rodata_section): Remove.
>>> * config/avr/avr.c (TARGET_ASM_FUNCTION_RODATA_SECTION): Remove.
>>>
>>> gcc/testsuite/ChangeLog
>>>
>>> 2016-06-03 Senthil Kumar Selvaraj
>>>
>>> * gcc/testsuite/gcc.target/avr/pr71151-1.c: New.
>>> * gcc/testsuite/gcc.target/avr/pr71151-2.c: New.
>>>
>>> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
>>> index ba5cd91..3cb8cb7 100644
>>> --- gcc/config/avr/avr.c
>>> +++ gcc/config/avr/avr.c
>>> @@ -9488,65 +9488,6 @@ avr_asm_init_sections (void)
>>> }
>>>
>>>
>>> -/* Implement `TARGET_ASM_FUNCTION_RODATA_SECTION'. */
>>> -
>>> -static section*
>>> -avr_asm_function_rodata_section (tree decl)
>>> -{
>>> - /* If a function is unused and optimized out by -ffunction-sections
>>> - and --gc-sections, ensure that the same will happen for its jump
>>> - tables by putting them into individual sections. */
>>> -
>>> - unsigned int flags;
>>> - section * frodata;
>>> -
>>> - /* Get the frodata section from the default function in varasm.c
>>> - but treat function-associated data-like jump tables as code
>>> - rather than as user defined data. AVR has no constant pools. */
>>> - {
>>> -int fdata = flag_data_sections;
>>> -
>>> -flag_data_sections = flag_function_sections;
>>>
Re: [Patch, avr] Fix PR 71151
Denis Chertykov writes:
> 2016-06-16 10:27 GMT+03:00 Senthil Kumar Selvaraj
> :
>>
>> Senthil Kumar Selvaraj writes:
>>
>>> Georg-Johann Lay writes:
>>>
>>>> Senthil Kumar Selvaraj schrieb:
>>>>> Hi,
>>>>>
>>>>> This patch fixes PR 71151 by eliminating the
>>>>> TARGET_ASM_FUNCTION_RODATA_SECTION hook and setting
>>>>> JUMP_TABLES_IN_TEXT_SECTION to 1.
>>>>>
>>>>> As described in the bugzilla entry, this hook assumed it will get
>>>>> called only for jumptable rodata for functions. This was true until
>>>>> 6.1, when a commit in varasm.c started calling the hook for mergeable
>>>>> string/constant data as well.
>>>>>
>>>>> This resulted in string constants ending up in a section intended for
>>>>> jumptables (flash), and broke code using those constants, which
>>>>> expects them to be present in rodata (SRAM).
>>>>>
>>>>> Given that the original reason for placing jumptables in a section was
>>>>> fixed by Johann in PR 63323, this patch restores the original
>>>>> behavior. Reg testing on both gcc-6-branch and trunk showed no
>>>>> regressions.
>>>>
>>>> Just for the record:
>>>>
>>>> The intention for jump-tables in function-rodata-section was to get
>>>> fine-grained section for the tables so that --gc-sections and
>>>> -ffunction-sections not only gc unused functions but also unused
>>>> jump-tables. As these tables had to reside in the lowest 64KiB of flash
>>>> (.progmem section) neither .rodata nor .text was a correct placement,
>>>> hence the hacking in TARGET_ASM_FUNCTION_RODATA_SECTION.
>>>>
>>>> Before using TARGET_ASM_FUNCTION_RODATA_SECTION, all jump tables were
>>>> put into .progmem.gcc_sw_table by ASM_OUTPUT_BEFORE_CASE_LABEL switching
>>>> to that section.
>>>>
>>>> We actually never had fump-tables in .text before...
>>>
>>> JUMP_TABLES_IN_TEXT_SECTION was 1 before r37465 - that was when the
>>> progmem.gcc_sw_table section was introduced. But yes, I understand that
>>> the target hook for FUNCTION_RODATA_SECTION was done to get them gc'ed
>>> along with the code.
>>>
>>>>
>>>> The purpose of PR63323 was to have more generic jump-table
>>>> implementation that also works if the table does NOT reside in the lower
>>>> 64KiB. This happens when moving whole whole TEXT section around like
>>>> for a bootloader.
>>>
>>> Understood.
>>>>
>>>>> As pointed out by Johann, this may end up increasing code
>>>>> size if there are lots of branches that cross the jump tables. I
>>>>> intend to propose a separate patch that gives additional information
>>>>> to the target hook (SECCAT_RODATA_{STRING,JUMPTABLE}) so it can know
>>>>> what type of function rodata is coming on. Johann also suggested
>>>>> handling jump table generation ourselves - I'll experiment with that
>>>>> some more.
>>>>>
>>>>> If ok, could someone commit please? Could you also backport to
>>>>> gcc-6-branch?
>>>>>
>>>>> Regards
>>>>> Senthil
>>>>>
>>>>> gcc/ChangeLog
>>>>>
>>>>> 2016-06-03 Senthil Kumar Selvaraj
>>>>>
>>>>> * config/avr/avr.c (avr_asm_function_rodata_section): Remove.
>>>>> * config/avr/avr.c (TARGET_ASM_FUNCTION_RODATA_SECTION): Remove.
>>>>>
>>>>> gcc/testsuite/ChangeLog
>>>>>
>>>>> 2016-06-03 Senthil Kumar Selvaraj
>>>>>
>>>>> * gcc/testsuite/gcc.target/avr/pr71151-1.c: New.
>>>>> * gcc/testsuite/gcc.target/avr/pr71151-2.c: New.
>>>>>
>>>>> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
>>>>> index ba5cd91..3cb8cb7 100644
>>>>> --- gcc/config/avr/avr.c
>>>>> +++ gcc/config/avr/avr.c
>>>>> @@ -9488,65 +9488,6 @@ avr_asm_init_sections (void)
>>>>> }
>>>>>
>>>>>
>>>>> -/* Implement `TARGET_ASM_FUNCTION_RODATA_SECTION'. */
>>>>> -
>>>>> -static section*
>>>>> -avr_asm_function_rodata
MAINTAINERS (Write After Approval): Add myself
Added myself to MAINTAINERS (Write After Approval). Regards Senthil Index: ChangeLog === --- ChangeLog (revision 237571) +++ ChangeLog (working copy) @@ -1,3 +1,7 @@ +2016-06-18 Senthil Kumar Selvaraj + + * MAINTAINERS (Write After Approval): Add myself. + 2016-06-16 Michael Collison * MAINTAINERS (Write After Approval): Add myself. Index: MAINTAINERS === --- MAINTAINERS (revision 237571) +++ MAINTAINERS (working copy) @@ -569,6 +569,7 @@ Martin Sebor Dodji Seketeli Svein Seldal +Senthil Kumar Selvaraj Thiemo Seufer Bill Seurer Marcus Shawcroft
Re: [Patch, avr] Fix PR 71151
Georg-Johann Lay writes:
> Senthil Kumar Selvaraj schrieb:
>> Hi,
>>
>> This patch fixes PR 71151 by eliminating the
>> TARGET_ASM_FUNCTION_RODATA_SECTION hook and setting
>> JUMP_TABLES_IN_TEXT_SECTION to 1.
>>
>> As described in the bugzilla entry, this hook assumed it will get
>> called only for jumptable rodata for functions. This was true until
>> 6.1, when a commit in varasm.c started calling the hook for mergeable
>> string/constant data as well.
>>
>> This resulted in string constants ending up in a section intended for
>> jumptables (flash), and broke code using those constants, which
>> expects them to be present in rodata (SRAM).
>>
>> Given that the original reason for placing jumptables in a section was
>> fixed by Johann in PR 63323, this patch restores the original
>> behavior. Reg testing on both gcc-6-branch and trunk showed no regressions.
>>
>> As pointed out by Johann, this may end up increasing code
>> size if there are lots of branches that cross the jump tables. I
>> intend to propose a separate patch that gives additional information
>> to the target hook (SECCAT_RODATA_{STRING,JUMPTABLE}) so it can know
>> what type of function rodata is coming on. Johann also suggested
>> handling jump table generation ourselves - I'll experiment with that
>> some more.
>>
>> If ok, could someone commit please? Could you also backport to
>> gcc-6-branch?
>>
>> Regards
>> Senthil
>>
>> gcc/ChangeLog
>>
>> 2016-06-03 Senthil Kumar Selvaraj
>>
>
> Missing PR target/71151
>
>> * config/avr/avr.c (avr_asm_function_rodata_section): Remove.
>> * config/avr/avr.c (TARGET_ASM_FUNCTION_RODATA_SECTION): Remove.
>>
>> gcc/testsuite/ChangeLog
>>
>> 2016-06-03 Senthil Kumar Selvaraj
>>
>
> Missing PR target/71151
>
>> * gcc/testsuite/gcc.target/avr/pr71151-1.c: New.
>> * gcc/testsuite/gcc.target/avr/pr71151-2.c: New.
>>
Sorry I missed that. Is it ok to modify the entry alone again?
I just started using the mklog perl script from gcc/contrib
instead of writing the entries by hand - I didn't realize
that script doesn't know about PRs.
Regards
Senthil
[Patch, testsuite] Mark some more tests as UNSUPPORTED for avr
Hi,
This patch fixes some bogus failures for the avr target by requiring
int32plus or ptr32plus support.
Ok for trunk?
Regards
Senthil
gcc/testsuite/ChangeLog:
2016-06-20 Senthil Kumar Selvaraj
* c-c++-common/pr68657-1.c: Require ptr32plus support.
* c-c++-common/pr68657-2.c: Likewise.
* c-c++-common/pr68657-3.c: Likewise.
* gcc.dg/torture/pr69714.c: Require int32plus support.
* gcc.dg/torture/pr70025.c: Likewise.
* gcc.dg/torture/pr70083.c: Likewise.
* gcc.dg/torture/pr70542.c: Likewise.
* gcc.dg/torture/pr70935.c: Require ptr32plus support.
diff --git gcc/testsuite/c-c++-common/pr68657-1.c
gcc/testsuite/c-c++-common/pr68657-1.c
index 3db6f49..84f3e54 100644
--- gcc/testsuite/c-c++-common/pr68657-1.c
+++ gcc/testsuite/c-c++-common/pr68657-1.c
@@ -1,5 +1,6 @@
/* PR c/68657 */
/* { dg-options "-Werror=sign-conversion -Werror=float-conversion
-Werror=frame-larger-than=65536" } */
+/* { dg-require-effective-target ptr32plus } */
void
f1 (void)
diff --git gcc/testsuite/c-c++-common/pr68657-2.c
gcc/testsuite/c-c++-common/pr68657-2.c
index 9eb68ce..1391088 100644
--- gcc/testsuite/c-c++-common/pr68657-2.c
+++ gcc/testsuite/c-c++-common/pr68657-2.c
@@ -1,6 +1,7 @@
/* PR c/68657 */
/* { dg-do compile } */
/* { dg-options "-Werror=larger-than=65536" } */
+/* { dg-require-effective-target ptr32plus } */
int a[131072]; /* { dg-error "size of 'a' is \[1-9]\[0-9]* bytes" } */
int b[1024]; /* { dg-bogus "size of 'b' is \[1-9]\[0-9]* bytes" } */
diff --git gcc/testsuite/c-c++-common/pr68657-3.c
gcc/testsuite/c-c++-common/pr68657-3.c
index 84622fc..1e80c5b 100644
--- gcc/testsuite/c-c++-common/pr68657-3.c
+++ gcc/testsuite/c-c++-common/pr68657-3.c
@@ -1,5 +1,6 @@
/* PR c/68657 */
/* { dg-do compile } */
+/* { dg-require-effective-target ptr32plus } */
#pragma GCC diagnostic error "-Wlarger-than=65536"
int a[131072]; /* { dg-error "size of 'a' is \[1-9]\[0-9]* bytes" } */
diff --git gcc/testsuite/gcc.dg/torture/pr69714.c
gcc/testsuite/gcc.dg/torture/pr69714.c
index 229b7ad..85de8be 100644
--- gcc/testsuite/gcc.dg/torture/pr69714.c
+++ gcc/testsuite/gcc.dg/torture/pr69714.c
@@ -1,5 +1,6 @@
/* { dg-do run } */
/* { dg-options "-fno-strict-aliasing" } */
+/* { dg-require-effective-target int32plus } */
#include
#include
diff --git gcc/testsuite/gcc.dg/torture/pr70025.c
gcc/testsuite/gcc.dg/torture/pr70025.c
index dafae0b..6c43a0a 100644
--- gcc/testsuite/gcc.dg/torture/pr70025.c
+++ gcc/testsuite/gcc.dg/torture/pr70025.c
@@ -1,6 +1,7 @@
/* PR middle-end/70025 */
/* { dg-do run } */
/* { dg-additional-options "-mtune=z10" { target s390*-*-* } } */
+/* { dg-require-effective-target int32plus } */
typedef char (*F) (unsigned long, void *);
typedef union { struct A { char a1, a2, a3, a4; unsigned long a5; F a6; void
*a7; } b; char c[1]; } B;
diff --git gcc/testsuite/gcc.dg/torture/pr70083.c
gcc/testsuite/gcc.dg/torture/pr70083.c
index 0cf2892..f33cb74 100644
--- gcc/testsuite/gcc.dg/torture/pr70083.c
+++ gcc/testsuite/gcc.dg/torture/pr70083.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-Wno-psabi" } */
+/* { dg-require-effective-target int32plus } */
typedef short v16hi __attribute__ ((vector_size (32)));
typedef int v8si __attribute__ ((vector_size (32)));
diff --git gcc/testsuite/gcc.dg/torture/pr70542.c
gcc/testsuite/gcc.dg/torture/pr70542.c
index ed7ab9d..39c7f81 100644
--- gcc/testsuite/gcc.dg/torture/pr70542.c
+++ gcc/testsuite/gcc.dg/torture/pr70542.c
@@ -1,5 +1,6 @@
/* PR rtl-optimization/70542 */
/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
int a[113], d[113];
short b[113], c[113], e[113];
diff --git gcc/testsuite/gcc.dg/torture/pr70935.c
gcc/testsuite/gcc.dg/torture/pr70935.c
index eb7f034..f1dd9e4 100644
--- gcc/testsuite/gcc.dg/torture/pr70935.c
+++ gcc/testsuite/gcc.dg/torture/pr70935.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O3 -g" } */
+/* { dg-require-effective-target ptr32plus } */
int d0, sj, v0, rp, zi;
--
2.7.4
Re: [Patch, avr] Fix PR 71151
Georg-Johann Lay writes: > Senthil Kumar Selvaraj schrieb: >> Senthil Kumar Selvaraj writes: >> >>> Georg-Johann Lay writes: >>> >>>> Senthil Kumar Selvaraj schrieb: >>>>> Hi, >>>>> >>>>> [set JUMP_TABLES_IN_TEXT_SECTION to 1] >>> >>> I added tests that use linker relaxation and discovered a relaxation bug >>> in binutils 2.26 (and later) that messes up symbol values in the >>> presence of alignment directives. I'm working on that right now - >>> hopefully, it'll get backported to the release branch. >>> >>> Once that gets upstream, I'll resend the patch - with more tests, and >>> incorporating your comments. >>> >> >> There were two binutils bugs (PR ld/20221 and ld/20254) that were >> blocking this patch - on enabling, relaxation, jumptables were >> getting corrupted. Both of the issues are now fixed, and the fixes >> are in master and 2.26 branch. > > Should we mention in the release notes that Binutils >= 2.26 is needed > for avr-gcc >= 6 ? Yes, we should document it. binutils 2.25 would probably work too, as the bugs were introduced only in binutils 2.26. I'll check and send a patch. > > Maybe even check during configure whether an appropriate version of > Binutils is used? That would be nice, but is it ok to add target specific conditions to configure.ac? Regards Senthil
Re: [patch,avr]: ad PR71151: Make test cases pass on smaller targets.
Georg-Johann Lay writes:
> On 22.06.2016 19:06, Mike Stump wrote:
>> On Jun 22, 2016, at 7:21 AM, Georg-Johann Lay wrote:
>>>
>>> Some tests for PR71151 assume that the target MCU has a 3-byte PC. The
>>> tests are failing because the simulator (avrtest) rejects to load the
>>> respective executables if .text exceeds 128KiB, e.g. for -mmcu=atmega128
>>> which has only flash of 128KiB and only a 2-byte PC.
>>>
>>> Hence the tests have to be skipped if the target MCU has no 3-byte PC,
>>> hence a new dg-require-effective-target proc supporting "avr_3byte_pc".
>>>
>>> I added the new proc right after the last check_effective_target_arm_***
>>> so that the test is in ASCII collating order.
>>>
>>> Ok for trunk and v6?
>>
>> No. Please see target-utils.exp and ensure that the tools generate a
>> stylized message and then add support for that to target-utils.exp. If you
>> are using binutils, the text should go into a memory segment that will fill
>
> Binutils don't produce a message so there is nothing to scan for. Hacking
> binutils is beyond my scope.
binutils doesn't produce a message because
1. The size of text is not device specific right now - IIRC, it's set to
the max flash size for the emulation. I have a partial fix for this -
the text region size can now be set via a linker symbol
(__TEXT_REGION_LENGTH__). I'm planning to patch avr-libc to
automatically set this symbol based on flash size information
present in the device header file.
2. Even if (1) is fixed, the custom section (.foo) is not mapped to
any output section or region in the linker script. The linker can
error out only if the contents overflow a region.
If we have a custom linker script that places .foo in the text region,
and if we set the location counter to the address .foo should be placed,
i.e. something like
.text :
{
...
*(.fini0) /* Infinite loop after program termination. */
KEEP (*(.fini0))
. = FOO_START;
KEEP(*(.foo))
_etext = . ;
} > text
and then if we pass -Wl,--defsym=FOO_START=0x20002 when linking, we'll get
the linker to report overflow.
Not sure if it's worth the effort though.
Mike, how about effective targets for sub arch/ISA variants
(avr51/avrxmega6/avrtiny..)? I guess arm has these for thumb1/thumb2/neon/dsp
etc. That would help us skip arch specific test cases, and will help
with testcases like these too - we can infer PC size from the arch.
Regards
Senthil
>
>> when it is too large. When it does, then binutils will generate one of the
>> messages already handled, then you're done.
>
> avrtest behaves just as if the program under test would call abort. There
> are
> at least 2 other AVR simulators; dunno how they would handle the situation.
>
> I don't see how an a-posteriori test could be independent of simulator,
> independent of board descriptions and all that stuff.
>
> The tests in question don't fail because the program is too big as a result
> of
> some mussed optimization; some code is deliberately placed across a 64KiB or
> 128KiB boundary or beyond 128KiB. All this is known a priori.
>
> Hence dropping the original patch and proposing a new one that doesn't need
> extensions to lib.
>
> The new tests just won't put any code at places where we know in advance some
> simulator might barf. As the compiler has no idea of exact flash size, the
> relevant flash property is deduced from ISA properties.
>
> Is this one ok?
>
> Johann
>
>
> gcc/testsuite/
> PR target/71151
> * gcc.target/avr/pr71151-common.h (foo): Use macro SECTION_NAME
> instead of ".foo" for its section name.
> * gcc.target/avr/pr71151-2.c (SECTION_NAME): Define appropriately
> depending on MCU's flash size.
> * gcc.target/avr/pr71151-3.c (SECTION_NAME): Dito.
> * gcc.target/avr/pr71151-4.c (SECTION_NAME): Dito.
> * gcc.target/avr/pr71151-5.c (SECTION_NAME): Dito.
> * gcc.target/avr/pr71151-6.c (SECTION_NAME): Dito.
> * gcc.target/avr/pr71151-7.c (SECTION_NAME): Dito.
> * gcc.target/avr/pr71151-8.c (SECTION_NAME): Dito.
[Patch, testsuite, committed] Fix cunroll-13.c failure for avr
Hi,
The test reports bogus failures because the loop variable i is declared
as int, and the constant expected in the dump doesn't fit in an int for avr.
Fixed by explicitly using __INT32_TYPE__ for targets with __SIZEOF_INT__ < 4.
Committed to trunk as obvious.
Regards
Senthil
gcc/testsuite/
2017-05-09 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/cunroll-13.c: Use __INT32_TYPE__ for
for targets with __SIZEOF_INT__ < 4.
diff --git gcc/testsuite/gcc.dg/tree-ssa/cunroll-13.c
gcc/testsuite/gcc.dg/tree-ssa/cunroll-13.c
index f3fe8b51468..904e6dc075b 100644
--- gcc/testsuite/gcc.dg/tree-ssa/cunroll-13.c
+++ gcc/testsuite/gcc.dg/tree-ssa/cunroll-13.c
@@ -1,10 +1,17 @@
/* { dg-do compile } */
/* { dg-options "-O3 -fdisable-tree-evrp -fdisable-tree-cunrolli
-fdisable-tree-vrp1 -fdump-tree-cunroll-blocks-details" } */
+
+#if __SIZEOF_INT__ < 4
+__extension__ typedef __INT32_TYPE__ i32;
+#else
+typedef int i32;
+#endif
+
struct a {int a[8];int b;};
void
t(struct a *a)
{
- for (int i=0;i<123456 && a->a[i];i++)
+ for (i32 i=0;i<123456 && a->a[i];i++)
a->a[i]++;
}
/* This pass relies on the fact that we do not eliminate the redundant test
for i early.
[Patch, testsuite, committed] Fix bogus pr78886.c for avr
Hi,
The test declares malloc with an unsigned long parameter. This causes
a warning for avr, as it's size_t is only unsigned int.
Fixed by typdef'ing __SIZE_TYPE__ to size_t and using it in the malloc
function's declaration.
Committed as obvious.
Regards
Senthil
gcc/testsuite/ChangeLog
2017-05-17 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/pr78886.c: Use __SIZE_TYPE__ instead of
unsigned long.
Index: gcc/testsuite/gcc.dg/tree-ssa/pr78886.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr78886.c (revision 248137)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr78886.c (working copy)
@@ -1,7 +1,9 @@
/* { dg-do compile } */
/* { dg-options "-O2" } */
-void *malloc(unsigned long x);
+__extension__ typedef __SIZE_TYPE__ size_t;
+void *malloc(size_t x);
+
void foo(void)
{
volatile int i;
[Patch, testsuite, committed] Fix bogus builtin-snprintf-warn-3.c failure for avr
Hi,
The below patch fixes failures in builtin-snprintf-warn-3.c for the
avr target.
The test declares a struct with an array member that has INT_MAX/32767
elements. This causes a "type xxx is too large" error for targets like
the avr, which have pointers smaller or equal to (16 bit) int size.
Fixed by marking the test as unsupported for targets with ptr size <
32. Committed as obvious.
Regards
Senthil
gcc/testsuite/
2017-05-23 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/builtin-snprintf-warn-3.c: Require ptr32plus.
Index: gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-warn-3.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-warn-3.c (revision
248360)
+++ gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-warn-3.c (working copy)
@@ -1,6 +1,7 @@
/* PR middle-end/79448 - unhelpful -Wformat-truncation=2 warning
{ dg-do compile }
- { dg-options "-O2 -Wformat -Wformat-truncation=2 -ftrack-macro-expansion=0"
} */
+ { dg-options "-O2 -Wformat -Wformat-truncation=2 -ftrack-macro-expansion=0"
}
+ { dg-require-effective-target ptr32plus } */
typedef __SIZE_TYPE__ size_t;
typedef __WCHAR_TYPE__ wchar_t;
[Patch, testsuite] Fix failing overflow-1.c for avr
Hi,
The test assumes 32 bit ints, and expects a constant in the
dump that is only valid for 32 bit ints. This trivial patch
fixes that by explicitly specifying __UINT32_TYPE__ as the type.
Committed as obvious.
Regards
Senthil
gcc/testsuite/ChangeLog
2017-03-21 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/overflow-1.c: Use __UINT32_TYPE__ for targets
with sizeof(int) < 4.
diff --git gcc/testsuite/gcc.dg/tree-ssa/overflow-1.c
gcc/testsuite/gcc.dg/tree-ssa/overflow-1.c
index e126609c53d9..b664d0f120aa 100644
--- gcc/testsuite/gcc.dg/tree-ssa/overflow-1.c
+++ gcc/testsuite/gcc.dg/tree-ssa/overflow-1.c
@@ -1,14 +1,20 @@
/* { dg-do compile } */
/* { dg-options "-O -fdump-tree-optimized" } */
-int f(unsigned a){
-unsigned b=5;
-unsigned c=a-b;
+#if __SIZEOF_INT__ < 4
+ __extension__ typedef __UINT32_TYPE__ uint32_t;
+#else
+ typedef unsigned uint32_t;
+#endif
+
+int f(uint32_t a){
+uint32_t b=5;
+uint32_t c=a-b;
return c>a;
}
-int g(unsigned a){
-unsigned b=32;
-unsigned c=a+b;
+int g(uint32_t a){
+uint32_t b=32;
+uint32_t c=a+b;
return c
[Patch, testsuite] Fix more failing tests for avr
Hi,
The below trivial patch fixes some more testsuite failures for the avr
target. They fail for avr because they assume ints are 32 bits or
wider. The patch uses explicit __{U}INT32_TYPE__ for targets with
smaller int size.
Committed as obvious.
Regards
Senthil
2017-03-28 Senthil Kumar Selvaraj
* gcc.c-torture/execute/pr79121.c:Use __{U}INT32_TYPE__ for targets
with sizeof(int) < 4.
* gcc.c-torture/execute/pr79737-1.c (struct S): Likewise.
* gcc.c-torture/execute/pr79737-2.c: Likewise.
* gcc.dg/torture/pr79777.c: Likewise.
* gcc.dg/torture/pr79910.c: Likewise.
Index: gcc.c-torture/execute/pr79121.c
===
--- gcc.c-torture/execute/pr79121.c (revision 246528)
+++ gcc.c-torture/execute/pr79121.c (working copy)
@@ -1,21 +1,29 @@
+#if __SIZEOF_INT__ < 4
+ __extension__ typedef __UINT32_TYPE__ uint32_t;
+ __extension__ typedef __INT32_TYPE__ int32_t;
+#else
+ typedef unsigned uint32_t;
+ typedef int int32_t;
+#endif
+
extern void abort (void);
-__attribute__ ((noinline, noclone)) unsigned long long f1 (int x)
+__attribute__ ((noinline, noclone)) unsigned long long f1 (int32_t x)
{
return ((unsigned long long) x) << 4;
}
-__attribute__ ((noinline, noclone)) long long f2 (unsigned x)
+__attribute__ ((noinline, noclone)) long long f2 (uint32_t x)
{
return ((long long) x) << 4;
}
-__attribute__ ((noinline, noclone)) unsigned long long f3 (unsigned x)
+__attribute__ ((noinline, noclone)) unsigned long long f3 (uint32_t x)
{
return ((unsigned long long) x) << 4;
}
-__attribute__ ((noinline, noclone)) long long f4 (int x)
+__attribute__ ((noinline, noclone)) long long f4 (int32_t x)
{
return ((long long) x) << 4;
}
Index: gcc.c-torture/execute/pr79737-1.c
===
--- gcc.c-torture/execute/pr79737-1.c (revision 246528)
+++ gcc.c-torture/execute/pr79737-1.c (working copy)
@@ -1,13 +1,19 @@
/* PR tree-optimization/79737 */
+#if __SIZEOF_INT__ < 4
+ __extension__ typedef __INT32_TYPE__ int32_t;
+#else
+ typedef int int32_t;
+#endif
+
#pragma pack(1)
struct S
{
- int b:18;
- int c:1;
- int d:24;
- int e:15;
- int f:14;
+ int32_t b:18;
+ int32_t c:1;
+ int32_t d:24;
+ int32_t e:15;
+ int32_t f:14;
} i;
int g, j, k;
static struct S h;
Index: gcc.c-torture/execute/pr79737-2.c
===
--- gcc.c-torture/execute/pr79737-2.c (revision 246528)
+++ gcc.c-torture/execute/pr79737-2.c (working copy)
@@ -1,13 +1,19 @@
/* PR tree-optimization/79737 */
+#if __SIZEOF_INT__ < 4
+ __extension__ typedef __INT32_TYPE__ int32_t;
+#else
+ typedef int int32_t;
+#endif
+
#pragma pack(1)
struct S
{
- int b:18;
- int c:1;
- int d:24;
- int e:15;
- int f:14;
+ int32_t b:18;
+ int32_t c:1;
+ int32_t d:24;
+ int32_t e:15;
+ int32_t f:14;
} i, j;
void
Index: gcc.dg/torture/pr79777.c
===
--- gcc.dg/torture/pr79777.c(revision 246528)
+++ gcc.dg/torture/pr79777.c(working copy)
@@ -1,9 +1,14 @@
/* { dg-do compile } */
typedef unsigned short __u16;
-typedef unsigned int __u32;
+#if __SIZEOF_INT__ < 4
+ __extension__ typedef __UINT32_TYPE__ __u32;
+ __extension__ typedef __UINT32_TYPE__ u32;
+#else
+ typedef unsigned int __u32;
+ typedef unsigned int u32;
+#endif
typedef unsigned char u8;
-typedef unsigned int u32;
typedef __u16 __le16;
typedef __u32 __le32;
typedef u32 secno;
Index: gcc.dg/torture/pr79910.c
===
--- gcc.dg/torture/pr79910.c(revision 246528)
+++ gcc.dg/torture/pr79910.c(working copy)
@@ -2,7 +2,11 @@
/* { dg-additional-options "-fweb" } */
typedef unsigned char u8;
-typedef unsigned int u32;
+#if __SIZEOF_INT__ < 4
+ __extension__ typedef __UINT32_TYPE__ u32;
+#else
+ typedef unsigned int u32;
+#endif
typedef unsigned long long u64;
int a;
[Patch, testsuite] Fix failing builtin-sprintf-warn-{3,10}.c for avr
Hi,
This patch fixes a whole bunch of failures reported for
gcc.dg/tree-ssa/builtin-sprintf-warn-{3,10}.c for the avr target.
builtin-sprintf-warn-10.c fails because the bounds in the warning
messages expect 4 digit wide exponents i.e. __DBL_MAX_EXP__ > 999.
For the avr, floats and doubles are both 32 bits wide, __DBL_MAX_EXP__
== 128, and the max number of exponent digits can only be 3 .
The computed size thus ends up one short of the value the test
expects. The patch makes the test run only for targets with double64plus.
builtin-sprintf-warn-3.c fails because the test appears to assume all
non lp64 targets to be ilp32. For the avr, pointer size and int size
are equal, but both are 16 bits, not 32. The patch fixes this by
explicitly adding avr to the dejagnu selector.
Ok for trunk?
Regards
Senthil
gcc/testsuite/ChangeLog:
2017-04-06 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/builtin-sprintf-warn-10.c: Require double64plus.
* gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (void test_too_large): Add
avr-*-* to non-lp64 selector.
diff --git gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-10.c
gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-10.c
index 1213e89f7bb..30599ad04dc 100644
--- gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-10.c
+++ gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-10.c
@@ -2,6 +2,7 @@
Test to verify the correctness of ranges of output computed for floating
point directives.
{ dg-do compile }
+ { dg-require-effective-target double64plus }
{ dg-options "-O2 -Wformat -Wformat-overflow -ftrack-macro-expansion=0" } */
typedef __builtin_va_list va_list;
diff --git gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
index 72ec3afaa41..9db7ad74f37 100644
--- gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
+++ gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
@@ -358,19 +358,19 @@ void test_too_large (char *d, int x, __builtin_va_list va)
__builtin_snprintf (d, imax,"%c", x);
__builtin_snprintf (d, imax_p1, "%c", x); /* { dg-warning "specified bound
\[0-9\]+ exceeds .INT_MAX." "INT_MAX + 1" { target lp64 } } */
- /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { ilp32 } } .-1 } */
+ /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { { avr-*-* } || ilp32 } } .-1 } */
__builtin_vsnprintf (d, imax,"%c", va);
__builtin_vsnprintf (d, imax_p1, "%c", va); /* { dg-warning "specified
bound \[0-9\]+ exceeds .INT_MAX." "INT_MAX + 1" { target lp64 } } */
- /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { ilp32 } } .-1 } */
+ /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { { avr-*-* } || ilp32 } } .-1 } */
__builtin___snprintf_chk (d, imax,0, imax,"%c", x);
__builtin___snprintf_chk (d, imax_p1, 0, imax_p1, "%c", x); /* {
dg-warning "specified bound \[0-9\]+ exceeds .INT_MAX." "INT_MAX + 1" { target
lp64 } } */
- /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { ilp32 } } .-1 } */
+ /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { { avr-*-* } || ilp32 } } .-1 } */
__builtin___vsnprintf_chk (d, imax,0, imax,"%c", va);
__builtin___vsnprintf_chk (d, imax_p1, 0, imax_p1, "%c", va); /* {
dg-warning "specified bound \[0-9\]+ exceeds .INT_MAX." "INT_MAX + 1" { target
lp64 } } */
- /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { ilp32 } } .-1 } */
+ /* { dg-warning "specified bound \[0-9\]+ exceeds maximum object size"
"INT_MAX + 1" { target { { avr-*-* } || ilp32 } } .-1 } */
const size_t ptrmax = __PTRDIFF_MAX__;
const size_t ptrmax_m1 = ptrmax - 1;
[Patch, testsuite] Fix broken pr80341.c for avr
Hi,
This patch skips pr80341.c for targets with int size less than 32 bits.
The assertion in the testcase holds only if sizeof(int) > sizeof(short),
which isn't true for smaller int size targets like the avr.
Specifically, after integer promotion, the "usual arithmetic
conversion" of the unsigned short to signed int doesn't occur, and
this causes the test to fail.
Committed as obvious.
Regards
Senthil
2017-04-19 Senthil Kumar Selvaraj
* gcc.dg/torture/pr80341.c: Require int32plus.
Index: gcc.dg/torture/pr80341.c
===
--- gcc.dg/torture/pr80341.c(revision 246991)
+++ gcc.dg/torture/pr80341.c(working copy)
@@ -1,5 +1,6 @@
/* { dg-do run } */
/* { dg-additional-options "-Wno-overflow" } */
+/* { dg-require-effective-target int32plus } */
const signed char c = -84;
signed char s;
[Patch, testsuite] Skip pr80170.c for non-ptr32plus targets
Hi,
The below patch skips gcc.dg/pr80170.c for targets with pointer size
less than 32. The testcase uses pointer offsets that are 32 bit or
higher, and this ends up triggering an ICE because a ptrofftype_p
assert fires for non-ptr32 plus targets.
Committed as obvious.
Regards
Senthil
gcc/testsuite/ChangeLog
2017-04-19 Senthil Kumar Selvaraj
* gcc.dg/pr80170.c: Require ptr32plus.
Index: gcc.dg/pr80170.c
===
--- gcc.dg/pr80170.c(revision 247010)
+++ gcc.dg/pr80170.c(working copy)
@@ -1,5 +1,6 @@
/* { dg-do run } */
/* { dg-options "-fgimple -O2 -ftree-slp-vectorize" } */
+/* { dg-require-effective-target ptr32plus } *
struct A
{
[Patch, testsuite] Fix failing attr-alloc_size-10.c for avr
Hi,
Integer promotion combined with equal sizes for short and int (16 bits)
causes overflow warnings when expanding the TEST macro for USHRT_MAX.
Fixed by explicitly disabling overflow warnings for targets with !int32plus.
Committed as obvious.
Regards
Senthil
gcc/testsuite/ChangeLog
2017-04-25 Senthil Kumar Selvaraj
* gcc.dg/attr-alloc_size-10.c: Ignore overflow warnings
for targets with int size less than 32 bits.
Index: gcc/testsuite/gcc.dg/attr-alloc_size-10.c
===
--- gcc/testsuite/gcc.dg/attr-alloc_size-10.c (revision 247191)
+++ gcc/testsuite/gcc.dg/attr-alloc_size-10.c (working copy)
@@ -4,7 +4,8 @@
range.
{ dg-do compile }
- { dg-options "-O2 -Walloc-size-larger-than=12" } */
+ { dg-options "-O2 -Walloc-size-larger-than=12" }
+ { dg-options "-Wno-overflow" { target { ! int32plus } } } */
#define SCHAR_MAX __SCHAR_MAX__
#define SCHAR_MIN (-SCHAR_MAX - 1)
[Patch, testsuite] Fix bogus pr78138.c failure for avr
Hi, The trivial patch below fixes a bogus testsuite failure (gcc.dg/pr78138.c) for the avr target. The declaration for memcpy had the size parameter declared as unsigned long. For avr, __SIZE_TYPE__ is unsigned int, and this caused a builtin-declaration-mismatch warning, resulting in a couple of FAILs. The patch fixes that by typedef'ing __SIZE_TYPE__ to size_t and using size_t as the type for memcpy's third parameter. Committed to trunk as obvious. Regards Senthil gcc/testsuite/ChangeLog 2017-05-02 Senthil Kumar Selvaraj * gcc.dg/pr78138.c: Use __SIZE_TYPE__ instead of unsigned long. Index: gcc.dg/pr78138.c === --- gcc.dg/pr78138.c(revision 247481) +++ gcc.dg/pr78138.c(working copy) @@ -5,7 +5,9 @@ char d [5]; -void* memcpy (void*, const void*, unsigned long); +__extension__ typedef __SIZE_TYPE__ size_t; + +void* memcpy (void*, const void*, size_t); extern char* strcpy (char*, const char*); void f (int i, int j)
[Patch, avr]Fix multiple ICE fallout of PR 69764
Hi,
This patch fixes ~230 internal compiler errors that showed up after
the fix for PR 69764. After the patch, target backends need to
explicitly specify mode for operand 2 of shift and rotate patterns -
see md.texi mod at
https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/doc/md.texi?r1=233358&r2=233613&pathrev=233613.
The avr backend had VOIDmode as the mode for rotl SPN, and this patch
sets it to the mode of the insn to fix the problem.
Reg testing shows ICE fixes and no new failures.
If this is ok, could someone commit please? I don't have commit
access.
Regards
Senthil
2016-03-10 Senthil Kumar Selvaraj
* config/avr/avr.md (rotl3): Set
mode for operand 2.
diff --git gcc/config/avr/avr.md gcc/config/avr/avr.md
index ff26f2e..c988446 100644
--- gcc/config/avr/avr.md
+++ gcc/config/avr/avr.md
@@ -3351,7 +3351,7 @@
(define_expand "rotl3"
[(parallel [(set (match_operand:HISI 0 "register_operand" "")
(rotate:HISI (match_operand:HISI 1 "register_operand" "")
-(match_operand:VOID 2 "const_int_operand" "")))
+(match_operand:HISI 2 "const_int_operand" "")))
(clobber (match_dup 3))])]
""
{
[Patch, testsuite] Skip testcase for avr
Hi,
This trivial patch skips gcc.c-torture/compile/20151204.c for the avr
target - the test allocates ~64K on the stack, which is too big
for the avr target. Right now, the test errors out with "total
size of local objects too large".
If this is ok, could someone commit please? I don't have commit access.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-03-16 Senthil Kumar Selvaraj
* gcc.c-torture/compile/20151204.c: Skip for avr.
diff --git a/gcc/testsuite/gcc.c-torture/compile/20151204.c
b/gcc/testsuite/gcc.c-torture/compile/20151204.c
index 036316c..0a60871 100644
--- a/gcc/testsuite/gcc.c-torture/compile/20151204.c
+++ b/gcc/testsuite/gcc.c-torture/compile/20151204.c
@@ -1,3 +1,5 @@
+/* { dg-skip-if "Array too big" { "avr-*-*" } { "*" } { "" } } */
+
typedef __SIZE_TYPE__ size_t;
int strcmp (const char*, const char*);
Re: [Patch, testsuite] Skip testcase for avr
Rainer Orth writes:
> Hi Senthil,
>
>> diff --git a/gcc/testsuite/gcc.c-torture/compile/20151204.c
>> b/gcc/testsuite/gcc.c-torture/compile/20151204.c
>> index 036316c..0a60871 100644
>> --- a/gcc/testsuite/gcc.c-torture/compile/20151204.c
>> +++ b/gcc/testsuite/gcc.c-torture/compile/20151204.c
>> @@ -1,3 +1,5 @@
>> +/* { dg-skip-if "Array too big" { "avr-*-*" } { "*" } { "" } } */
>
> just one nit: please omit the default args to dg-skip-if, they are
> unnecessary.
>
Done.
gcc/testsuite/ChangeLog
2016-03-16 Senthil Kumar Selvaraj
* gcc.c-torture/compile/20151204.c: Skip for avr.
diff --git gcc/testsuite/gcc.c-torture/compile/20151204.c
gcc/testsuite/gcc.c-torture/compile/20151204.c
index 036316c..e07e13d 100644
--- gcc/testsuite/gcc.c-torture/compile/20151204.c
+++ gcc/testsuite/gcc.c-torture/compile/20151204.c
@@ -1,3 +1,5 @@
+/* { dg-skip-if "Array too big" { "avr-*-*" } } */
+
typedef __SIZE_TYPE__ size_t;
int strcmp (const char*, const char*);
[Patch, testsuite] Require int32plus and scheduling support for some tests
Hi,
This patch add dg-require-effective-target directives to a few tests
that were failing unnecessarily for the AVR target.
One of them invokes the compiler with -fschedule-insns2 - I've
required scheduling support for that testcase. For all other tests,
I've required int32plus - they either use bit shifts wider than
16 bits (AVR's int size), or use int constants that are too big
for a 16 bit int.
If ok, could someone commit please? I don't have commit access.
Regards
Senthil
2016-04-04 Senthil Kumar Selvaraj
* gcc.c-torture/compile/pr69102.c: Require scheduling support.
* gcc.c-torture/compile/pr37669.c: Require >=32 bit integers.
* gcc.c-torture/execute/bitfld-6.c: Likewise.
* gcc.c-torture/execute/bitfld-7.c: Likewise.
* gcc.c-torture/execute/pr38151.c: Likewise.
* gcc.c-torture/execute/pr66556.c: Likewise.
* gcc.c-torture/execute/pr67781.c: Likewise.
* gcc.c-torture/execute/pr68648.c: Likewise.
diff --git gcc/testsuite/gcc.c-torture/compile/pr37669.c
gcc/testsuite/gcc.c-torture/compile/pr37669.c
index c78243b..a2eafc7 100644
--- gcc/testsuite/gcc.c-torture/compile/pr37669.c
+++ gcc/testsuite/gcc.c-torture/compile/pr37669.c
@@ -1,5 +1,6 @@
/* This testcase used to fail because a miscompiled execute_fold_all_builtins.
*/
/* { dg-options "-fgnu89-inline" } */
+/* { dg-require-effective-target int32plus } */
typedef __SIZE_TYPE__ size_t;
extern __inline __attribute__ ((__always_inline__)) int __attribute__
diff --git gcc/testsuite/gcc.c-torture/compile/pr69102.c
gcc/testsuite/gcc.c-torture/compile/pr69102.c
index b1328ca..1f0cdc6 100644
--- gcc/testsuite/gcc.c-torture/compile/pr69102.c
+++ gcc/testsuite/gcc.c-torture/compile/pr69102.c
@@ -1,4 +1,5 @@
/* { dg-options "-Og -fPIC -fschedule-insns2 -fselective-scheduling2
-fno-tree-fre --param=max-sched-extend-regions-iters=10" } */
+/* { dg-require-effective-target scheduling } */
void bar (unsigned int);
void
diff --git gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
index 50927dc..b8c5cbd 100644
--- gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
+++ gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
@@ -1,3 +1,4 @@
+/* { dg-require-effective-target int32plus } */
union U
{
const int a;
diff --git gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
index e9a61df..350e7a3 100644
--- gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
+++ gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
@@ -1,3 +1,4 @@
+/* { dg-require-effective-target int32plus } */
union U
{
const int a;
diff --git gcc/testsuite/gcc.c-torture/execute/pr38151.c
gcc/testsuite/gcc.c-torture/execute/pr38151.c
index 5ee058d..86c8f77 100644
--- gcc/testsuite/gcc.c-torture/execute/pr38151.c
+++ gcc/testsuite/gcc.c-torture/execute/pr38151.c
@@ -1,4 +1,5 @@
/* { dg-options "-Wno-psabi" } */
+/* { dg-require-effective-target int32plus } */
void abort (void);
struct S2848
diff --git gcc/testsuite/gcc.c-torture/execute/pr66556.c
gcc/testsuite/gcc.c-torture/execute/pr66556.c
index f7acf1c..d1259c4 100644
--- gcc/testsuite/gcc.c-torture/execute/pr66556.c
+++ gcc/testsuite/gcc.c-torture/execute/pr66556.c
@@ -1,4 +1,5 @@
/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
extern void abort (void);
diff --git gcc/testsuite/gcc.c-torture/execute/pr67781.c
gcc/testsuite/gcc.c-torture/execute/pr67781.c
index bf50aa2..71ccd6a 100644
--- gcc/testsuite/gcc.c-torture/execute/pr67781.c
+++ gcc/testsuite/gcc.c-torture/execute/pr67781.c
@@ -1,3 +1,4 @@
+/* { dg-require-effective-target int32plus } */
#ifdef __UINT32_TYPE__
typedef __UINT32_TYPE__ uint32_t;
#else
diff --git gcc/testsuite/gcc.c-torture/execute/pr68648.c
gcc/testsuite/gcc.c-torture/execute/pr68648.c
index fc66806..db55bd0 100644
--- gcc/testsuite/gcc.c-torture/execute/pr68648.c
+++ gcc/testsuite/gcc.c-torture/execute/pr68648.c
@@ -1,3 +1,4 @@
+/* { dg-require-effective-target int32plus } */
int __attribute__ ((noinline))
foo (void)
{
[Patch] Fix PR 60040
Hi,
The below patch fixes PR 60040 by not halting with a hard error on
a spill failure, if reload knows that it has to run again anyway.
It also fixes two reload related ICEs on trunk
(gcc.c-torture/compile/920625-1.c and gcc.dg/tree-ssa/pr70232.c)
for the AVR target. I've slighly reworked the patch - the original
patch by Joern Rennecke did not skip the setting of failure to 1; it
never gets reset afterwards.
Bootstrapped and regtested on/for x86_64-linux with no regressions.
Thee avr target shows additional PASSes for the above two testcases
and no other regressions.
If ok, could someone commit please? I don't have commit access.
Regards
Senthil
gcc/ChangeLog
2016-04-07 Joern Rennecke
Senthil Kumar Selvaraj
PR target/60040
* reload1.c (find_reload_regs): Add tentative parameter.
and don't report spill failure if param set.
(reload): Propagate something_changed to
select_reload_regs.
(select_reload_regs): Add tentative parameter.
gcc/testsuite/ChangeLog
2016-04-07 Sebastian Huber
Matthijs Kooijman
Senthil Kumar Selvaraj
PR target/60040
* gcc.target/avr/pr60040-1.c: New.
* gcc.target/avr/pr60040-2.c: Likewise.
diff --git gcc/reload1.c gcc/reload1.c
index c2800f8..58f58a9 100644
--- gcc/reload1.c
+++ gcc/reload1.c
@@ -346,8 +346,8 @@ static void maybe_fix_stack_asms (void);
static void copy_reloads (struct insn_chain *);
static void calculate_needs_all_insns (int);
static int find_reg (struct insn_chain *, int);
-static void find_reload_regs (struct insn_chain *);
-static void select_reload_regs (void);
+static void find_reload_regs (struct insn_chain *, bool);
+static void select_reload_regs (bool);
static void delete_caller_save_insns (void);
static void spill_failure (rtx_insn *, enum reg_class);
@@ -1022,7 +1022,7 @@ reload (rtx_insn *first, int global)
something_changed = 1;
}
- select_reload_regs ();
+ select_reload_regs (something_changed);
if (failure)
goto failed;
@@ -1960,10 +1960,13 @@ find_reg (struct insn_chain *chain, int order)
is given by CHAIN.
Do it by ascending class number, since otherwise a reg
might be spilled for a big class and might fail to count
- for a smaller class even though it belongs to that class. */
+ for a smaller class even though it belongs to that class.
+ TENTATIVE means that we had some changes that might have invalidated
+ the reloads and that we are going to loop again anyway, so don't give
+ a hard error on failure to find a reload reg. */
static void
-find_reload_regs (struct insn_chain *chain)
+find_reload_regs (struct insn_chain *chain, bool tentative)
{
int i;
@@ -2012,9 +2015,12 @@ find_reload_regs (struct insn_chain *chain)
{
if (dump_file)
fprintf (dump_file, "reload failure for reload %d\n", r);
- spill_failure (chain->insn, rld[r].rclass);
- failure = 1;
- return;
+ if (!tentative)
+ {
+ spill_failure (chain->insn, rld[r].rclass);
+ failure = 1;
+ return;
+ }
}
}
@@ -2025,14 +2031,14 @@ find_reload_regs (struct insn_chain *chain)
}
static void
-select_reload_regs (void)
+select_reload_regs (bool tentative)
{
struct insn_chain *chain;
/* Try to satisfy the needs for each insn. */
for (chain = insns_need_reload; chain != 0;
chain = chain->next_need_reload)
-find_reload_regs (chain);
+find_reload_regs (chain, tentative);
}
/* Delete all insns that were inserted by emit_caller_save_insns during
diff --git gcc/testsuite/gcc.target/avr/pr60040-1.c
gcc/testsuite/gcc.target/avr/pr60040-1.c
new file mode 100644
index 000..4fe296b
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr60040-1.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+
+float dhistory[10];
+float test;
+
+float getSlope(float history[]) {
+ float sumx = 0;
+ float sumy = 0;
+ float sumxy = 0;
+ float sumxsq = 0;
+ float rate = 0;
+ int n = 10;
+
+ int i;
+ for (i=1; i< 11; i++) {
+sumx = sumx + i;
+sumy = sumy + history[i-1];
+sumy = sumy + history[i-1];
+sumxsq = sumxsq + (i*i);
+ }
+
+ rate = sumy+sumx+sumxsq;
+ return rate;
+}
+
+void loop() {
+ test = getSlope(dhistory);
+}
diff --git gcc/testsuite/gcc.target/avr/pr60040-2.c
gcc/testsuite/gcc.target/avr/pr60040-2.c
new file mode 100644
index 000..c40d49f
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr60040-2.c
@@ -0,0 +1,112 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef unsigned char __uint8_t;
+typedef short unsigned int __uint16_t;
+typedef long unsigned int __uint32_t;
+typedef __uint8_t uint8_t ;
+typedef
Re: [Patch, testsuite] Require int32plus and scheduling support for some tests
Ping!
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Hi,
>
> This patch add dg-require-effective-target directives to a few tests
> that were failing unnecessarily for the AVR target.
>
> One of them invokes the compiler with -fschedule-insns2 - I've
> required scheduling support for that testcase. For all other tests,
> I've required int32plus - they either use bit shifts wider than
> 16 bits (AVR's int size), or use int constants that are too big
> for a 16 bit int.
>
> If ok, could someone commit please? I don't have commit access.
>
> Regards
> Senthil
>
> 2016-04-04 Senthil Kumar Selvaraj
>
> * gcc.c-torture/compile/pr69102.c: Require scheduling support.
> * gcc.c-torture/compile/pr37669.c: Require >=32 bit integers.
> * gcc.c-torture/execute/bitfld-6.c: Likewise.
> * gcc.c-torture/execute/bitfld-7.c: Likewise.
> * gcc.c-torture/execute/pr38151.c: Likewise.
> * gcc.c-torture/execute/pr66556.c: Likewise.
> * gcc.c-torture/execute/pr67781.c: Likewise.
> * gcc.c-torture/execute/pr68648.c: Likewise.
>
>
> diff --git gcc/testsuite/gcc.c-torture/compile/pr37669.c
> gcc/testsuite/gcc.c-torture/compile/pr37669.c
> index c78243b..a2eafc7 100644
> --- gcc/testsuite/gcc.c-torture/compile/pr37669.c
> +++ gcc/testsuite/gcc.c-torture/compile/pr37669.c
> @@ -1,5 +1,6 @@
> /* This testcase used to fail because a miscompiled
> execute_fold_all_builtins. */
> /* { dg-options "-fgnu89-inline" } */
> +/* { dg-require-effective-target int32plus } */
>
> typedef __SIZE_TYPE__ size_t;
> extern __inline __attribute__ ((__always_inline__)) int __attribute__
> diff --git gcc/testsuite/gcc.c-torture/compile/pr69102.c
> gcc/testsuite/gcc.c-torture/compile/pr69102.c
> index b1328ca..1f0cdc6 100644
> --- gcc/testsuite/gcc.c-torture/compile/pr69102.c
> +++ gcc/testsuite/gcc.c-torture/compile/pr69102.c
> @@ -1,4 +1,5 @@
> /* { dg-options "-Og -fPIC -fschedule-insns2 -fselective-scheduling2
> -fno-tree-fre --param=max-sched-extend-regions-iters=10" } */
> +/* { dg-require-effective-target scheduling } */
> void bar (unsigned int);
>
> void
> diff --git gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> index 50927dc..b8c5cbd 100644
> --- gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> +++ gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
> union U
> {
>const int a;
> diff --git gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
> gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
> index e9a61df..350e7a3 100644
> --- gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
> +++ gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
> union U
> {
>const int a;
> diff --git gcc/testsuite/gcc.c-torture/execute/pr38151.c
> gcc/testsuite/gcc.c-torture/execute/pr38151.c
> index 5ee058d..86c8f77 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr38151.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr38151.c
> @@ -1,4 +1,5 @@
> /* { dg-options "-Wno-psabi" } */
> +/* { dg-require-effective-target int32plus } */
> void abort (void);
>
> struct S2848
> diff --git gcc/testsuite/gcc.c-torture/execute/pr66556.c
> gcc/testsuite/gcc.c-torture/execute/pr66556.c
> index f7acf1c..d1259c4 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr66556.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr66556.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-require-effective-target int32plus } */
>
> extern void abort (void);
>
> diff --git gcc/testsuite/gcc.c-torture/execute/pr67781.c
> gcc/testsuite/gcc.c-torture/execute/pr67781.c
> index bf50aa2..71ccd6a 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr67781.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr67781.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
> #ifdef __UINT32_TYPE__
> typedef __UINT32_TYPE__ uint32_t;
> #else
> diff --git gcc/testsuite/gcc.c-torture/execute/pr68648.c
> gcc/testsuite/gcc.c-torture/execute/pr68648.c
> index fc66806..db55bd0 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr68648.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr68648.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
> int __attribute__ ((noinline))
> foo (void)
> {
Re: [Patch] Fix PR 60040
Bernd Schmidt writes:
> On 04/07/2016 01:52 PM, Senthil Kumar Selvaraj wrote:
>>The below patch fixes PR 60040 by not halting with a hard error on
>>a spill failure, if reload knows that it has to run again anyway.
>
> Some additional information as to how this situation creates a spill
> failure would be useful. It's hard to tell whether this patch just
> papers over a problem that can still trigger in other circumstances.
For both testcases in the PR, reload fails to take into account that
FP-SP elimination can no longer be performed, and tries to find reload
regs for an rtx generated when FP-SP elimination was valid.
1. reload initializes elim table with FP->SP elimination enabled.
2. alter_reg for a pseudo allocates a stack slot for the pseudo, and sets
reg_equiv_memory_loc to frame_pointer_rtx plus offset. It also sets
something_was_spilled to true.
3. The main reload loop starts, and it resets something_was_spilled to false.
4. reload calls eliminate_regs for the pseudo and sets reg_equiv_address to
(mem(SP + offset)).
5. calculate_needs_all_insns pushes a reload for SP (for the AVR target,
SP cannot be a pointer reg - it needs to be reloaded into X Y or Z regs).
6. update_eliminables_and_spill calls targetm.frame_pointer_required,
which returns true. That causes can_eliminate for FP->SP to be reset
to zero, and FP to be added to bad_spill_regs_global. For the AVR,
FP is Y, one of the 3 pointer regs. reload also notes that something
has changed, and that the loop needs to run again.
7. reload still calls select_reload_regs, and find_regs fails to find a
pointer reg to reload SP, which is unnecessary as FP->SP elimination
had been disabled anyway in (6).
IOW, reload fails to find pointer regs for an RTL expression that was
created when FP->SP elimination was true, even after it turns out that
the elimination can't be done after all. The patch tries to detect that
- if it knows the loop is going to run again, it silences the failure.
Also note that at a different point in the loop, the reload loop starts
over if something_was_spilled (line 982-986). If set outside the reload
loop by alter_reg, it gets reset at (3) - not sure why. I'd think a
"continue" after update_eliminables_and_spill (line 1019-1022) would
also work - haven't tested it though.
What do you think?
>
>> -spill_failure (chain->insn, rld[r].rclass);
>> -failure = 1;
>> -return;
>> +if (!tentative)
>> +{
>> +spill_failure (chain->insn, rld[r].rclass);
>> +failure = 1;
>> +return;
>> +}
>> }
>
> The indentation looks all wrong.
>
Fixed now - mixed up tabs and spaces.
gcc/ChangeLog
2016-04-07 Joern Rennecke
Senthil Kumar Selvaraj
PR target/60040
* reload1.c (find_reload_regs): Add tentative parameter.
and don't report spill failure if param set.
(reload): Propagate something_changed to
select_reload_regs.
(select_reload_regs): Add tentative parameter.
gcc/testsuite/ChangeLog
2016-04-07 Sebastian Huber
Matthijs Kooijman
Senthil Kumar Selvaraj
PR target/60040
* gcc.target/avr/pr60040-1.c: New.
* gcc.target/avr/pr60040-2.c: Likewise.
diff --git gcc/reload1.c gcc/reload1.c
index c2800f8..58993a3 100644
--- gcc/reload1.c
+++ gcc/reload1.c
@@ -346,8 +346,8 @@ static void maybe_fix_stack_asms (void);
static void copy_reloads (struct insn_chain *);
static void calculate_needs_all_insns (int);
static int find_reg (struct insn_chain *, int);
-static void find_reload_regs (struct insn_chain *);
-static void select_reload_regs (void);
+static void find_reload_regs (struct insn_chain *, bool);
+static void select_reload_regs (bool);
static void delete_caller_save_insns (void);
static void spill_failure (rtx_insn *, enum reg_class);
@@ -1022,7 +1022,7 @@ reload (rtx_insn *first, int global)
something_changed = 1;
}
- select_reload_regs ();
+ select_reload_regs (something_changed);
if (failure)
goto failed;
@@ -1960,10 +1960,13 @@ find_reg (struct insn_chain *chain, int order)
is given by CHAIN.
Do it by ascending class number, since otherwise a reg
might be spilled for a big class and might fail to count
- for a smaller class even though it belongs to that class. */
+ for a smaller class even though it belongs to that class.
+ TENTATIVE means that we had some changes that might have invalidated
+ the reloads and that we are going to loop again anyway, so don't give
+ a hard error on failure to find a reload reg. */
static void
-find_reload_regs (struct insn_chain *chain)
+find_reload_regs (stru
Re: [Patch] Fix PR 60040
Ping!
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Bernd Schmidt writes:
>
>> On 04/07/2016 01:52 PM, Senthil Kumar Selvaraj wrote:
>>>The below patch fixes PR 60040 by not halting with a hard error on
>>>a spill failure, if reload knows that it has to run again anyway.
>>
>> Some additional information as to how this situation creates a spill
>> failure would be useful. It's hard to tell whether this patch just
>> papers over a problem that can still trigger in other circumstances.
>
> For both testcases in the PR, reload fails to take into account that
> FP-SP elimination can no longer be performed, and tries to find reload
> regs for an rtx generated when FP-SP elimination was valid.
>
> 1. reload initializes elim table with FP->SP elimination enabled.
> 2. alter_reg for a pseudo allocates a stack slot for the pseudo, and sets
>reg_equiv_memory_loc to frame_pointer_rtx plus offset. It also sets
>something_was_spilled to true.
> 3. The main reload loop starts, and it resets something_was_spilled to false.
> 4. reload calls eliminate_regs for the pseudo and sets reg_equiv_address to
>(mem(SP + offset)).
> 5. calculate_needs_all_insns pushes a reload for SP (for the AVR target,
>SP cannot be a pointer reg - it needs to be reloaded into X Y or Z regs).
> 6. update_eliminables_and_spill calls targetm.frame_pointer_required,
>which returns true. That causes can_eliminate for FP->SP to be reset
>to zero, and FP to be added to bad_spill_regs_global. For the AVR,
>FP is Y, one of the 3 pointer regs. reload also notes that something
>has changed, and that the loop needs to run again.
> 7. reload still calls select_reload_regs, and find_regs fails to find a
>pointer reg to reload SP, which is unnecessary as FP->SP elimination
>had been disabled anyway in (6).
>
> IOW, reload fails to find pointer regs for an RTL expression that was
> created when FP->SP elimination was true, even after it turns out that
> the elimination can't be done after all. The patch tries to detect that
> - if it knows the loop is going to run again, it silences the failure.
>
> Also note that at a different point in the loop, the reload loop starts
> over if something_was_spilled (line 982-986). If set outside the reload
> loop by alter_reg, it gets reset at (3) - not sure why. I'd think a
> "continue" after update_eliminables_and_spill (line 1019-1022) would
> also work - haven't tested it though.
>
> What do you think?
>
>
>>
>>> - spill_failure (chain->insn, rld[r].rclass);
>>> - failure = 1;
>>> - return;
>>> + if (!tentative)
>>> + {
>>> + spill_failure (chain->insn, rld[r].rclass);
>>> + failure = 1;
>>> + return;
>>> + }
>>> }
>>
>> The indentation looks all wrong.
>>
>
> Fixed now - mixed up tabs and spaces.
>
> gcc/ChangeLog
>
> 2016-04-07 Joern Rennecke
> Senthil Kumar Selvaraj
>
> PR target/60040
> * reload1.c (find_reload_regs): Add tentative parameter.
> and don't report spill failure if param set.
> (reload): Propagate something_changed to
> select_reload_regs.
> (select_reload_regs): Add tentative parameter.
>
> gcc/testsuite/ChangeLog
>
> 2016-04-07 Sebastian Huber
> Matthijs Kooijman
> Senthil Kumar Selvaraj
>
> PR target/60040
> * gcc.target/avr/pr60040-1.c: New.
> * gcc.target/avr/pr60040-2.c: Likewise.
>
> diff --git gcc/reload1.c gcc/reload1.c
> index c2800f8..58993a3 100644
> --- gcc/reload1.c
> +++ gcc/reload1.c
> @@ -346,8 +346,8 @@ static void maybe_fix_stack_asms (void);
> static void copy_reloads (struct insn_chain *);
> static void calculate_needs_all_insns (int);
> static int find_reg (struct insn_chain *, int);
> -static void find_reload_regs (struct insn_chain *);
> -static void select_reload_regs (void);
> +static void find_reload_regs (struct insn_chain *, bool);
> +static void select_reload_regs (bool);
> static void delete_caller_save_insns (void);
>
> static void spill_failure (rtx_insn *, enum reg_class);
> @@ -1022,7 +1022,7 @@ reload (rtx_insn *first, int global)
> something_changed = 1;
> }
>
> - select_reload_regs ();
> + select_reload_regs (something_changed);
>if (failure)
> goto failed;
>
> @@ -1960,10 +1960,13 @@ find_reg (struct insn_chain *chain, int order)
> is given
[Patch, testsuite] Require int32plus for pr70421.c
Hi,
This patch fixes a bogus testsuite failure for the avr target. The
test has integer literals that only fit on targets with int size >= 32.
Committed to trunk.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-09-16 Senthil Kumar Selvaraj
* gcc.dg/torture/pr70421.c: Require int32plus.
Index: gcc.dg/torture/pr70421.c
===
--- gcc.dg/torture/pr70421.c(revision 240004)
+++ gcc.dg/torture/pr70421.c(working copy)
@@ -1,5 +1,6 @@
/* PR target/70421 */
/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
/* { dg-additional-options "-Wno-psabi -w" } */
typedef unsigned V __attribute__ ((vector_size (64)));
[Patch, reload, tentative, PR 71627] Tweak conditions in find_valid_class_1
Hi,
The below patch fixes what I think are a couple of problems with
reload.c:find_valid_class_1.
First, even if no regno is in_hard_reg_set_p, it goes ahead and
considers rclass as valid. bad is set only if a regno is in the reg
class *and* HARD_REGNO_MODE_OK is false - if both are false, bad isn't
set and the reload gets a wrong rclass. If that happens to be the best
one, this eventually leads to find_reg running out of registers to
spill, because the chosen rclass won't have enough regs.
Second, it expects every regno in rclass to be valid for mode i.e., if
any regno fails HARD_REGNO_MODE_OK, it rejects the rclass. The
comments in the original commit for find_valid_class_1 say atleast one
regno is ok. This was updated to say "class which contains only
registers" when in_hard_reg_set_p was introduced in place of just
TEST_HARD_REG_BIT.
Is it meant to really test all regs? For the avr target, all regs are
8 bits wide, and HARD_REGNO_MODE_OK returns false for odd regnos if
mode != QImode. With the current code, it ignores a bunch of otherwise
perfectly legal reg classes.
To fix the first problem, the patch adds regno_in_rclass_mode to track
whether there's atleast one regno in hard_reg_set_p. To fix the second
problem, the patch sets bad initially, and resets it if
HARD_REGNO_MODE_OK succeeded for a regno, thus breaking the loop.
Of course, if both my points above are valid, we can do away with
regno_in_rclass_mode, just bad would do.
Does this make sense? I ran a reg test for the avr target with a
slightly older version of this patch, it did not show any regressions.
If this is the right fix, I'll make sure to run reg tests on x86_64
after backporting to a gcc version where that target used reload.
Regards
Senthil
Index: reload.c
===
--- reload.c(revision 240185)
+++ reload.c(working copy)
@@ -714,17 +714,22 @@
for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
{
- int bad = 0;
- for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
- {
- if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
- && !HARD_REGNO_MODE_OK (regno, mode))
- bad = 1;
- }
-
- if (bad)
- continue;
+ int bad = 1;
+ int regno_in_rclass_mode = 0;
+ for (regno = 0; regno < FIRST_PSEUDO_REGISTER && bad; regno++)
+{
+ if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno))
+{
+ regno_in_rclass_mode = 1;
+ if (HARD_REGNO_MODE_OK (regno, mode))
+bad = 0;
+}
+}
+
+ if (bad || !regno_in_rclass_mode)
+continue;
+
cost = register_move_cost (outer, (enum reg_class) rclass, dest_class);
if ((reg_class_size[rclass] > best_size
[Patch, testsuite] Make pr64130.c pass for avr
Hi,
For the lower vrp bound to be 2/-2, unsigned ints must be 4 bytes wide. This
isn't true for targets like avr. Explicitly using __UINT32_TYPE__
behind a typedef makes the testcase pass for all targets.
Committed to trunk.
Regards
Senthil
gcc/testsuite/ChangeLog:
2016-09-21 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/pr64130.c: Use __UINT32_TYPE__ instead of int.
Index: gcc.dg/tree-ssa/pr64130.c
===
--- gcc.dg/tree-ssa/pr64130.c (revision 240299)
+++ gcc.dg/tree-ssa/pr64130.c (revision 240300)
@@ -2,12 +2,14 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-evrp" } */
-int funsigned (unsigned a)
+__extension__ typedef __UINT32_TYPE__ uint32_t;
+
+int funsigned (uint32_t a)
{
return 0x1L / a == 0;
}
-int funsigned2 (unsigned a)
+int funsigned2 (uint32_t a)
{
if (a < 1) return 1;
return (-1 * 0x1L) / a == 0;
[Patch, avr] Provide correct LD offset bound in avr_address_cost
Hi,
This patch fixes cost computation in avr_address_cost - instead of the
hardcoded 61, it uses the already existing MAX_LD_OFFSET(mode) macro.
This showed up when investigating a code size regression in the ivopts
pass. That pass computes address_cost with and without an offset to
decide on the right induction variable candidate(s). The legitimate
address target hook returns false for offsets more than 63, so the
pass calls the TARGET_ADDRESS_COST hook with 62 as the offset.
The hook implementation returns 18, and the ivopts pass concludes that
the cost of address with *any* offset is 18, which is not true - the higher
cost is incurred only with offsets bigger than MAX_LD_OFFSET. This in
turn results in a suboptimal choice of induction variables in the
ivopts pass. The patch changes the hardcoded 61 to use the mode
specific MAX_LD_OFFSET instead.
Regression testing with just that fix showed one additional
compilation timeout. That turned out to be the same as
https://lists.nongnu.org/archive/html/avr-gcc-list/2014-03/msg00010.html
- the middle end takes too much time to decide on the best strategy to
multiply DImode values on a 64 bit host. This already causes timeouts
for a few builtin-arith-overflow-* tests (see
https://gcc.gnu.org/ml/gcc-testresults/2016-09/msg02018.html), so it
isn't really related to this fix. Just providing a cost estimate for
DImode mul fixes the timeout though, so the patch does that by scaling
SImode costs by 2 for DImode muls.
With both changes in, there are no regressions, and the
builtin-arith-overflow-* tests now PASS and don't timeout.
Ok to commit to trunk and backport to 5.x?
Regards
Senthil
gcc/ChangeLog:
2016-09-21 Senthil Kumar Selvaraj
* config/avr/avr.c (avr_rtx_costs_1): Handle DImode MULT.
(avr_address_cost): Replace 61 with MAX_LD_OFFSET(mode).
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 148a61d..29f0022 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -10257,6 +10257,7 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int
outer_code ATTRIBUTE_UNUSED,
break;
case SImode:
+ case DImode:
if (AVR_HAVE_MUL)
{
if (!speed)
@@ -10282,7 +10283,10 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int
outer_code ATTRIBUTE_UNUSED,
*total = COSTS_N_INSNS (AVR_HAVE_JMP_CALL ? 5 : 4);
}
- return true;
+ if (mode == DImode)
+*total *= 2;
+
+ return true;
default:
return false;
@@ -10863,7 +10867,7 @@ avr_address_cost (rtx x, machine_mode mode
ATTRIBUTE_UNUSED,
&& (REG_P (XEXP (x, 0))
|| GET_CODE (XEXP (x, 0)) == SUBREG))
{
- if (INTVAL (XEXP (x, 1)) >= 61)
+ if (INTVAL (XEXP (x, 1)) > MAX_LD_OFFSET(mode))
cost = 18;
}
else if (CONSTANT_ADDRESS_P (x))
Backport fix for PR 65210 to gcc-5-branch
Hi,
Is it ok to backport PR 65210 to gcc-5-branch? The patch is already in
6.x and trunk.
Regards
Senthil
gcc/ChangeLog
2016-09-22 Senthil Kumar Selvaraj
Backport from trunk r227496
PR target/65210
* config/avr/avr.c (avr_eval_addr_attrib): Look for io_low
attribute as well.
gcc/testsuite/ChangeLog
2016-09-22 Senthil Kumar Selvaraj
Backport from trunk r227496
PR target/65210
* gcc.target/avr/pr65210.c: New test.
Index: gcc/config/avr/avr.c
===
--- gcc/config/avr/avr.c(revision 240340)
+++ gcc/config/avr/avr.c(working copy)
@@ -9122,6 +9122,8 @@
if (SYMBOL_REF_FLAGS (x) & SYMBOL_FLAG_IO)
{
attr = lookup_attribute ("io", DECL_ATTRIBUTES (decl));
+ if (!attr || !TREE_VALUE (attr))
+ attr = lookup_attribute ("io_low", DECL_ATTRIBUTES (decl));
gcc_assert (attr);
}
if (!attr || !TREE_VALUE (attr))
Index: gcc/testsuite/gcc.target/avr/pr65210.c
===
--- gcc/testsuite/gcc.target/avr/pr65210.c (nonexistent)
+++ gcc/testsuite/gcc.target/avr/pr65210.c (working copy)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+
+/* This testcase exposes PR65210. Usage of the io_low attribute
+ causes assertion failure because code only looks for the io
+ attribute if SYMBOL_FLAG_IO is set. */
+
+volatile char q __attribute__((io_low,address(0x81)));
Re: [Patch, avr] Backport fix for PR 65210 to gcc-5-branch
Ping!
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Hi,
>
> Is it ok to backport PR 65210 to gcc-5-branch? The patch is already in
> 6.x and trunk.
>
> Regards
> Senthil
>
> gcc/ChangeLog
>
> 2016-09-22 Senthil Kumar Selvaraj
>
> Backport from trunk r227496
>
> PR target/65210
> * config/avr/avr.c (avr_eval_addr_attrib): Look for io_low
> attribute as well.
>
> gcc/testsuite/ChangeLog
>
> 2016-09-22 Senthil Kumar Selvaraj
>
> Backport from trunk r227496
>
> PR target/65210
> * gcc.target/avr/pr65210.c: New test.
>
> Index: gcc/config/avr/avr.c
> ===
> --- gcc/config/avr/avr.c (revision 240340)
> +++ gcc/config/avr/avr.c (working copy)
> @@ -9122,6 +9122,8 @@
>if (SYMBOL_REF_FLAGS (x) & SYMBOL_FLAG_IO)
> {
> attr = lookup_attribute ("io", DECL_ATTRIBUTES (decl));
> + if (!attr || !TREE_VALUE (attr))
> + attr = lookup_attribute ("io_low", DECL_ATTRIBUTES (decl));
> gcc_assert (attr);
> }
>if (!attr || !TREE_VALUE (attr))
>Index: gcc/testsuite/gcc.target/avr/pr65210.c
> ===
> --- gcc/testsuite/gcc.target/avr/pr65210.c(nonexistent)
> +++ gcc/testsuite/gcc.target/avr/pr65210.c(working copy)
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +
> +/* This testcase exposes PR65210. Usage of the io_low attribute
> + causes assertion failure because code only looks for the io
> + attribute if SYMBOL_FLAG_IO is set. */
> +
> +volatile char q __attribute__((io_low,address(0x81)));
Re: [Patch, reload, tentative, PR 71627] Tweak conditions in find_valid_class_1
Ping!
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Hi,
>
> The below patch fixes what I think are a couple of problems with
> reload.c:find_valid_class_1.
>
> First, even if no regno is in_hard_reg_set_p, it goes ahead and
> considers rclass as valid. bad is set only if a regno is in the reg
> class *and* HARD_REGNO_MODE_OK is false - if both are false, bad isn't
> set and the reload gets a wrong rclass. If that happens to be the best
> one, this eventually leads to find_reg running out of registers to
> spill, because the chosen rclass won't have enough regs.
>
> Second, it expects every regno in rclass to be valid for mode i.e., if
> any regno fails HARD_REGNO_MODE_OK, it rejects the rclass. The
> comments in the original commit for find_valid_class_1 say atleast one
> regno is ok. This was updated to say "class which contains only
> registers" when in_hard_reg_set_p was introduced in place of just
> TEST_HARD_REG_BIT.
>
> Is it meant to really test all regs? For the avr target, all regs are
> 8 bits wide, and HARD_REGNO_MODE_OK returns false for odd regnos if
> mode != QImode. With the current code, it ignores a bunch of otherwise
> perfectly legal reg classes.
>
> To fix the first problem, the patch adds regno_in_rclass_mode to track
> whether there's atleast one regno in hard_reg_set_p. To fix the second
> problem, the patch sets bad initially, and resets it if
> HARD_REGNO_MODE_OK succeeded for a regno, thus breaking the loop.
>
> Of course, if both my points above are valid, we can do away with
> regno_in_rclass_mode, just bad would do.
>
> Does this make sense? I ran a reg test for the avr target with a
> slightly older version of this patch, it did not show any regressions.
> If this is the right fix, I'll make sure to run reg tests on x86_64
> after backporting to a gcc version where that target used reload.
>
> Regards
> Senthil
>
>
> Index: reload.c
> ===
> --- reload.c (revision 240185)
> +++ reload.c (working copy)
> @@ -714,17 +714,22 @@
>
>for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
> {
> - int bad = 0;
> - for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
> - {
> - if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
> - && !HARD_REGNO_MODE_OK (regno, mode))
> - bad = 1;
> - }
> -
> - if (bad)
> - continue;
> + int bad = 1;
> + int regno_in_rclass_mode = 0;
>
> + for (regno = 0; regno < FIRST_PSEUDO_REGISTER && bad; regno++)
> +{
> + if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno))
> +{
> + regno_in_rclass_mode = 1;
> + if (HARD_REGNO_MODE_OK (regno, mode))
> +bad = 0;
> +}
> +}
> +
> + if (bad || !regno_in_rclass_mode)
> +continue;
> +
>cost = register_move_cost (outer, (enum reg_class) rclass, dest_class);
>
>if ((reg_class_size[rclass] > best_size
[Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c
Hi,
This patch requires int32plus for
gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
failures for a 16 bit int target like the avr. The "%u" format
specifier tests, for example, use int literals big enough to only fit
in a long int, and this causes unexpected warnings.
Comitted to trunk.
Regards
Senthil
2016-09-27 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus.
PR fortran/77666
Index: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
===
--- gcc.dg/tree-ssa/builtin-sprintf-warn-1.c(revision 240524)
+++ gcc.dg/tree-ssa/builtin-sprintf-warn-1.c(working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-std=c99 -Wformat -Wformat-length=1
-ftrack-macro-expansion=0" } */
+/* { dg-require-effective-target int32plus } */
/* When debugging, define LINE to the line number of the test case to exercise
and avoid exercising any of the others. The buffer and objsize macros
Re: [Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c
James Greenhalgh writes:
> On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote:
>> Hi,
>>
>> This patch requires int32plus for
>> gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
>> failures for a 16 bit int target like the avr. The "%u" format
>> specifier tests, for example, use int literals big enough to only fit
>> in a long int, and this causes unexpected warnings.
>>
>> Comitted to trunk.
>
> This change is obviously incomplete as it does not update the expected
> line numbers for warnings generated by this testcase.
Sorry for the breakage. While I tested that it reports UNSUPPORTED for
avr, I didn't test that it doesn't break other targets. I thought I'd
just modified behavior to skip the test, didn't realize the side effect
of adding a new line.
Guess Martin already has a patch fixing this and a couple of other
things (https://gcc.gnu.org/ml/gcc-patches/2016-09/msg02073.html).
Thanks Martin!
Regards
Senthil
>
> Found with my bisect robot:
>
> Failures:
> gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
>
> Bisected to:
>
> Author: saaadhu
> Date: Tue Sep 27 11:05:25 2016 +
>
> Fix bogus test failure for avr
>
> The test has a bunch of hardcoded integer literals that would fit only in
> a
> 32 bits+ int, causing overflow warnings for a 16 bit int target like avr.
>
> gcc/testsuite/ChangeLog
>
> 2016-09-27 Senthil Kumar Selvaraj
>
> * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus.
>
>
> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240528
>
>
> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (nil) (test for warnings,
> line 96)
> /* { dg-warning "nul past the end" "(nil)" { target *-linux-gnu
> *-*-uclinux } 96 } */
>
> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c Glibc %p (test for warnings,
> line 108)
> /* { dg-warning "nul past the end" "Glibc %p" { target *-linux-gnu } 108
> } */
> /* { dg-warning "nul past the end" "Generic %p" { target *-*-uclinux }
> 108 } */
>
> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)
>
> The line numbers here need bumped to match the change you've made.
>
> Thanks,
> James
>
>
>> 2016-09-27 Senthil Kumar Selvaraj
>>
>> * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus.
>>
>> PR fortran/77666
>> Index: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
>> ===
>> --- gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (revision 240524)
>> +++ gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (working copy)
>> @@ -1,5 +1,6 @@
>> /* { dg-do compile } */
>> /* { dg-options "-std=c99 -Wformat -Wformat-length=1
>> -ftrack-macro-expansion=0" } */
>> +/* { dg-require-effective-target int32plus } */
>>
>> /* When debugging, define LINE to the line number of the test case to
>> exercise
>> and avoid exercising any of the others. The buffer and objsize macros
>>
[Patch, testsuite] Add ffat-lto-objects to gcc.target/avr/torture/builtins_error.c
Hi,
This patch adds -ffat-lto-objects option to an avr target testcase.
The compiler defaults to thin LTO objects if built with linker plugin
support, and the error expected by the testcase appears only at link
time, if at all. Forcing fat LTO object file creation generates the
error consistently at compile, as expected.
Committed to trunk.
Regards
Senthil
gcc/testsuite/ChangeLog:
2016-10-03 Senthil Kumar Selvaraj
* gcc.target/avr/torture/builtins-error.c: Add -ffat-lto-objects
option.
Index: gcc/testsuite/gcc.target/avr/torture/builtins-error.c
===
--- gcc/testsuite/gcc.target/avr/torture/builtins-error.c (revision
240709)
+++ gcc/testsuite/gcc.target/avr/torture/builtins-error.c (working copy)
@@ -1,4 +1,5 @@
/* { dg-do assemble } */
+/* { dg-options "-ffat-lto-objects" } */
char insert (long a)
{
[Patch, testsuite] Fix pr69941.c test failure for avr
Hi,
The below patch fixes gcc.dg/torture/pr69941.c to pass for
int size != 32 targets like avr.
For the avr target, ints are 16 bits wide. VRP concludes
that a right shift by 9 followed by an equality comparison
to 0x74 can never be true, and ends up eliminating the
conditional. The code ends up unconditionally
calling __builtin_abort and obviously fails when run.
The patch fixes the testcase to use __INT32_TYPE__ (via a
typedef) if __SIZEOF_INT__ is less than 4.
Regtested with both avr and x86_64, the test passes with
both targets.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-10-05 Senthil Kumar Selvaraj
* gcc.dg/torture/pr69941.c: Use __INT32_TYPE__ instead
of int if __SIZEOF_INT__ is less than 4 bytes.
Index: gcc/testsuite/gcc.dg/torture/pr69941.c
===
--- gcc/testsuite/gcc.dg/torture/pr69941.c (revision 240781)
+++ gcc/testsuite/gcc.dg/torture/pr69941.c (working copy)
@@ -1,11 +1,17 @@
/* { dg-do run } */
+
+#if __SIZEOF_INT__ < 4
+__extension__ typedef __INT32_TYPE__ int32_t;
+#else
+typedef int int32_t;
+#endif
int a = 0;
int b = 0;
int c = 0;
-int e = 0;
+int32_t e = 0;
int f = 0;
-int *g = &e;
+int32_t *g = &e;
int fn1() { return b ? a : b; }
[Patch, testsuite] Fix gcc.g/tree-ssa/pr59597.c failure for avr
Hi,
This patch declares loop index variable j as a 32 bit int instead of
assuming ints are 32 bits. The smaller int size on the avr makes prior
passes optimize away the loop exit check (j < 1000), as the constant
is outside the range of a 16 bit int.
Committed to trunk, after reg testing with avr and x86_64-pc-linux
Regards
Senthil
gcc/testsuite/ChangeLog
2016-10-11 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/pr59597.c: Typedef __INT32_TYPE__ to i32.
(main): Declare j as i32 instead of int.
Index: gcc/testsuite/gcc.dg/tree-ssa/pr59597.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr59597.c (revision 240974)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr59597.c (working copy)
@@ -4,6 +4,8 @@
typedef unsigned short u16;
typedef unsigned char u8;
typedef unsigned int u32;
+__extension__ typedef __INT32_TYPE__ i32;
+
long int random(int);
#define NNN 10
@@ -37,7 +39,7 @@
int
main (int argc, char argv[])
{
- int i, j; u16 crc;
+ int i; i32 j; u16 crc;
for (j = 0; j < 1000; j++)
{
for (i = 0; i < NNN; i++)
Re: [Patch, reload, tentative, PR 71627] Tweak conditions in find_valid_class_1
Bernd Schmidt writes:
> On 09/16/2016 09:02 PM, Senthil Kumar Selvaraj wrote:
>> Does this make sense? I ran a reg test for the avr target with a
>> slightly older version of this patch, it did not show any regressions.
>> If this is the right fix, I'll make sure to run reg tests on x86_64
>> after backporting to a gcc version where that target used reload.
>
> It's hard to say, and could have different effects on different targets.
> One thing though, at the very least the reg_class_size test would have
> to be adapted - the idea is to find the largest class, and there's a
> risk here of ending up with a large class that only has one valid register.
Agreed - I've updated the patch to compute rclass sizes based on regno
availability i.e., only if in_hard_reg_set_p and HARD_REGNO_MODE_OK, and
then use the computed sizes when calculating best_size.
>
> You'll also want to verify this against
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54814
Yes, this patch doesn't break the fix for PR54814. The change to
in_hard_reg_set_p was what fixed that, and that remains unmodified.
Reg tested this on top of trunk@190252 with the in_hard_reg_set_p
backport. x86_64-pc-linux bootstrapped and regtested ok. avr showed
no regressions either.
Ok for trunk?
Regards
Senthil
gcc/ChangeLog:
2016-10-13 Senthil Kumar Selvaraj
* reload.c (find_valid_class_1): Allow regclass if atleast one
regno in class is ok. Compute and use rclass size based on
actually available regnos for mode in rclass.
gcc/testsuite/ChangeLog:
2016-10-13 Senthil Kumar Selvaraj
* gcc.target/avr/pr71627.c: New.
Index: gcc/reload.c
===
--- gcc/reload.c(revision 240989)
+++ gcc/reload.c(working copy)
@@ -711,31 +711,36 @@
enum reg_class best_class = NO_REGS;
unsigned int best_size = 0;
int cost;
+ unsigned int computed_rclass_sizes[N_REG_CLASSES] = { 0 };
for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
{
- int bad = 0;
- for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
- {
- if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
- && !HARD_REGNO_MODE_OK (regno, mode))
- bad = 1;
- }
-
- if (bad)
- continue;
+ int atleast_one_regno_ok = 0;
+ for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+{
+ if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno))
+{
+ atleast_one_regno_ok = 1;
+ if (HARD_REGNO_MODE_OK (regno, mode))
+computed_rclass_sizes[rclass]++;
+}
+}
+
+ if (!atleast_one_regno_ok)
+continue;
+
cost = register_move_cost (outer, (enum reg_class) rclass, dest_class);
- if ((reg_class_size[rclass] > best_size
- && (best_cost < 0 || best_cost >= cost))
- || best_cost > cost)
- {
- best_class = (enum reg_class) rclass;
- best_size = reg_class_size[rclass];
- best_cost = register_move_cost (outer, (enum reg_class) rclass,
- dest_class);
- }
+ if ((computed_rclass_sizes[rclass] > best_size
+ && (best_cost < 0 || best_cost >= cost))
+ || best_cost > cost)
+{
+ best_class = (enum reg_class) rclass;
+ best_size = computed_rclass_sizes[rclass];
+ best_cost = register_move_cost (outer, (enum reg_class) rclass,
+ dest_class);
+}
}
gcc_assert (best_size != 0);
Index: gcc/testsuite/gcc.target/avr/pr71627.c
===
--- gcc/testsuite/gcc.target/avr/pr71627.c (nonexistent)
+++ gcc/testsuite/gcc.target/avr/pr71627.c (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+
+extern volatile __memx const long a, b, c, d, e, f;
+extern volatile long result;
+
+extern void vfunc (const char*, ...);
+
+void foo (void)
+{
+ result = a + b + c + d + e + f;
+ vfunc ("text", a, b, c, d, e, f, result);
+}
[Patch] Backport fix for PR 52085 to gcc-5-branch?
Hi,
The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the
same issue on a gcc 5.x and found that the fix didn't get backported.
Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to
backport to gcc-5-branch?
Regards
Senthil
gcc/c/ChangeLog
2016-10-17 Senthil Kumar Selvaraj
Backport from mainline
2015-04-25 Marek Polacek
PR c/52085
* c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for "mode"
attribute.
gcc/testsuite/ChangeLog
2016-10-17 Senthil Kumar Selvaraj
Backport from mainline
2015-04-25 Marek Polacek
PR c/52085
* gcc.dg/enum-incomplete-2.c: New test.
* gcc.dg/enum-mode-1.c: New test.
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index d1e7444..c508e7f 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -8050,7 +8050,7 @@ finish_enum (tree enumtype, tree values, tree attributes)
/* If the precision of the type was specified with an attribute and it
was too small, give an error. Otherwise, use it. */
- if (TYPE_PRECISION (enumtype))
+ if (TYPE_PRECISION (enumtype) && lookup_attribute ("mode", attributes))
{
if (precision > TYPE_PRECISION (enumtype))
{
@@ -8078,6 +8078,7 @@ finish_enum (tree enumtype, tree values, tree attributes)
TYPE_MIN_VALUE (enumtype) = TYPE_MIN_VALUE (tem);
TYPE_MAX_VALUE (enumtype) = TYPE_MAX_VALUE (tem);
TYPE_UNSIGNED (enumtype) = TYPE_UNSIGNED (tem);
+ TYPE_ALIGN (enumtype) = TYPE_ALIGN (tem);
TYPE_SIZE (enumtype) = 0;
TYPE_PRECISION (enumtype) = TYPE_PRECISION (tem);
diff --git gcc/testsuite/gcc.dg/enum-incomplete-2.c
gcc/testsuite/gcc.dg/enum-incomplete-2.c
new file mode 100644
index 000..5970551
--- /dev/null
+++ gcc/testsuite/gcc.dg/enum-incomplete-2.c
@@ -0,0 +1,41 @@
+/* PR c/52085 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+#define SA(X) _Static_assert((X),#X)
+
+enum e1;
+enum e1 { A } __attribute__ ((__packed__));
+enum e2 { B } __attribute__ ((__packed__));
+SA (sizeof (enum e1) == sizeof (enum e2));
+SA (_Alignof (enum e1) == _Alignof (enum e2));
+
+enum e3;
+enum e3 { C = 256 } __attribute__ ((__packed__));
+enum e4 { D = 256 } __attribute__ ((__packed__));
+SA (sizeof (enum e3) == sizeof (enum e4));
+SA (_Alignof (enum e3) == _Alignof (enum e4));
+
+enum e5;
+enum e5 { E = __INT_MAX__ } __attribute__ ((__packed__));
+enum e6 { F = __INT_MAX__ } __attribute__ ((__packed__));
+SA (sizeof (enum e5) == sizeof (enum e6));
+SA (_Alignof (enum e5) == _Alignof (enum e6));
+
+enum e7;
+enum e7 { G } __attribute__ ((__mode__(__byte__)));
+enum e8 { H } __attribute__ ((__mode__(__byte__)));
+SA (sizeof (enum e7) == sizeof (enum e8));
+SA (_Alignof (enum e7) == _Alignof (enum e8));
+
+enum e9;
+enum e9 { I } __attribute__ ((__packed__, __mode__(__byte__)));
+enum e10 { J } __attribute__ ((__packed__, __mode__(__byte__)));
+SA (sizeof (enum e9) == sizeof (enum e10));
+SA (_Alignof (enum e9) == _Alignof (enum e10));
+
+enum e11;
+enum e11 { K } __attribute__ ((__mode__(__word__)));
+enum e12 { L } __attribute__ ((__mode__(__word__)));
+SA (sizeof (enum e11) == sizeof (enum e12));
+SA (_Alignof (enum e11) == _Alignof (enum e12));
diff --git gcc/testsuite/gcc.dg/enum-mode-1.c gcc/testsuite/gcc.dg/enum-mode-1.c
new file mode 100644
index 000..a701123
--- /dev/null
+++ gcc/testsuite/gcc.dg/enum-mode-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+
+enum e1 { A = 256 } __attribute__((__mode__(__byte__))); /* { dg-error
"specified mode too small for enumeral values" } */
+enum e2 { B = 256 } __attribute__((__packed__, __mode__(__byte__))); /* {
dg-error "specified mode too small for enumeral values" } */
+
+enum e3 { C = __INT_MAX__ } __attribute__((__mode__(__QI__))); /* { dg-error
"specified mode too small for enumeral values" } */
+enum e4 { D = __INT_MAX__ } __attribute__((__packed__, __mode__(__QI__))); /*
{ dg-error "specified mode too small for enumeral values" } */
+
+enum e5 { E = __INT_MAX__ } __attribute__((__mode__(__HI__))); /* { dg-error
"specified mode too small for enumeral values" } */
+enum e6 { F = __INT_MAX__ } __attribute__((__packed__, __mode__(__HI__))); /*
{ dg-error "specified mode too small for enumeral values" } */
Re: [Patch] Backport fix for PR 52085 to gcc-5-branch?
Richard Biener writes:
> On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj
> wrote:
>> Hi,
>>
>> The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the
>> same issue on a gcc 5.x and found that the fix didn't get backported.
>>
>> Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to
>> backport to gcc-5-branch?
>
> Ok with me but please double-check there was no fallout.
I boostrapped and ran against x86_64-pc-linux again, just to be sure.
No regressions.
I'll run the reg tests against arm-none-eabi. Can I commit it if that
passes?
Regards
Senthil
>
> Richard.
>
>> Regards
>> Senthil
>>
>> gcc/c/ChangeLog
>>
>> 2016-10-17 Senthil Kumar Selvaraj
>>
>> Backport from mainline
>> 2015-04-25 Marek Polacek
>> PR c/52085
>> * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for
>> "mode"
>> attribute.
>>
>> gcc/testsuite/ChangeLog
>> 2016-10-17 Senthil Kumar Selvaraj
>>
>> Backport from mainline
>> 2015-04-25 Marek Polacek
>> PR c/52085
>> * gcc.dg/enum-incomplete-2.c: New test.
>> * gcc.dg/enum-mode-1.c: New test.
>>
>>
>> diff --git gcc/c/c-decl.c gcc/c/c-decl.c
>> index d1e7444..c508e7f 100644
>> --- gcc/c/c-decl.c
>> +++ gcc/c/c-decl.c
>> @@ -8050,7 +8050,7 @@ finish_enum (tree enumtype, tree values, tree
>> attributes)
>>
>>/* If the precision of the type was specified with an attribute and it
>> was too small, give an error. Otherwise, use it. */
>> - if (TYPE_PRECISION (enumtype))
>> + if (TYPE_PRECISION (enumtype) && lookup_attribute ("mode", attributes))
>> {
>>if (precision > TYPE_PRECISION (enumtype))
>> {
>> @@ -8078,6 +8078,7 @@ finish_enum (tree enumtype, tree values, tree
>> attributes)
>>TYPE_MIN_VALUE (enumtype) = TYPE_MIN_VALUE (tem);
>>TYPE_MAX_VALUE (enumtype) = TYPE_MAX_VALUE (tem);
>>TYPE_UNSIGNED (enumtype) = TYPE_UNSIGNED (tem);
>> + TYPE_ALIGN (enumtype) = TYPE_ALIGN (tem);
>>TYPE_SIZE (enumtype) = 0;
>>TYPE_PRECISION (enumtype) = TYPE_PRECISION (tem);
>>
>> diff --git gcc/testsuite/gcc.dg/enum-incomplete-2.c
>> gcc/testsuite/gcc.dg/enum-incomplete-2.c
>> new file mode 100644
>> index 000..5970551
>> --- /dev/null
>> +++ gcc/testsuite/gcc.dg/enum-incomplete-2.c
>> @@ -0,0 +1,41 @@
>> +/* PR c/52085 */
>> +/* { dg-do compile } */
>> +/* { dg-options "" } */
>> +
>> +#define SA(X) _Static_assert((X),#X)
>> +
>> +enum e1;
>> +enum e1 { A } __attribute__ ((__packed__));
>> +enum e2 { B } __attribute__ ((__packed__));
>> +SA (sizeof (enum e1) == sizeof (enum e2));
>> +SA (_Alignof (enum e1) == _Alignof (enum e2));
>> +
>> +enum e3;
>> +enum e3 { C = 256 } __attribute__ ((__packed__));
>> +enum e4 { D = 256 } __attribute__ ((__packed__));
>> +SA (sizeof (enum e3) == sizeof (enum e4));
>> +SA (_Alignof (enum e3) == _Alignof (enum e4));
>> +
>> +enum e5;
>> +enum e5 { E = __INT_MAX__ } __attribute__ ((__packed__));
>> +enum e6 { F = __INT_MAX__ } __attribute__ ((__packed__));
>> +SA (sizeof (enum e5) == sizeof (enum e6));
>> +SA (_Alignof (enum e5) == _Alignof (enum e6));
>> +
>> +enum e7;
>> +enum e7 { G } __attribute__ ((__mode__(__byte__)));
>> +enum e8 { H } __attribute__ ((__mode__(__byte__)));
>> +SA (sizeof (enum e7) == sizeof (enum e8));
>> +SA (_Alignof (enum e7) == _Alignof (enum e8));
>> +
>> +enum e9;
>> +enum e9 { I } __attribute__ ((__packed__, __mode__(__byte__)));
>> +enum e10 { J } __attribute__ ((__packed__, __mode__(__byte__)));
>> +SA (sizeof (enum e9) == sizeof (enum e10));
>> +SA (_Alignof (enum e9) == _Alignof (enum e10));
>> +
>> +enum e11;
>> +enum e11 { K } __attribute__ ((__mode__(__word__)));
>> +enum e12 { L } __attribute__ ((__mode__(__word__)));
>> +SA (sizeof (enum e11) == sizeof (enum e12));
>> +SA (_Alignof (enum e11) == _Alignof (enum e12));
>> diff --git gcc/testsuite/gcc.dg/enum-mode-1.c
>> gcc/testsuite/gcc.dg/enum-mode-1.c
>> new file mode 100644
>> index 000..a701123
>> --- /dev/null
>> +++ gcc/testsuite/gcc.dg/enum-mode-1.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +
>> +enum e1 { A = 256 } __attribute__((__mode__(__byte__))); /* { dg-error
>> "specified mode too small for enumeral values" } */
>> +enum e2 { B = 256 } __attribute__((__packed__, __mode__(__byte__))); /* {
>> dg-error "specified mode too small for enumeral values" } */
>> +
>> +enum e3 { C = __INT_MAX__ } __attribute__((__mode__(__QI__))); /* {
>> dg-error "specified mode too small for enumeral values" } */
>> +enum e4 { D = __INT_MAX__ } __attribute__((__packed__, __mode__(__QI__)));
>> /* { dg-error "specified mode too small for enumeral values" } */
>> +
>> +enum e5 { E = __INT_MAX__ } __attribute__((__mode__(__HI__))); /* {
>> dg-error "specified mode too small for enumeral values" } */
>> +enum e6 { F = __INT_MAX__ } __attribute__((__packed__, __mode__(__HI__)));
>> /* { dg-error "specified mode too small for enumeral values" } */
Re: [Patch] Backport fix for PR 52085 to gcc-5-branch?
Jakub Jelinek writes: > On Tue, Oct 18, 2016 at 10:12:24AM +0200, Richard Biener wrote: >> On Mon, Oct 17, 2016 at 6:57 PM, Senthil Kumar Selvaraj >> wrote: >> > >> > Richard Biener writes: >> > >> >> On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj >> >> wrote: >> >>> Hi, >> >>> >> >>> The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the >> >>> same issue on a gcc 5.x and found that the fix didn't get backported. >> >>> >> >>> Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to >> >>> backport to gcc-5-branch? >> >> >> >> Ok with me but please double-check there was no fallout. >> > >> > I boostrapped and ran against x86_64-pc-linux again, just to be sure. >> > No regressions. >> >> I meant fallout only fixed with followup patches. ISTR some in that area >> but I might confuse it with another patch. Marek might remember. > > I'm not convinced it is desirable to backport such changes, it affects ABI, > people are used to deal with minor ABI changes in between major GCC > releases, but we'd need a strong reason to change it between minor releases. Hmm, I tracked this down from a (internal) bug reported on arm-none-eabi, where the inconsistent enum size (used in sizeof to malloc) was eventually causing heap corruption. When debugging the issue, I noticed you'd already backported https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69669, so I thought this should be good. Regards Senthil > >> >>> 2016-10-17 Senthil Kumar Selvaraj >> >>> >> >>> Backport from mainline >> >>> 2015-04-25 Marek Polacek >> >>> PR c/52085 >> >>> * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for >> >>> "mode" >> >>> attribute. >> >>> >> >>> gcc/testsuite/ChangeLog >> >>> 2016-10-17 Senthil Kumar Selvaraj >> >>> >> >>> Backport from mainline >> >>> 2015-04-25 Marek Polacek >> >>> PR c/52085 >> >>> * gcc.dg/enum-incomplete-2.c: New test. >> >>> * gcc.dg/enum-mode-1.c: New test. > > Jakub
Re: [Patch, reload, tentative, PR 71627] Tweak conditions in find_valid_class_1
Ping!
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Bernd Schmidt writes:
>
>> On 09/16/2016 09:02 PM, Senthil Kumar Selvaraj wrote:
>>> Does this make sense? I ran a reg test for the avr target with a
>>> slightly older version of this patch, it did not show any regressions.
>>> If this is the right fix, I'll make sure to run reg tests on x86_64
>>> after backporting to a gcc version where that target used reload.
>>
>> It's hard to say, and could have different effects on different targets.
>> One thing though, at the very least the reg_class_size test would have
>> to be adapted - the idea is to find the largest class, and there's a
>> risk here of ending up with a large class that only has one valid register.
>
> Agreed - I've updated the patch to compute rclass sizes based on regno
> availability i.e., only if in_hard_reg_set_p and HARD_REGNO_MODE_OK, and
> then use the computed sizes when calculating best_size.
>
>>
>> You'll also want to verify this against
>>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54814
>
> Yes, this patch doesn't break the fix for PR54814. The change to
> in_hard_reg_set_p was what fixed that, and that remains unmodified.
>
> Reg tested this on top of trunk@190252 with the in_hard_reg_set_p
> backport. x86_64-pc-linux bootstrapped and regtested ok. avr showed
> no regressions either.
>
> Ok for trunk?
>
> Regards
> Senthil
>
> gcc/ChangeLog:
>
> 2016-10-13 Senthil Kumar Selvaraj
>
> * reload.c (find_valid_class_1): Allow regclass if atleast one
> regno in class is ok. Compute and use rclass size based on
> actually available regnos for mode in rclass.
>
> gcc/testsuite/ChangeLog:
>
> 2016-10-13 Senthil Kumar Selvaraj
>
> * gcc.target/avr/pr71627.c: New.
>
>
> Index: gcc/reload.c
> ===
> --- gcc/reload.c (revision 240989)
> +++ gcc/reload.c (working copy)
> @@ -711,31 +711,36 @@
>enum reg_class best_class = NO_REGS;
>unsigned int best_size = 0;
>int cost;
> + unsigned int computed_rclass_sizes[N_REG_CLASSES] = { 0 };
>
>for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
> {
> - int bad = 0;
> - for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
> - {
> - if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
> - && !HARD_REGNO_MODE_OK (regno, mode))
> - bad = 1;
> - }
> -
> - if (bad)
> - continue;
> + int atleast_one_regno_ok = 0;
>
> + for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
> +{
> + if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno))
> +{
> + atleast_one_regno_ok = 1;
> + if (HARD_REGNO_MODE_OK (regno, mode))
> +computed_rclass_sizes[rclass]++;
> +}
> +}
> +
> + if (!atleast_one_regno_ok)
> +continue;
> +
>cost = register_move_cost (outer, (enum reg_class) rclass, dest_class);
>
> - if ((reg_class_size[rclass] > best_size
> -&& (best_cost < 0 || best_cost >= cost))
> - || best_cost > cost)
> - {
> - best_class = (enum reg_class) rclass;
> - best_size = reg_class_size[rclass];
> - best_cost = register_move_cost (outer, (enum reg_class) rclass,
> - dest_class);
> - }
> + if ((computed_rclass_sizes[rclass] > best_size
> + && (best_cost < 0 || best_cost >= cost))
> +|| best_cost > cost)
> + {
> +best_class = (enum reg_class) rclass;
> +best_size = computed_rclass_sizes[rclass];
> +best_cost = register_move_cost (outer, (enum reg_class) rclass,
> +dest_class);
> + }
> }
>
>gcc_assert (best_size != 0);
>
> Index: gcc/testsuite/gcc.target/avr/pr71627.c
> ===
> --- gcc/testsuite/gcc.target/avr/pr71627.c (nonexistent)
> +++ gcc/testsuite/gcc.target/avr/pr71627.c (working copy)
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1" } */
> +
> +
> +extern volatile __memx const long a, b, c, d, e, f;
> +extern volatile long result;
> +
> +extern void vfunc (const char*, ...);
> +
> +void foo (void)
> +{
> + result = a + b + c + d + e + f;
> + vfunc ("text", a, b, c, d, e, f, result);
> +}
Re: [Patch] Backport fix for PR 52085 to gcc-5-branch?
Jakub Jelinek writes: > On Tue, Oct 18, 2016 at 02:46:29PM +0530, Senthil Kumar Selvaraj wrote: >> > I'm not convinced it is desirable to backport such changes, it affects ABI, >> > people are used to deal with minor ABI changes in between major GCC >> > releases, but we'd need a strong reason to change it between minor >> > releases. >> >> Hmm, I tracked this down from a (internal) bug reported on arm-none-eabi, >> where the >> inconsistent enum size (used in sizeof to malloc) was eventually causing >> heap corruption. >> >> When debugging the issue, I noticed you'd already backported >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69669, so I thought this >> should be good. > > That one has been a regression, older GCCs handled it the same as does the 5 > branch now. Is that the case here? No, I can reproduce the bug on 4.9 as well. So not ok to backport then, I guess? Regards Senthil
[Patch,testsuite] Fix sso.exp not calling torture-finish for avr
Hi,
When analyzing reg test failures for the avr target, I noticed that the
torture options were different when running dg-torture.exp compared to
x86_64-pc-linux-gnu, resulting in additional failures. I also found
that a bunch of "torture-without-loops not empty as expected" errors
show up for a few .exp files.
I found that these did not occur when the exp files were run in
isolation. On further debugging, I found that sso.exp calls dg-init and
torture-init, and returns if !effective_target_int32. It does
not call the corresponding finish functions for targets like the avr
for which the effective target condition is true, and this leaves
torture-options set, which causes the errors and differing options.
The below patch makes the return occur earlier - before calling the
init functions.
Committed to trunk.
Regards
Senthil
2016-10-18 Senthil Kumar Selvaraj
* gcc.dg/sso/sso.exp: Return early if not
effective_target_int32.
Index: gcc.dg/sso/sso.exp
===
--- gcc.dg/sso/sso.exp (revision 241299)
+++ gcc.dg/sso/sso.exp (working copy)
@@ -18,6 +18,10 @@
load_lib gcc-dg.exp
load_lib torture-options.exp
+if { ![check_effective_target_int32] } {
+return
+}
+
# Initialize `dg'.
torture-init
dg-init
@@ -32,10 +36,6 @@
set-torture-options $SSO_TORTURE_OPTIONS
-if { ![check_effective_target_int32] } {
-return
-}
-
# Main loop.
gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.c]] "" ""
Re: [Patch, reload, tentative, PR 71627] Tweak conditions in find_valid_class_1
Bernd Schmidt writes:
> On 10/13/2016 08:57 AM, Senthil Kumar Selvaraj wrote:
>>
>> 2016-10-13 Senthil Kumar Selvaraj
>>
>> * reload.c (find_valid_class_1): Allow regclass if atleast one
>> regno in class is ok. Compute and use rclass size based on
>> actually available regnos for mode in rclass.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-13 Senthil Kumar Selvaraj
>>
>> * gcc.target/avr/pr71627.c: New.
>>
>>
>> Index: gcc/reload.c
>> ===
>> --- gcc/reload.c (revision 240989)
>> +++ gcc/reload.c (working copy)
>> @@ -711,31 +711,36 @@
>>enum reg_class best_class = NO_REGS;
>>unsigned int best_size = 0;
>>int cost;
>> + unsigned int computed_rclass_sizes[N_REG_CLASSES] = { 0 };
>
> As far as I can tell you're only accessing this as
> computed_rclass_size[rclass], i.e. with the current class in the loop.
> So I don't think you need the array at all, just a computed_size
> variable in the loop?
Yes - I mechanically replaced the original array with the computed one.
A variable would suffice.
>
>> + for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
>> +{
>> + if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno))
>> +{
>> + atleast_one_regno_ok = 1;
>> + if (HARD_REGNO_MODE_OK (regno, mode))
>> +computed_rclass_sizes[rclass]++;
>> +}
>> +}
>
> Don't you want to also ensure HARD_REGNO_MODE_OK before claiming that
> atleast_one_regno_ok? Maybe I'm forgetting the motivation but this seems
> odd. If so, the variable becomes unnecessary, just check the computed size.
True again - the original intention was to prevent the best_xxx
variables from getting set if no regno was in_hard_reg_set. Now the
computed class size would be zero, so the variable is unnecessary.
Will do both the changes and re-run the reg tests. Ok for trunk if the
tests pass for x86_64-pc-linux and avr?
Regards
Senthil
Re: [Patch, reload, tentative, PR 71627] Tweak conditions in find_valid_class_1
Bernd Schmidt writes:
> On 10/18/2016 02:15 PM, Senthil Kumar Selvaraj wrote:
>> Will do both the changes and re-run the reg tests. Ok for trunk if the
>> tests pass for x86_64-pc-linux and avr?
>>
> Probably but let's see the patch first.
How does this look?
Bootstrapped and reg tested x86_64-pc-linux on top of trunk@190252 with
the in_hard_reg_set_p patch backport - there were no failures. Also ran
regtests for avr on trunk, no failures there as well.
Ok to commit to trunk?
Regards
Senthil
gcc/ChangeLog:
2016-10-21 Senthil Kumar Selvaraj
* reload.c (find_valid_class_1): Allow regclass if atleast one
regno in class is ok. Compute and use rclass size based on
actually available regnos for mode in rclass.
gcc/testsuite/ChangeLog:
2016-10-21 Senthil Kumar Selvaraj
* gcc.target/avr/pr71627.c: New test.
diff --git gcc/reload.c gcc/reload.c
index 9a859e5..880099e 100644
--- gcc/reload.c
+++ gcc/reload.c
@@ -715,25 +715,23 @@ find_valid_class_1 (machine_mode outer ATTRIBUTE_UNUSED,
for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
{
- int bad = 0;
- for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
- {
- if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
- && !HARD_REGNO_MODE_OK (regno, mode))
- bad = 1;
- }
-
- if (bad)
- continue;
+ unsigned int computed_rclass_size = 0;
+
+ for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+{
+ if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
+ && (HARD_REGNO_MODE_OK (regno, mode)))
+computed_rclass_size++;
+}
cost = register_move_cost (outer, (enum reg_class) rclass, dest_class);
- if ((reg_class_size[rclass] > best_size
+ if ((computed_rclass_size > best_size
&& (best_cost < 0 || best_cost >= cost))
|| best_cost > cost)
{
best_class = (enum reg_class) rclass;
- best_size = reg_class_size[rclass];
+ best_size = computed_rclass_size;
best_cost = register_move_cost (outer, (enum reg_class) rclass,
dest_class);
}
diff --git gcc/testsuite/gcc.target/avr/pr71627.c
gcc/testsuite/gcc.target/avr/pr71627.c
new file mode 100644
index 000..eaef3d2
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr71627.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+
+extern volatile __memx const long a, b, c, d, e, f;
+extern volatile long result;
+
+extern void vfunc (const char*, ...);
+
+void foo (void)
+{
+ result = a + b + c + d + e + f;
+ vfunc ("text", a, b, c, d, e, f, result);
+}
[Patch, testsuite] Skip gcc.dg/lto/pr60449_0.c for avr
Hi,
gcc.dg/lto/pr60449_0.c fails to link for the avr target because it
doesn't have an implementation for gettimeofday. This patch therefore
skips the test for avr.
Committed to trunk as obvious.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-10-31 Senthil Kumar Selvaraj
* gcc.dg/lto/pr60449_0.c: Skip for avr.
Index: gcc/testsuite/gcc.dg/lto/pr60449_0.c
===
--- gcc/testsuite/gcc.dg/lto/pr60449_0.c(revision 241697)
+++ gcc/testsuite/gcc.dg/lto/pr60449_0.c(working copy)
@@ -1,4 +1,5 @@
/* { dg-lto-do link } */
+/* { dg-skip-if "Needs gettimeofday" { "avr-*-*" } } */
extern int printf (const char *__restrict __format, ...);
typedef long int __time_t;
[Patch, testsuite] Fix bogus PR 78170 failure for avr
Hi,
The below patch requires int32plus for
gcc.c-torture/execute/pr78170.c, as it has int bitfields that are more
than 16 bits wide.
Committed to trunk as obvious.
Regards
Senthil
gcc/testsuite/ChangeLog
2016-11-03 Senthil Kumar Selvaraj
* gcc.c-torture/execute/pr78170.c: Require int32plus.
Index: gcc.c-torture/execute/pr78170.c
===
--- gcc.c-torture/execute/pr78170.c (revision 241808)
+++ gcc.c-torture/execute/pr78170.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-require-effective-target int32plus } */
+
/* PR tree-optimization/78170.
Check that sign-extended store to a bitfield
doesn't overwrite other fields. */
[Patch, testsuite] Add new effective-target_store_merge
Hi,
The below patch adds a new effective target keyword (store_merge) for
use in the store_merging_xxx.c tests.
The tests currently require non_strict_align, but they still fail for the avr.
Eyeballing the dump, I found that the pass doesn't attempt merging as it is
unprofitable for a target like the avr which has only single byte
stores.
I figured store merging is unlikely to be profitable for targets with
smallish word sizes, and added a check_effective_target_store_merge
that combines non_strict_align and int32plus.
Is this ok for trunk?
Regards
Senthil
gcc/testsuite/ChangeLog
2016-11-03 Senthil Kumar Selvaraj
* gcc.dg/store_merging_1.c: Require store_merge.
* gcc.dg/store_merging_2.c: Likewise.
* gcc.dg/store_merging_4.c: Likewise.
* gcc.dg/store_merging_5.c: Likewise.
* gcc.dg/store_merging_6.c: Likewise.
* gcc.dg/store_merging_7.c: Likewise.
* gcc.dg/store_merging_8.c: Likewise.
* lib/target-supports.exp (check_effective_target_store_merge): New.
Index: gcc/testsuite/gcc.dg/store_merging_1.c
===
--- gcc/testsuite/gcc.dg/store_merging_1.c (revision 241808)
+++ gcc/testsuite/gcc.dg/store_merging_1.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-require-effective-target non_strict_align } */
+/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging" } */
struct bar {
Index: gcc/testsuite/gcc.dg/store_merging_2.c
===
--- gcc/testsuite/gcc.dg/store_merging_2.c (revision 241808)
+++ gcc/testsuite/gcc.dg/store_merging_2.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-require-effective-target non_strict_align } */
+/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging" } */
struct bar
Index: gcc/testsuite/gcc.dg/store_merging_4.c
===
--- gcc/testsuite/gcc.dg/store_merging_4.c (revision 241808)
+++ gcc/testsuite/gcc.dg/store_merging_4.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-require-effective-target non_strict_align } */
+/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging" } */
/* Check that we can merge interleaving stores that are guaranteed
Index: gcc/testsuite/gcc.dg/store_merging_5.c
===
--- gcc/testsuite/gcc.dg/store_merging_5.c (revision 241808)
+++ gcc/testsuite/gcc.dg/store_merging_5.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-require-effective-target non_strict_align } */
+/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging" } */
/* Make sure that non-aliasing non-constant interspersed stores do not
Index: gcc/testsuite/gcc.dg/store_merging_6.c
===
--- gcc/testsuite/gcc.dg/store_merging_6.c (revision 241808)
+++ gcc/testsuite/gcc.dg/store_merging_6.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-require-effective-target non_strict_align } */
+/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging" } */
/* Check that we can widen accesses to bitfields. */
Index: gcc/testsuite/gcc.dg/store_merging_7.c
===
--- gcc/testsuite/gcc.dg/store_merging_7.c (revision 241808)
+++ gcc/testsuite/gcc.dg/store_merging_7.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-require-effective-target non_strict_align } */
+/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging" } */
/* Check that we can merge consecutive array members through the pointer.
Index: gcc/testsuite/gcc.dg/store_merging_8.c
===
--- gcc/testsuite/gcc.dg/store_merging_8.c (revision 241808)
+++ gcc/testsuite/gcc.dg/store_merging_8.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-require-effective-target non_strict_align } */
+/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging" } */
struct baz {
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp (revision 241808)
+++ gcc/testsuite/lib/target-supports.exp (working copy)
@@ -8107,3 +8107,16 @@
return [check_effective_target_divmod]
}
+
+# Return 1 if store merging optimization is applicable for target.
+# Store merging is not profitable for ta
[Patch, avr] Fix PR 50739 - nameless error with -fmerge-all-constants
Hi,
This patch fixes a problem with fmerge-all-constants and the progmem
attribute - on trunk, the below testcase errors out with a section
conflict error.
When avr_asm_select_section renames .rodata.xyz section to
.progmem.xyz and calls get_section, it passes in the same flags in
sect. If the flags include SECTION_DECLARED, get_section barfs with a
section conflict error - the section flag comparison logic strips off
SECTION_DECLARED from existing section flags before comparing it with
the new incoming flags.
With -fmerge-all-constants, default_elf_select_section always returns
.rodata.strx.x. varasm switches to that section when writing out the
non progmem string literal, and that sets SECTION_DECLARED. The first
call to get_section with the section name transformed to
.progmem.data.strx.x then includes SECTION_DECLARED, but because this
is a new section, the section flag conflict logic doesn't kick in. The
second call to get_section, again including SECTION_DECLARED, triggers
the section flag conflict logic and causes the error.
Stripping off SECTION_DECLARED before calling get_section fixes the
problem - the flag is supposed to be set by switch_section anyway.
Reg testing showed no new regressions. Ok for trunk and backport to 6.x?
Regards
Senthil
gcc/testsuite/ChangeLog:
2016-07-05 Senthil Kumar Selvaraj
PR target/50739
* gcc.target/avr/pr50739.c: New test.
gcc/ChangeLog:
2016-07-05 Senthil Kumar Selvaraj
PR target/50739
* config/avr/avr.c (avr_asm_select_section):
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 18ed766..9b7b392 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -9641,7 +9641,7 @@ avr_asm_select_section (tree decl, int reloc, unsigned
HOST_WIDE_INT align)
{
const char *sname = ACONCAT ((new_prefix,
name + strlen (old_prefix), NULL));
- return get_section (sname, sect->common.flags, sect->named.decl);
+ return get_section (sname, sect->common.flags &
~SECTION_DECLARED, sect->named.decl);
}
}
diff --git gcc/testsuite/gcc.target/avr/pr50739.c
gcc/testsuite/gcc.target/avr/pr50739.c
new file mode 100644
index 000..a6850b7
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr50739.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-fmerge-all-constants" } */
+
+char *ca = "123";
+
+const char a[] __attribute__((__progmem__))= "a";
+const char b[] __attribute__((__progmem__))= "b";
--
2.7.4
Re: [Patch, avr] Fix PR 50739 - nameless error with -fmerge-all-constants
Senthil Kumar Selvaraj writes:
> Hi,
>
> This patch fixes a problem with fmerge-all-constants and the progmem
> attribute - on trunk, the below testcase errors out with a section
> conflict error.
>
> When avr_asm_select_section renames .rodata.xyz section to
> .progmem.xyz and calls get_section, it passes in the same flags in
> sect. If the flags include SECTION_DECLARED, get_section barfs with a
> section conflict error - the section flag comparison logic strips off
> SECTION_DECLARED from existing section flags before comparing it with
> the new incoming flags.
>
> With -fmerge-all-constants, default_elf_select_section always returns
> .rodata.strx.x. varasm switches to that section when writing out the
> non progmem string literal, and that sets SECTION_DECLARED. The first
> call to get_section with the section name transformed to
> .progmem.data.strx.x then includes SECTION_DECLARED, but because this
> is a new section, the section flag conflict logic doesn't kick in. The
> second call to get_section, again including SECTION_DECLARED, triggers
> the section flag conflict logic and causes the error.
>
> Stripping off SECTION_DECLARED before calling get_section fixes the
> problem - the flag is supposed to be set by switch_section anyway.
>
> Reg testing showed no new regressions. Ok for trunk and backport to 6.x?
>
> Regards
> Senthil
>
Added missing description in Changelog entry.
gcc/testsuite/ChangeLog:
2016-07-05 Senthil Kumar Selvaraj
PR target/50739
* gcc.target/avr/pr50739.c: New test.
gcc/ChangeLog:
2016-07-05 Senthil Kumar Selvaraj
PR target/50739
* config/avr/avr.c (avr_asm_select_section): Strip off
SECTION_DECLARED from flags when calling get_section.
Regards
Senthil
>
> gcc/testsuite/ChangeLog:
>
> 2016-07-05 Senthil Kumar Selvaraj
>
> PR target/50739
> * gcc.target/avr/pr50739.c: New test.
>
>
> gcc/ChangeLog:
>
> 2016-07-05 Senthil Kumar Selvaraj
>
> PR target/50739
> * config/avr/avr.c (avr_asm_select_section):
>
>
> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
> index 18ed766..9b7b392 100644
> --- gcc/config/avr/avr.c
> +++ gcc/config/avr/avr.c
> @@ -9641,7 +9641,7 @@ avr_asm_select_section (tree decl, int reloc, unsigned
> HOST_WIDE_INT align)
> {
>const char *sname = ACONCAT ((new_prefix,
> name + strlen (old_prefix),
> NULL));
> - return get_section (sname, sect->common.flags,
> sect->named.decl);
> + return get_section (sname, sect->common.flags &
> ~SECTION_DECLARED, sect->named.decl);
> }
> }
>
> diff --git gcc/testsuite/gcc.target/avr/pr50739.c
> gcc/testsuite/gcc.target/avr/pr50739.c
> new file mode 100644
> index 000..a6850b7
> --- /dev/null
> +++ gcc/testsuite/gcc.target/avr/pr50739.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fmerge-all-constants" } */
> +
> +char *ca = "123";
> +
> +const char a[] __attribute__((__progmem__))= "a";
> +const char b[] __attribute__((__progmem__))= "b";
