Re: [PATCH] avr: Set param_min_pagesize to 0 [PR105523]
On Mon, 2023-05-22 at 14:05 +0200, Richard Biener wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Fri, May 19, 2023 at 7:58 AM wrote:
> > On 26/04/23, 5:51 PM, "Richard Biener" > <mailto:richard.guent...@gmail.com>> wrote:
> > > On Wed, Apr 26, 2023 at 12:56 PM > > <mailto:senthilkumar.selva...@microchip.com>> wrote:
> > > > On Wed, Apr 26, 2023 at 3:15 PM Richard Biener via Gcc-patches
> > > > mailto:gcc-patches@gcc.gnu.org>> wrote:
> > > > > On Wed, Apr 26, 2023 at 11:42 AM Richard Biener
> > > > > mailto:richard.guent...@gmail.com>>
> > > > > wrote:
> > > > > > On Wed, Apr 26, 2023 at 11:01 AM SenthilKumar.Selvaraj--- via
> > > > > > Gcc-patches > > > > > <mailto:gcc-patches@gcc.gnu.org>> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > This patch fixes PR 105523 by setting param_min_pagesize to 0 for
> > > > > > > the
> > > > > > > avr target. For this target, zero and offsets from zero are
> > > > > > > perfectly
> > > > > > > valid addresses, and the default value of param_min_pagesize ends
> > > > > > > up
> > > > > > > triggering warnings on valid memory accesses.
> > > > > >
> > > > > > I think the proper configuration is to have
> > > > > > DEFAULT_ADDR_SPACE_ZERO_ADDRESS_VALID
> > > > >
> > > > > Err, TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
> > > >
> > > > That worked. Ok for trunk and backporting to 13 and 12 branches
> > > > (pending regression testing)?
> > >
> > > OK, but please let Denis time to comment.
> >
> > Didn't hear from Denis. When running regression tests with this patch,
> > I found that some tests with -fdelete-null-pointer-checks were
> > failing. Commit 19416210b37db0584cd0b3f3b3961324b8973d25 made
> > -fdelete-null-pointer-checks false by default, while still allowing it
> > to be overridden from the command line (it was previously
> > unconditionally false).
> >
> > To keep the same behavior, I modified the hook to report zero
> > addresses as valid only if -fdelete-null-pointer-checks is not set.
> > With this change, all regression tests pass.
> >
> > Ok for trunk and backporting to 13 and 12 branches?
>
> I think that's bit backwards - this hook conveys more precise information
> (it's address-space specific) and it is also more specific. Instead I'd
> suggest to set the flag to zero in the target like nios2 or msp430 do.
> In fact we should probably initialize it using this hook (and using the
> default address space).
Does the below patch work? The hook impl reports that zero address is
valid, and flag_delete_null_pointer_checks is set to zero if the
hook says zero is a valid address.
As flag_delete_null_pointer_checks is now always disabled for avr, I
removed the resetting code in avr-common.cc that disables it for
OPT_LEVELS_ALL by default, and added avr as a target that always keeps
null pointer checks in testsuite/lib/target-supports.exp.
I also removed ATTRIBUTE_UNUSED and the parameter name in the target
hook to address https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619014.html.
PR 105523
gcc/ChangeLog:
* common/config/avr/avr-common.cc: Remove setting
of OPT_fdelete_null_pointer_checks.
* config/avr/avr.cc (avr_option_override): Clear
flag_delete_null_pointer_checks if zero_address_valid.
(avr_addr_space_zero_address_valid): New function.
(TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): Provide target
hook.
gcc/testsuite/ChangeLog:
* lib/target-supports.exp
(check_effective_target_keeps_null_pointer_checks): Add
avr.
* gcc.target/avr/pr105523.c: New test.
diff --git gcc/common/config/avr/avr-common.cc
gcc/common/config/avr/avr-common.cc
index 2ad0244..2f874c5 100644
--- gcc/common/config/avr/avr-common.cc
+++ gcc/common/config/avr/avr-common.cc
@@ -29,12 +29,6 @@
/* Implement TARGET_OPTION_OPTIMIZATION_TABLE. */
static const struct default_options avr_option_optimization_table[] =
{
-// With -fdelete-null-pointer-checks option, the compiler assumes
-// that dereferencing of a null pointer would halt the program.
-// For AVR this assumption is not true and a program can safely
-// dereference null pointers. Changes made by this option may not
-// work properly for AVR. So disable this option.
-{ OPT_LEVELS_ALL, OPT_fdele
[PATCH, PR110086] avr: Fix ICE on optimize attribute
Hi,
This patch fixes an ICE when an optimize attribute changes the prevailing
optimization level.
I found https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105069 describing the
same ICE for the sh target, where the fix was to enable save/restore of
target specific options modified via TARGET_OPTIMIZATION_TABLE hook.
For the AVR target, mgas-isr-prologues and -mmain-is-OS_task are those
target specific options. As they enable generation of more optimal code,
this patch adds the Optimization option property to those option records,
and that fixes the ICE.
Regression run shows no regressions, and >100 new PASSes.
Ok to commit to master?
Regards
Senthil
PR 110086
gcc/ChangeLog:
* config/avr/avr.opt (mgas-isr-prologues, mmain-is-OS_task):
Add Optimization option property.
gcc/testsuite/ChangeLog:
* gcc.target/avr/pr110086.c: New test.
diff --git gcc/config/avr/avr.opt gcc/config/avr/avr.opt
index f62d746..5a0b465 100644
--- gcc/config/avr/avr.opt
+++ gcc/config/avr/avr.opt
@@ -27,7 +27,7 @@ Target RejectNegative Joined Var(avr_mmcu)
MissingArgError(missing device or arc
-mmcu=MCU Select the target MCU.
mgas-isr-prologues
-Target Var(avr_gasisr_prologues) UInteger Init(0)
+Target Var(avr_gasisr_prologues) UInteger Init(0) Optimization
Allow usage of __gcc_isr pseudo instructions in ISR prologues and epilogues.
mn-flash=
@@ -65,7 +65,7 @@ Target Joined RejectNegative UInteger Var(avr_branch_cost)
Init(0)
Set the branch costs for conditional branch instructions. Reasonable values
are small, non-negative integers. The default
branch cost is 0.
mmain-is-OS_task
-Target Mask(MAIN_IS_OS_TASK)
+Target Mask(MAIN_IS_OS_TASK) Optimization
Treat main as if it had attribute OS_task.
morder1
diff --git gcc/testsuite/gcc.target/avr/pr110086.c
gcc/testsuite/gcc.target/avr/pr110086.c
new file mode 100644
index 000..6b97620
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr110086.c
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+
+void __attribute__((optimize("O0"))) foo(void) {
+}
[IRA] Skip empty register classes in setup_reg_class_relations
Hi,
I've been spending some (spare) time trying to get LRA working
for the avr target. After making a couple of changes to get
libgcc going, I'm now hitting an assert at
lra-constraints.cc:4423 for a subarch (avrtiny) that has a
couple of regclasses with no available registers.
The assert fires because in_class_p (correctly) returns
false for get_reg_class (regno) = ALL_REGS, and new_class =
NO_LD_REGS. For avrtiny, NO_LD_REGS is an empty regset, and
therefore hard_reg_set_subset_p (NO_LD_REGS, lra_no_alloc_regs)
is always true, making in_class_p return false.
in_class_p picks NO_LD_REGS as new_class because common_class =
ira_reg_class_subset[ALL_REGS][NO_REGS] evaluates as
NO_LD_REGS. This appears wrong to me - it should be NO_REGS
instead (lra-constraints.cc:4421 checks for NO_REGS).
ira.cc:setup_reg_class_relations sets up
ira_reg_class_subset (among other things), and the problem
appears to be a missing continue statement if
reg_class_contents[cl3] (in the innermost loop) is empty.
In this case, for cl1 = ALL_REGS and cl2 = NO_REGS, cl3 =
NO_LD_REGS, temp_hard_regset and temp_set2 are both empty, and
hard_reg_subset_p (, ) is always true, so
ira_reg_class_subset[ALL_REGS][NO_REGS] ends up being set to
cl3 = NO_LD_REGS. Adding a continue if hard_reg_set_empty_p (temp_hard_regset)
fixes the problem for me.
Does the below patch look ok? Bootstrapping and regression
testing passed on x86_64.
Regards
Senthil
gcc/ChangeLog:
* ira.cc (setup_reg_class_relations): Skip if
cl3 is an empty register class.
--- gcc/ira.cc
+++ gcc/ira.cc
@@ -1259,6 +1259,9 @@ setup_reg_class_relations (void)
for (cl3 = 0; cl3 < N_REG_CLASSES; cl3++)
{
temp_hard_regset = reg_class_contents[cl3] & ~no_unit_alloc_regs;
+ if (hard_reg_set_empty_p (temp_hard_regset))
+ continue;
+
if (hard_reg_set_subset_p (temp_hard_regset, intersection_set))
{
/* CL3 allocatable hard register set is inside of
Re: [pushed][LRA] Check input insn pattern hard regs against early clobber hard regs for live info
On Fri, 2023-08-04 at 09:16 -0400, Vladimir Makarov wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > The following patch fixes a problem found by LRA port for avr target. > The problem description is in the commit message. > > The patch was successfully bootstrapped and tested on x86-64 and aarch64. I can confirm it fixes the problem on avr - thank you. Regards Senthil
Re: [pushed][LRA]: Spill pseudos assigned to fp when fp->sp elimination became impossible
On Wed, 2023-08-16 at 12:13 -0400, Vladimir Makarov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> The attached patch fixes recently found wrong insn removal in LRA port
> for AVR.
>
> The patch was successfully tested and bootstrapped on x86-64 and aarch64.
>
>
Hi Vladimir,
Thanks for working on this. After applying the patch, I'm seeing that the
pseudo in the frame pointer that got spilled is taking up the same stack
slot that was already assigned to a spilled pseudo, and that is causing
execution
failure (it is also causing a crash when building libgcc for avr)
(insn 108 15 3 3 (set (mem/c:SI (plus:HI (reg/f:HI 28 r28)
(const_int 1 [0x1])) [3 %sfp+1 S4 A8])
(reg:SI 4 r4 [orig:46 f.3_4 ] [46]))
"/home/i41766/code/personal/gcc/gcc/testsuite/gcc.c-torture/execute/20050224-
1.c":29:16 119 {*movsi_split}
(nil))
(insn 3 108 4 3 (set (mem/c:HI (plus:HI (reg/f:HI 28 r28)
(const_int 1 [0x1])) [3 %sfp+1 S2 A8])
(const_int 0 [0]))
"/home/i41766/code/personal/gcc/gcc/testsuite/gcc.c-torture/execute/20050224-1.c":19:21
101
{*movhi_split}
(expr_list:REG_EQUAL (const_int 0 [0])
(nil)))
Both pseudo 46, and the pseudo spilled off FP (51) get assigned stack slot 0.
This translates to this obviously wrong assembly code.
lds r4,f
lds r5,f+1
lds r6,f+2
lds r7,f+3
std Y+1,r4
std Y+2,r5
std Y+3,r6
std Y+4,r7
std Y+2,__zero_reg__
std Y+1,__zero_reg__
I tried a hacky workaround (see patch below) to create a new stack slot and
assign the spilled pseudo to it, and that works.
Not sure if that's the right way to do it though.
Regards
Senthil
diff --git gcc/lra-spills.cc gcc/lra-spills.cc
index 7e1d35b5e4e..e985ab56a60 100644
--- gcc/lra-spills.cc
+++ gcc/lra-spills.cc
@@ -359,11 +359,12 @@ add_pseudo_to_slot (int regno, int slot_num)
length N. Sort pseudos in PSEUDO_REGNOS for subsequent assigning
memory stack slots. */
static void
-assign_stack_slot_num_and_sort_pseudos (int *pseudo_regnos, int n)
+assign_stack_slot_num_and_sort_pseudos (int *pseudo_regnos, int n, bool reset)
{
int i, j, regno;
- slots_num = 0;
+ if (reset)
+slots_num = 0;
/* Assign stack slot numbers to spilled pseudos, use smaller numbers
for most frequently used pseudos. */
for (i = 0; i < n; i++)
@@ -628,14 +629,15 @@ lra_spill (void)
/* Sort regnos according their usage frequencies. */
qsort (pseudo_regnos, n, sizeof (int), regno_freq_compare);
n = assign_spill_hard_regs (pseudo_regnos, n);
- assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n);
+ assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n, true);
for (i = 0; i < n; i++)
if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
assign_mem_slot (pseudo_regnos[i]);
- if ((n2 = lra_update_fp2sp_elimination (pseudo_regnos)) > 0)
+ if ((n2 = lra_update_fp2sp_elimination (&pseudo_regnos[n])) > 0)
{
+ assign_stack_slot_num_and_sort_pseudos (&pseudo_regnos[n], n2, false);
/* Assign stack slots to spilled pseudos assigned to fp. */
- for (i = 0; i < n2; i++)
+ for (i = n; i < n + n2; i++)
if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
assign_mem_slot (pseudo_regnos[i]);
}
Regards
Senthil
[Ping] Re: [PATCH] avr: Set param_min_pagesize to 0 [PR105523]
On Fri, 2023-06-02 at 12:32 +0530, Senthil Kumar Selvaraj wrote: > On Mon, 2023-05-22 at 14:05 +0200, Richard Biener wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > On Fri, May 19, 2023 at 7:58 AM wrote: > > > On 26/04/23, 5:51 PM, "Richard Biener" > > <mailto:richard.guent...@gmail.com>> wrote: > > > > On Wed, Apr 26, 2023 at 12:56 PM > > > <mailto:senthilkumar.selva...@microchip.com>> wrote: > > > > > On Wed, Apr 26, 2023 at 3:15 PM Richard Biener via Gcc-patches > > > > > mailto:gcc-patches@gcc.gnu.org>> wrote: > > > > > > On Wed, Apr 26, 2023 at 11:42 AM Richard Biener > > > > > > mailto:richard.guent...@gmail.com>> > > > > > > wrote: > > > > > > > On Wed, Apr 26, 2023 at 11:01 AM SenthilKumar.Selvaraj--- via > > > > > > > Gcc-patches > > > > > > <mailto:gcc-patches@gcc.gnu.org>> wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > This patch fixes PR 105523 by setting param_min_pagesize to 0 > > > > > > > > for the > > > > > > > > avr target. For this target, zero and offsets from zero are > > > > > > > > perfectly > > > > > > > > valid addresses, and the default value of param_min_pagesize > > > > > > > > ends up > > > > > > > > triggering warnings on valid memory accesses. > > > > > > > > > > > > > > I think the proper configuration is to have > > > > > > > DEFAULT_ADDR_SPACE_ZERO_ADDRESS_VALID > > > > > > > > > > > > Err, TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID > > > > > > > > > > That worked. Ok for trunk and backporting to 13 and 12 branches > > > > > (pending regression testing)? > > > > > > > > OK, but please let Denis time to comment. > > > > > > Didn't hear from Denis. When running regression tests with this patch, > > > I found that some tests with -fdelete-null-pointer-checks were > > > failing. Commit 19416210b37db0584cd0b3f3b3961324b8973d25 made > > > -fdelete-null-pointer-checks false by default, while still allowing it > > > to be overridden from the command line (it was previously > > > unconditionally false). > > > > > > To keep the same behavior, I modified the hook to report zero > > > addresses as valid only if -fdelete-null-pointer-checks is not set. > > > With this change, all regression tests pass. > > > > > > Ok for trunk and backporting to 13 and 12 branches? > > > > I think that's bit backwards - this hook conveys more precise information > > (it's address-space specific) and it is also more specific. Instead I'd > > suggest to set the flag to zero in the target like nios2 or msp430 do. > > In fact we should probably initialize it using this hook (and using the > > default address space). > > Does the below patch work? The hook impl reports that zero address is > valid, and flag_delete_null_pointer_checks is set to zero if the > hook says zero is a valid address. > > As flag_delete_null_pointer_checks is now always disabled for avr, I > removed the resetting code in avr-common.cc that disables it for > OPT_LEVELS_ALL by default, and added avr as a target that always keeps > null pointer checks in testsuite/lib/target-supports.exp. > > I also removed ATTRIBUTE_UNUSED and the parameter name in the target > hook to address > https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619014.html. > > PR 105523 > > gcc/ChangeLog: > > * common/config/avr/avr-common.cc: Remove setting > of OPT_fdelete_null_pointer_checks. > * config/avr/avr.cc (avr_option_override): Clear > flag_delete_null_pointer_checks if zero_address_valid. > (avr_addr_space_zero_address_valid): New function. > (TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): Provide target > hook. > > gcc/testsuite/ChangeLog: > > * lib/target-supports.exp > (check_effective_target_keeps_null_pointer_checks): Add > avr. > * gcc.target/avr/pr105523.c: New test. > > diff --git gcc/common/config/avr/avr-common.cc > gcc/common/config/avr/avr-common.cc > index 2ad0244..2f874c5 100644 > --- gcc/common/config/avr/avr-common.cc > +++ gcc/common/config/avr/avr-common.cc >
[PATCH] Update array address space in c_build_qualified_type
Hi,
When c-typeck.cc:c_build_qualified_type builds an array type
from its element type, it does not copy the address space of
the element type to the array type itself. This is unlike
tree.cc:build_array_type_1, which explicitly does
TYPE_ADDR_SPACE (t) = TYPE_ADDR_SPACE (elt_type);
This causes the ICE described in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86869.
struct S {
char y[2];
};
extern const __memx struct S *s;
extern void bar(const __memx void*);
void foo(void) {
bar(&s->y);
}
build_component_ref calls c_build_qualified_type, passing in the
array type and quals including the address space (ADDR_SPACE_MEMX
in this case). Because of this missing address space copy, the
returned array type remains in the generic address space. Later
down the line, expand_expr_addr_expr detects the mismatch in
address space/mode and tries to convert, and that leads to the
ICE described in the bug.
This patch sets the address space of the array type to that of the
element type.
Regression tests for avr look ok. Ok for trunk?
Regards
Senthil
PR 86869
gcc/c/ChangeLog:
* c-typeck.cc (c_build_qualified_type): Set
TYPE_ADDR_SPACE for ARRAY_TYPE.
gcc/testsuite/ChangeLog:
* gcc.target/avr/pr86869.c: New test.
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 22e240a3c2a..d4ab1d1bd46 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -16284,6 +16284,7 @@ c_build_qualified_type (tree type, int type_quals, tree
orig_qual_type,
t = build_variant_type_copy (type);
TREE_TYPE (t) = element_type;
+ TYPE_ADDR_SPACE (t) = TYPE_ADDR_SPACE (element_type);
if (TYPE_STRUCTURAL_EQUALITY_P (element_type)
|| (domain && TYPE_STRUCTURAL_EQUALITY_P (domain)))
diff --git a/gcc/testsuite/gcc.target/avr/pr86869.c
b/gcc/testsuite/gcc.target/avr/pr86869.c
new file mode 100644
index 000..54cd984276e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/pr86869.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+extern void bar(const __memx void* p);
+
+struct S {
+ char y[2];
+};
+extern const __memx struct S *s;
+
+void foo(void) {
+ bar(&s->y);
+}
Re: [PATCH] Update array address space in c_build_qualified_type
On Wed, 2023-06-21 at 18:37 +, Joseph Myers wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Wed, 21 Jun 2023, Richard Biener via Gcc-patches wrote:
>
> > > This patch sets the address space of the array type to that of the
> > > element type.
> > >
> > > Regression tests for avr look ok. Ok for trunk?
> >
> > The patch looks OK to me but please let a C frontend maintainer
> > double-check (I've CCed Joseph for this).
>
> The question would be whether there are any TYPE_QUALS uses in the C front
> end that behave incorrectly given TYPE_ADDR_SPACE (acting as qualifiers)
> being set on an array type - conceptually, before C2x, array types are
> unqualified, only the element types are qualified.
Hmm, but tree.cc:build_array_type_1 sets the address space of the element
type to the array type, and the relevant commit's ChangeLog entry (from 2009)
says "Inherit array address space from element type."
On the avr target, for an array like
const __flash int arr[] = {1,2,3};
I can see that the array type gets the address space of the element type
already, even before this patch. Surely that must have caused any code that
doesn't expect address space in array type quals to be broken then?
Regards
Senthil
[Testsuite] Skip -fdelete-null-pointer-check tests if target keeps_null_pointer_checks
Hi,
When running regression tests related to
https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616772.html,
I noticed a bunch of failures because some tests explicitly pass in
-fdelete-null-pointer-checks, even if the target is configured to keep them.
This patch skips such failing tests by adding a dg-skip-if for
keeps_null_pointer_checks.
Ok to commit?
Regards
Senthil
gcc/testsuite/ChangeLog:
* gcc.dg/attr-returns-nonnull.c: Skip if
keeps_null_pointer_checks.
* gcc.dg/init-compare-1.c: Likewise.
* gcc.dg/ipa/pr85734.c: Likewise.
* gcc.dg/ipa/propmalloc-1.c: Likewise.
* gcc.dg/ipa/propmalloc-2.c: Likewise.
* gcc.dg/ipa/propmalloc-3.c: Likewise.
* gcc.dg/ipa/propmalloc-4.c: Likewise.
* gcc.dg/tree-ssa/evrp11.c: Likewise.
* gcc.dg/tree-ssa/pr83648.c: Likewise.
diff --git gcc/testsuite/gcc.dg/attr-returns-nonnull.c
gcc/testsuite/gcc.dg/attr-returns-nonnull.c
index e4e20b8..d7f39be 100644
--- gcc/testsuite/gcc.dg/attr-returns-nonnull.c
+++ gcc/testsuite/gcc.dg/attr-returns-nonnull.c
@@ -1,7 +1,8 @@
/* Verify that attribute returns_nonnull on global and local function
declarations is merged.
{ dg-do compile }
- { dg-options "-Wall -fdump-tree-optimized -fdelete-null-pointer-checks" } */
+ { dg-options "-Wall -fdump-tree-optimized -fdelete-null-pointer-checks" }
+ { dg-skip-if "" keeps_null_pointer_checks } */
void foo (void);
diff --git gcc/testsuite/gcc.dg/init-compare-1.c
gcc/testsuite/gcc.dg/init-compare-1.c
index 6737c85..a473f59 100644
--- gcc/testsuite/gcc.dg/init-compare-1.c
+++ gcc/testsuite/gcc.dg/init-compare-1.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-additional-options "-fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
extern int a, b;
int c = &a == &a;
diff --git gcc/testsuite/gcc.dg/ipa/pr85734.c gcc/testsuite/gcc.dg/ipa/pr85734.c
index cbd524b..b3f5c81 100644
--- gcc/testsuite/gcc.dg/ipa/pr85734.c
+++ gcc/testsuite/gcc.dg/ipa/pr85734.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -Wsuggest-attribute=malloc -fdelete-null-pointer-checks"
} */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
__attribute__((noinline))
static void *f1(__SIZE_TYPE__ sz) /* { dg-bogus "function might be candidate
for attribute 'malloc'" } */
diff --git gcc/testsuite/gcc.dg/ipa/propmalloc-1.c
gcc/testsuite/gcc.dg/ipa/propmalloc-1.c
index d7c13af..f5e8676 100644
--- gcc/testsuite/gcc.dg/ipa/propmalloc-1.c
+++ gcc/testsuite/gcc.dg/ipa/propmalloc-1.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-ipa-pure-const-details
-fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
__attribute__((noinline, no_icf, used))
static void *f(__SIZE_TYPE__ n)
diff --git gcc/testsuite/gcc.dg/ipa/propmalloc-2.c
gcc/testsuite/gcc.dg/ipa/propmalloc-2.c
index 2332d9a..e26af41 100644
--- gcc/testsuite/gcc.dg/ipa/propmalloc-2.c
+++ gcc/testsuite/gcc.dg/ipa/propmalloc-2.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-ipa-pure-const-details
-fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
__attribute__((noinline, used, no_icf))
static void *foo (__SIZE_TYPE__ n)
diff --git gcc/testsuite/gcc.dg/ipa/propmalloc-3.c
gcc/testsuite/gcc.dg/ipa/propmalloc-3.c
index 5386695..3329a99 100644
--- gcc/testsuite/gcc.dg/ipa/propmalloc-3.c
+++ gcc/testsuite/gcc.dg/ipa/propmalloc-3.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-ipa-pure-const-details
-fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
static void *foo(__SIZE_TYPE__, int) __attribute__((noinline, no_icf, used));
diff --git gcc/testsuite/gcc.dg/ipa/propmalloc-4.c
gcc/testsuite/gcc.dg/ipa/propmalloc-4.c
index 9552b73..23566e6 100644
--- gcc/testsuite/gcc.dg/ipa/propmalloc-4.c
+++ gcc/testsuite/gcc.dg/ipa/propmalloc-4.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-local-pure-const-details
-fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
void *foo(int cond1, int cond2, int cond3)
{
diff --git gcc/testsuite/gcc.dg/tree-ssa/evrp11.c
gcc/testsuite/gcc.dg/tree-ssa/evrp11.c
index d791305..018aded 100644
--- gcc/testsuite/gcc.dg/tree-ssa/evrp11.c
+++ gcc/testsuite/gcc.dg/tree-ssa/evrp11.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-evrp -fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
extern void link_error ();
diff --git gcc/testsuite/gcc.dg/tree-ssa/pr83648.c
gcc/testsuite/gcc.dg/tree-ssa/pr83648.c
index 954eb2f..d3dd12d 100644
--- gcc/testsuite/gcc.dg/tree-ssa/pr83648.c
+++ gcc/testsuite/gcc.dg/tree-ssa/pr83648.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-local-pure-const-details
-fdelete-null-pointer-checks" } */
+/* { dg-skip-if "" keeps_null_pointer
Re: [PATCH] avr: Set param_min_pagesize to 0 [PR105523]
On 26/04/23, 5:51 PM, "Richard Biener" mailto:richard.guent...@gmail.com>> wrote:
> On Wed, Apr 26, 2023 at 12:56 PM <mailto:senthilkumar.selva...@microchip.com>> wrote:
> >
> > On Wed, Apr 26, 2023 at 3:15 PM Richard Biener via Gcc-patches
> > mailto:gcc-patches@gcc.gnu.org>> wrote:
> > >
> > > On Wed, Apr 26, 2023 at 11:42 AM Richard Biener
> > > mailto:richard.guent...@gmail.com>> wrote:
> > > >
> > > > On Wed, Apr 26, 2023 at 11:01 AM SenthilKumar.Selvaraj--- via
> > > > Gcc-patches mailto:gcc-patches@gcc.gnu.org>>
> > > > wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > This patch fixes PR 105523 by setting param_min_pagesize to 0 for the
> > > > > avr target. For this target, zero and offsets from zero are perfectly
> > > > > valid addresses, and the default value of param_min_pagesize ends up
> > > > > triggering warnings on valid memory accesses.
> > > >
> > > > I think the proper configuration is to have
> > > > DEFAULT_ADDR_SPACE_ZERO_ADDRESS_VALID
> > >
> > > Err, TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
> >
> > That worked. Ok for trunk and backporting to 13 and 12 branches
> > (pending regression testing)?
>
>
> OK, but please let Denis time to comment.
Didn't hear from Denis. When running regression tests with this patch,
I found that some tests with -fdelete-null-pointer-checks were
failing. Commit 19416210b37db0584cd0b3f3b3961324b8973d25 made
-fdelete-null-pointer-checks false by default, while still allowing it
to be overridden from the command line (it was previously
unconditionally false).
To keep the same behavior, I modified the hook to report zero
addresses as valid only if -fdelete-null-pointer-checks is not set.
With this change, all regression tests pass.
Ok for trunk and backporting to 13 and 12 branches?
Regards
Senthil
PR 105523
gcc/ChangeLog:
* config/avr/avr.cc (avr_addr_space_zero_address_valid):
(TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): Return true if
flag_delete_null_pointer_checks is not set.
gcc/testsuite/ChangeLog:
* gcc.target/avr/pr105523.c: New test.
diff --git gcc/config/avr/avr.cc gcc/config/avr/avr.cc
index d5af40f..4c9eb84 100644
--- gcc/config/avr/avr.cc
+++ gcc/config/avr/avr.cc
@@ -9787,6 +9787,18 @@ avr_addr_space_diagnose_usage (addr_space_t as,
location_t loc)
(void) avr_addr_space_supported_p (as, loc);
}
+/* Implement `TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID. Zero is a valid
+ address in all address spaces. Even in ADDR_SPACE_FLASH1 etc..,
+ a zero address is valid and means 0x, where RAMPZ is
+ set to the appropriate segment value.
+ If the user explicitly passes in -fdelete-null-pointer-checks though,
+ assume zero addresses are invalid.*/
+
+static bool
+avr_addr_space_zero_address_valid (addr_space_t as ATTRIBUTE_UNUSED)
+{
+ return flag_delete_null_pointer_checks == 0;
+}
/* Look if DECL shall be placed in program memory space by
means of attribute `progmem' or some address-space qualifier.
@@ -14687,6 +14699,9 @@ avr_float_lib_compare_returns_bool (machine_mode mode,
enum rtx_code)
#undef TARGET_ADDR_SPACE_DIAGNOSE_USAGE
#define TARGET_ADDR_SPACE_DIAGNOSE_USAGE avr_addr_space_diagnose_usage
+#undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
+#define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID avr_addr_space_zero_address_valid
+
#undef TARGET_MODE_DEPENDENT_ADDRESS_P
#define TARGET_MODE_DEPENDENT_ADDRESS_P avr_mode_dependent_address_p
diff --git gcc/testsuite/gcc.target/avr/pr105523.c
gcc/testsuite/gcc.target/avr/pr105523.c
new file mode 100644
index 000..fbbf7bf
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr105523.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -Wall" } */
+
+/* Verify no "array subscript 0 is outside array bounds of" is generated
+ for accessing memory addresses in the 0-4096 range. */
+
+typedef __UINT8_TYPE__ uint8_t;
+
+#define SREG (*(volatile uint8_t*) (0x3F + __AVR_SFR_OFFSET__ ))
+
+void bar (void)
+{
+SREG = 0;
+}
[PATCH] avr: Set param_min_pagesize to 0 [PR105523]
Hi,
This patch fixes PR 105523 by setting param_min_pagesize to 0 for the
avr target. For this target, zero and offsets from zero are perfectly
valid addresses, and the default value of param_min_pagesize ends up
triggering warnings on valid memory accesses.
Ok for trunk and backporting to 13 and 12 branches?
Regards
Senthil
PR target/105523
gcc/ChangeLog:
* config/avr/avr.cc (avr_option_override): Set
param_min_pagesize to 0.
gcc/testsuite/ChangeLog:
* gcc.target/avr/pr105523.c: New test.
diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc
index c193430cf07..3b862f4e4ac 100644
--- a/gcc/config/avr/avr.cc
+++ b/gcc/config/avr/avr.cc
@@ -56,6 +56,7 @@
#include "tree-pass.h"
#include "print-rtl.h"
#include "rtl-iter.h"
+#include "opts.h"
/* This file should be included last. */
#include "target-def.h"
@@ -769,6 +770,9 @@ avr_option_override (void)
avr_gasisr_prologues = 0;
#endif
+ SET_OPTION_IF_UNSET (&global_options, &global_options_set,
+ param_min_pagesize, 0);
+
if (!avr_set_core_architecture())
return;
diff --git a/gcc/testsuite/gcc.target/avr/pr105523.c
b/gcc/testsuite/gcc.target/avr/pr105523.c
new file mode 100644
index 000..fbbf7bf4422
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/pr105523.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -Wall" } */
+
+/* Verify no "array subscript 0 is outside array bounds of" is generated
+ for accessing memory addresses in the 0-4096 range. */
+
+typedef __UINT8_TYPE__ uint8_t;
+
+#define SREG (*(volatile uint8_t*) (0x3F + __AVR_SFR_OFFSET__ ))
+
+void bar (void)
+{
+SREG = 0;
+}
Re: [PATCH] avr: Set param_min_pagesize to 0 [PR105523]
On Wed, Apr 26, 2023 at 3:15 PM Richard Biener via Gcc-patches
wrote:
>
> On Wed, Apr 26, 2023 at 11:42 AM Richard Biener
> wrote:
> >
> > On Wed, Apr 26, 2023 at 11:01 AM SenthilKumar.Selvaraj--- via
> > Gcc-patches wrote:
> > >
> > > Hi,
> > >
> > > This patch fixes PR 105523 by setting param_min_pagesize to 0 for the
> > > avr target. For this target, zero and offsets from zero are perfectly
> > > valid addresses, and the default value of param_min_pagesize ends up
> > > triggering warnings on valid memory accesses.
> >
> > I think the proper configuration is to have
> > DEFAULT_ADDR_SPACE_ZERO_ADDRESS_VALID
>
> Err, TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
That worked. Ok for trunk and backporting to 13 and 12 branches
(pending regression testing)?
Regards,
Senthil
PR 105523
gcc/ChangeLog:
* config/avr/avr.cc (avr_addr_space_zero_address_valid):
(TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID): Return true.
gcc/testsuite/ChangeLog:
* gcc.target/avr/pr105523.c: New test.
diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc
index c193430cf07..5439eb8e55c 100644
--- a/gcc/config/avr/avr.cc
+++ b/gcc/config/avr/avr.cc
@@ -9788,6 +9788,16 @@ avr_addr_space_diagnose_usage (addr_space_t as,
location_t loc)
(void) avr_addr_space_supported_p (as, loc);
}
+/* Implement `TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID. Zero is a valid
+ address in all address spaces. Even in ADDR_SPACE_FLASH1 etc.,
+ a zero address is valid and means 0x, where RAMPZ is
+ set to the appropriate segment value. */
+
+static bool
+avr_addr_space_zero_address_valid (addr_space_t as)
+{
+ return true;
+}
/* Look if DECL shall be placed in program memory space by
means of attribute `progmem' or some address-space qualifier.
@@ -14688,6 +14698,9 @@ avr_float_lib_compare_returns_bool (machine_mode mode,
enum rtx_code)
#undef TARGET_ADDR_SPACE_DIAGNOSE_USAGE
#define TARGET_ADDR_SPACE_DIAGNOSE_USAGE avr_addr_space_diagnose_usage
+#undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID
+#define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID avr_addr_space_zero_address_valid
+
#undef TARGET_MODE_DEPENDENT_ADDRESS_P
#define TARGET_MODE_DEPENDENT_ADDRESS_P avr_mode_dependent_address_p
diff --git a/gcc/testsuite/gcc.target/avr/pr105523.c
b/gcc/testsuite/gcc.target/avr/pr105523.c
new file mode 100644
index 000..fbbf7bf4422
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/pr105523.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -Wall" } */
+
+/* Verify no "array subscript 0 is outside array bounds of" is generated
+ for accessing memory addresses in the 0-4096 range. */
+
+typedef __UINT8_TYPE__ uint8_t;
+
+#define SREG (*(volatile uint8_t*) (0x3F + __AVR_SFR_OFFSET__ ))
+
+void bar (void)
+{
+SREG = 0;
+}
