* [PATCH] `const volatile' sections selection - bugzilla #107181 @ 2022-12-02 17:52 Cupertino Miranda 2022-12-02 17:52 ` [PATCH 1/2] select .rodata for const volatile variables Cupertino Miranda 2022-12-02 17:52 ` [PATCH 2/2] Corrected pr25521.c target matching Cupertino Miranda 0 siblings, 2 replies; 30+ messages in thread From: Cupertino Miranda @ 2022-12-02 17:52 UTC (permalink / raw) To: gcc-patches; +Cc: cupertino.miranda, jose.marchesi Hi everyone, Recently I looked over the issue reported in bugzilla #107181, related to commit `a0aafbc'. The commit changes the object section for `const volatile' objects. However it does it only for the targets using the default section selection hook. The included patches addresses all the architectures that define a custom section select by verifying its result against pr25521.c test. From all architectures verified only powerpc and v850 seem to be having a non-default behaviour and required changes. - v850_select_section was changed to follow the behaviour the default select_section hook. - powerpc only has issues in 32bit, since the compiler is eager to allocate the simple const volatile object in small data section, which does not appear to have a readonly counter-part support. Please find more details in the patch commit message. Looking forward to your review. Best regards, Cupertino Miranda ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/2] select .rodata for const volatile variables. 2022-12-02 17:52 [PATCH] `const volatile' sections selection - bugzilla #107181 Cupertino Miranda @ 2022-12-02 17:52 ` Cupertino Miranda 2022-12-05 18:06 ` Jeff Law 2022-12-02 17:52 ` [PATCH 2/2] Corrected pr25521.c target matching Cupertino Miranda 1 sibling, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2022-12-02 17:52 UTC (permalink / raw) To: gcc-patches; +Cc: cupertino.miranda, jose.marchesi Changed target code to select .rodata section for 'const volatile' defined variables. This change is in the context of the bugzilla #170181. gcc/ChangeLog: v850.c(v850_select_section): Changed function. --- gcc/config/v850/v850.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc index c7d432990ab..e66893fede4 100644 --- a/gcc/config/v850/v850.cc +++ b/gcc/config/v850/v850.cc @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, { int is_const; if (!TREE_READONLY (exp) - || TREE_SIDE_EFFECTS (exp) || !DECL_INITIAL (exp) || (DECL_INITIAL (exp) != error_mark_node && !TREE_CONSTANT (DECL_INITIAL (exp)))) -- 2.30.2 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2022-12-02 17:52 ` [PATCH 1/2] select .rodata for const volatile variables Cupertino Miranda @ 2022-12-05 18:06 ` Jeff Law 2022-12-07 15:12 ` Cupertino Miranda 2023-01-09 7:57 ` Richard Biener 0 siblings, 2 replies; 30+ messages in thread From: Jeff Law @ 2022-12-05 18:06 UTC (permalink / raw) To: Cupertino Miranda, gcc-patches; +Cc: jose.marchesi On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: > Changed target code to select .rodata section for 'const volatile' > defined variables. > This change is in the context of the bugzilla #170181. > > gcc/ChangeLog: > > v850.c(v850_select_section): Changed function. I'm not sure this is safe/correct. ISTM that you need to look at the underlying TREE_TYPE to check for const-volatile rather than TREE_SIDE_EFFECTS. Of secondary importance is the ChangeLog. Just saying "Changed function" provides no real information. Something like this would be better: * config/v850/v850.c (v850_select_section): Put const volatile objects into read-only sections. Jeff > --- > gcc/config/v850/v850.cc | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc > index c7d432990ab..e66893fede4 100644 > --- a/gcc/config/v850/v850.cc > +++ b/gcc/config/v850/v850.cc > @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, > { > int is_const; > if (!TREE_READONLY (exp) > - || TREE_SIDE_EFFECTS (exp) > || !DECL_INITIAL (exp) > || (DECL_INITIAL (exp) != error_mark_node > && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2022-12-05 18:06 ` Jeff Law @ 2022-12-07 15:12 ` Cupertino Miranda 2022-12-15 10:13 ` Cupertino Miranda 2023-01-09 7:57 ` Richard Biener 1 sibling, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2022-12-07 15:12 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, jose.marchesi Hi Jeff, First of all thanks for your quick review. Apologies for the delay replying, the message got lost in my inbox. > On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >> Changed target code to select .rodata section for 'const volatile' >> defined variables. >> This change is in the context of the bugzilla #170181. >> gcc/ChangeLog: >> v850.c(v850_select_section): Changed function. > I'm not sure this is safe/correct. ISTM that you need to look at the underlying > TREE_TYPE to check for const-volatile rather than TREE_SIDE_EFFECTS. I believe this was asked by Jose when he first sent the generic patches. Please notice my change is influenced by his original patch that does the same and was approved. https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599348.html https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602374.html > > Of secondary importance is the ChangeLog. Just saying "Changed function" > provides no real information. Something like this would be better: > > * config/v850/v850.c (v850_select_section): Put const volatile > objects into read-only sections. > > > Jeff > > > > >> --- >> gcc/config/v850/v850.cc | 1 - >> 1 file changed, 1 deletion(-) >> diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc >> index c7d432990ab..e66893fede4 100644 >> --- a/gcc/config/v850/v850.cc >> +++ b/gcc/config/v850/v850.cc >> @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, >> { >> int is_const; >> if (!TREE_READONLY (exp) >> - || TREE_SIDE_EFFECTS (exp) >> || !DECL_INITIAL (exp) >> || (DECL_INITIAL (exp) != error_mark_node >> && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2022-12-07 15:12 ` Cupertino Miranda @ 2022-12-15 10:13 ` Cupertino Miranda 2022-12-22 17:21 ` [PING] " Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2022-12-15 10:13 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, jose.marchesi gentle ping Cupertino Miranda writes: > Hi Jeff, > > First of all thanks for your quick review. > Apologies for the delay replying, the message got lost in my inbox. > >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> Changed target code to select .rodata section for 'const volatile' >>> defined variables. >>> This change is in the context of the bugzilla #170181. >>> gcc/ChangeLog: >>> v850.c(v850_select_section): Changed function. >> I'm not sure this is safe/correct. ISTM that you need to look at the underlying >> TREE_TYPE to check for const-volatile rather than TREE_SIDE_EFFECTS. > > I believe this was asked by Jose when he first sent the generic patches. > Please notice my change is influenced by his original patch that does > the same and was approved. > > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599348.html > https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602374.html > >> >> Of secondary importance is the ChangeLog. Just saying "Changed function" >> provides no real information. Something like this would be better: >> >> * config/v850/v850.c (v850_select_section): Put const volatile >> objects into read-only sections. >> >> >> Jeff >> >> >> >> >>> --- >>> gcc/config/v850/v850.cc | 1 - >>> 1 file changed, 1 deletion(-) >>> diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc >>> index c7d432990ab..e66893fede4 100644 >>> --- a/gcc/config/v850/v850.cc >>> +++ b/gcc/config/v850/v850.cc >>> @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, >>> { >>> int is_const; >>> if (!TREE_READONLY (exp) >>> - || TREE_SIDE_EFFECTS (exp) >>> || !DECL_INITIAL (exp) >>> || (DECL_INITIAL (exp) != error_mark_node >>> && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PING] Re: [PATCH 1/2] select .rodata for const volatile variables. 2022-12-15 10:13 ` Cupertino Miranda @ 2022-12-22 17:21 ` Cupertino Miranda 2023-01-02 10:42 ` Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2022-12-22 17:21 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Jeff Law, jose.marchesi, gcc-patches Cupertino Miranda via Gcc-patches writes: > gentle ping > > Cupertino Miranda writes: > >> Hi Jeff, >> >> First of all thanks for your quick review. >> Apologies for the delay replying, the message got lost in my inbox. >> >>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>> Changed target code to select .rodata section for 'const volatile' >>>> defined variables. >>>> This change is in the context of the bugzilla #170181. >>>> gcc/ChangeLog: >>>> v850.c(v850_select_section): Changed function. >>> I'm not sure this is safe/correct. ISTM that you need to look at the underlying >>> TREE_TYPE to check for const-volatile rather than TREE_SIDE_EFFECTS. >> >> I believe this was asked by Jose when he first sent the generic patches. >> Please notice my change is influenced by his original patch that does >> the same and was approved. >> >> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599348.html >> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602374.html >> >>> >>> Of secondary importance is the ChangeLog. Just saying "Changed function" >>> provides no real information. Something like this would be better: >>> >>> * config/v850/v850.c (v850_select_section): Put const volatile >>> objects into read-only sections. >>> >>> >>> Jeff >>> >>> >>> >>> >>>> --- >>>> gcc/config/v850/v850.cc | 1 - >>>> 1 file changed, 1 deletion(-) >>>> diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc >>>> index c7d432990ab..e66893fede4 100644 >>>> --- a/gcc/config/v850/v850.cc >>>> +++ b/gcc/config/v850/v850.cc >>>> @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, >>>> { >>>> int is_const; >>>> if (!TREE_READONLY (exp) >>>> - || TREE_SIDE_EFFECTS (exp) >>>> || !DECL_INITIAL (exp) >>>> || (DECL_INITIAL (exp) != error_mark_node >>>> && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PING] Re: [PATCH 1/2] select .rodata for const volatile variables. 2022-12-22 17:21 ` [PING] " Cupertino Miranda @ 2023-01-02 10:42 ` Cupertino Miranda 0 siblings, 0 replies; 30+ messages in thread From: Cupertino Miranda @ 2023-01-02 10:42 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Jeff Law, jose.marchesi, gcc-patches PING PING Cupertino Miranda writes: > Cupertino Miranda via Gcc-patches writes: > >> gentle ping >> >> Cupertino Miranda writes: >> >>> Hi Jeff, >>> >>> First of all thanks for your quick review. >>> Apologies for the delay replying, the message got lost in my inbox. >>> >>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>> Changed target code to select .rodata section for 'const volatile' >>>>> defined variables. >>>>> This change is in the context of the bugzilla #170181. >>>>> gcc/ChangeLog: >>>>> v850.c(v850_select_section): Changed function. >>>> I'm not sure this is safe/correct. ISTM that you need to look at the underlying >>>> TREE_TYPE to check for const-volatile rather than TREE_SIDE_EFFECTS. >>> >>> I believe this was asked by Jose when he first sent the generic patches. >>> Please notice my change is influenced by his original patch that does >>> the same and was approved. >>> >>> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599348.html >>> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602374.html >>> >>>> >>>> Of secondary importance is the ChangeLog. Just saying "Changed function" >>>> provides no real information. Something like this would be better: >>>> >>>> * config/v850/v850.c (v850_select_section): Put const volatile >>>> objects into read-only sections. >>>> >>>> >>>> Jeff >>>> >>>> >>>> >>>> >>>>> --- >>>>> gcc/config/v850/v850.cc | 1 - >>>>> 1 file changed, 1 deletion(-) >>>>> diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc >>>>> index c7d432990ab..e66893fede4 100644 >>>>> --- a/gcc/config/v850/v850.cc >>>>> +++ b/gcc/config/v850/v850.cc >>>>> @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, >>>>> { >>>>> int is_const; >>>>> if (!TREE_READONLY (exp) >>>>> - || TREE_SIDE_EFFECTS (exp) >>>>> || !DECL_INITIAL (exp) >>>>> || (DECL_INITIAL (exp) != error_mark_node >>>>> && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2022-12-05 18:06 ` Jeff Law 2022-12-07 15:12 ` Cupertino Miranda @ 2023-01-09 7:57 ` Richard Biener 2023-01-13 15:06 ` Cupertino Miranda 2023-01-22 18:49 ` Jeff Law 1 sibling, 2 replies; 30+ messages in thread From: Richard Biener @ 2023-01-09 7:57 UTC (permalink / raw) To: Jeff Law; +Cc: Cupertino Miranda, gcc-patches, jose.marchesi On Mon, Dec 5, 2022 at 7:07 PM Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: > > Changed target code to select .rodata section for 'const volatile' > > defined variables. > > This change is in the context of the bugzilla #170181. > > > > gcc/ChangeLog: > > > > v850.c(v850_select_section): Changed function. > I'm not sure this is safe/correct. ISTM that you need to look at the > underlying TREE_TYPE to check for const-volatile rather than > TREE_SIDE_EFFECTS. Just to quote tree.h: /* In any expression, decl, or constant, nonzero means it has side effects or reevaluation of the whole expression could produce a different value. This is set if any subexpression is a function call, a side effect or a reference to a volatile variable. In a ..._DECL, this is set only if the declaration said `volatile'. This will never be set for a constant. */ #define TREE_SIDE_EFFECTS(NODE) \ (NON_TYPE_CHECK (NODE)->base.side_effects_flag) so if exp is a decl then that's the volatile check. > Of secondary importance is the ChangeLog. Just saying "Changed > function" provides no real information. Something like this would be > better: > > * config/v850/v850.c (v850_select_section): Put const volatile > objects into read-only sections. > > > Jeff > > > > > > --- > > gcc/config/v850/v850.cc | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc > > index c7d432990ab..e66893fede4 100644 > > --- a/gcc/config/v850/v850.cc > > +++ b/gcc/config/v850/v850.cc > > @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, > > { > > int is_const; > > if (!TREE_READONLY (exp) > > - || TREE_SIDE_EFFECTS (exp) > > || !DECL_INITIAL (exp) > > || (DECL_INITIAL (exp) != error_mark_node > > && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2023-01-09 7:57 ` Richard Biener @ 2023-01-13 15:06 ` Cupertino Miranda 2023-01-19 9:59 ` Cupertino Miranda 2023-01-22 18:49 ` Jeff Law 1 sibling, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-01-13 15:06 UTC (permalink / raw) To: Richard Biener; +Cc: Jeff Law, gcc-patches, jose.marchesi Richard Biener writes: > On Mon, Dec 5, 2022 at 7:07 PM Jeff Law via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> >> >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >> > Changed target code to select .rodata section for 'const volatile' >> > defined variables. >> > This change is in the context of the bugzilla #170181. >> > >> > gcc/ChangeLog: >> > >> > v850.c(v850_select_section): Changed function. >> I'm not sure this is safe/correct. ISTM that you need to look at the >> underlying TREE_TYPE to check for const-volatile rather than >> TREE_SIDE_EFFECTS. > > Just to quote tree.h: > > /* In any expression, decl, or constant, nonzero means it has side effects or > reevaluation of the whole expression could produce a different value. > This is set if any subexpression is a function call, a side effect or a > reference to a volatile variable. In a ..._DECL, this is set only if the > declaration said `volatile'. This will never be set for a constant. */ > #define TREE_SIDE_EFFECTS(NODE) \ > (NON_TYPE_CHECK (NODE)->base.side_effects_flag) > > so if exp is a decl then that's the volatile check. > Thank you Richard for the review. Jeff: Can you please let me know if Richard comments reply to your concerns? Cupertino >> Of secondary importance is the ChangeLog. Just saying "Changed >> function" provides no real information. Something like this would be >> better: >> >> * config/v850/v850.c (v850_select_section): Put const volatile >> objects into read-only sections. >> >> >> Jeff >> >> >> >> >> > --- >> > gcc/config/v850/v850.cc | 1 - >> > 1 file changed, 1 deletion(-) >> > >> > diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc >> > index c7d432990ab..e66893fede4 100644 >> > --- a/gcc/config/v850/v850.cc >> > +++ b/gcc/config/v850/v850.cc >> > @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, >> > { >> > int is_const; >> > if (!TREE_READONLY (exp) >> > - || TREE_SIDE_EFFECTS (exp) >> > || !DECL_INITIAL (exp) >> > || (DECL_INITIAL (exp) != error_mark_node >> > && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2023-01-13 15:06 ` Cupertino Miranda @ 2023-01-19 9:59 ` Cupertino Miranda 2023-01-22 18:43 ` Jeff Law 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-01-19 9:59 UTC (permalink / raw) To: Jeff Law; +Cc: Richard Biener, jose.marchesi, gcc-patches Hi Jeff, Kindly calling your attention to this thread. Regards, Cupertino Cupertino Miranda via Gcc-patches writes: > Richard Biener writes: > >> On Mon, Dec 5, 2022 at 7:07 PM Jeff Law via Gcc-patches >> <gcc-patches@gcc.gnu.org> wrote: >>> >>> >>> >>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> > Changed target code to select .rodata section for 'const volatile' >>> > defined variables. >>> > This change is in the context of the bugzilla #170181. >>> > >>> > gcc/ChangeLog: >>> > >>> > v850.c(v850_select_section): Changed function. >>> I'm not sure this is safe/correct. ISTM that you need to look at the >>> underlying TREE_TYPE to check for const-volatile rather than >>> TREE_SIDE_EFFECTS. >> >> Just to quote tree.h: >> >> /* In any expression, decl, or constant, nonzero means it has side effects or >> reevaluation of the whole expression could produce a different value. >> This is set if any subexpression is a function call, a side effect or a >> reference to a volatile variable. In a ..._DECL, this is set only if the >> declaration said `volatile'. This will never be set for a constant. */ >> #define TREE_SIDE_EFFECTS(NODE) \ >> (NON_TYPE_CHECK (NODE)->base.side_effects_flag) >> >> so if exp is a decl then that's the volatile check. >> > > Thank you Richard for the review. > Jeff: Can you please let me know if Richard comments reply to your > concerns? > > Cupertino > >>> Of secondary importance is the ChangeLog. Just saying "Changed >>> function" provides no real information. Something like this would be >>> better: >>> >>> * config/v850/v850.c (v850_select_section): Put const volatile >>> objects into read-only sections. >>> >>> >>> Jeff >>> >>> >>> >>> >>> > --- >>> > gcc/config/v850/v850.cc | 1 - >>> > 1 file changed, 1 deletion(-) >>> > >>> > diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc >>> > index c7d432990ab..e66893fede4 100644 >>> > --- a/gcc/config/v850/v850.cc >>> > +++ b/gcc/config/v850/v850.cc >>> > @@ -2865,7 +2865,6 @@ v850_select_section (tree exp, >>> > { >>> > int is_const; >>> > if (!TREE_READONLY (exp) >>> > - || TREE_SIDE_EFFECTS (exp) >>> > || !DECL_INITIAL (exp) >>> > || (DECL_INITIAL (exp) != error_mark_node >>> > && !TREE_CONSTANT (DECL_INITIAL (exp)))) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2023-01-19 9:59 ` Cupertino Miranda @ 2023-01-22 18:43 ` Jeff Law 0 siblings, 0 replies; 30+ messages in thread From: Jeff Law @ 2023-01-22 18:43 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Richard Biener, jose.marchesi, gcc-patches On 1/19/23 02:59, Cupertino Miranda wrote: > > Hi Jeff, > > Kindly calling your attention to this thread. Sorry, just crazy busy around here. Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] select .rodata for const volatile variables. 2023-01-09 7:57 ` Richard Biener 2023-01-13 15:06 ` Cupertino Miranda @ 2023-01-22 18:49 ` Jeff Law 1 sibling, 0 replies; 30+ messages in thread From: Jeff Law @ 2023-01-22 18:49 UTC (permalink / raw) To: Richard Biener; +Cc: Cupertino Miranda, gcc-patches, jose.marchesi On 1/9/23 00:57, Richard Biener wrote: > On Mon, Dec 5, 2022 at 7:07 PM Jeff Law via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> >> >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> Changed target code to select .rodata section for 'const volatile' >>> defined variables. >>> This change is in the context of the bugzilla #170181. >>> >>> gcc/ChangeLog: >>> >>> v850.c(v850_select_section): Changed function. >> I'm not sure this is safe/correct. ISTM that you need to look at the >> underlying TREE_TYPE to check for const-volatile rather than >> TREE_SIDE_EFFECTS. > > Just to quote tree.h: > > /* In any expression, decl, or constant, nonzero means it has side effects or > reevaluation of the whole expression could produce a different value. > This is set if any subexpression is a function call, a side effect or a > reference to a volatile variable. In a ..._DECL, this is set only if the > declaration said `volatile'. This will never be set for a constant. */ > #define TREE_SIDE_EFFECTS(NODE) \ > (NON_TYPE_CHECK (NODE)->base.side_effects_flag) > > so if exp is a decl then that's the volatile check. Ah, then we can just use TREE_SIDE_EFFECTS for testing if a _DECL node is volatile. So that concern is a non-issue. I think that was the only concern with patch #1. I'll install it momentarily. Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-02 17:52 [PATCH] `const volatile' sections selection - bugzilla #107181 Cupertino Miranda 2022-12-02 17:52 ` [PATCH 1/2] select .rodata for const volatile variables Cupertino Miranda @ 2022-12-02 17:52 ` Cupertino Miranda 2022-12-05 18:47 ` Jeff Law 1 sibling, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2022-12-02 17:52 UTC (permalink / raw) To: gcc-patches; +Cc: cupertino.miranda, jose.marchesi This commit is a follow up of bugzilla #107181. The commit /a0aafbc/ changed the default implementation of the SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the placement of `const volatile' objects. However, the following targets use target-specific selection functions and they choke on the testcase pr25521.c: *rx - target sets its const variables as '.section C,"a",@progbits'. *powerpc - its 32bit version is eager to allocate globals in .sdata sections. Normally, one can expect for the variable to be allocated in .srodata, however, in case of powerpc-*-* or powerpc64-*-* (with -m32) 'targetm.have_srodata_section == false' and the code in categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. /* If the target uses small data sections, select it. */ else if (targetm.in_small_data_p (decl)) { if (ret == SECCAT_BSS) ret = SECCAT_SBSS; else if targetm.have_srodata_section && ret == SECCAT_RODATA) ret = SECCAT_SRODATA; else ret = SECCAT_SDATA; } LLVM compiler does not generate .sdata symbols at all, having different code generation even for non const volatile symbols. Targets that for acceptable reasons could not match the LLVM generated code were marked as XFAIL. gcc/testsuite/ChangeLog: * lib/target-supports.exp: Added check_effective_target_const_volatile_readonly_section. * gcc.dg/pr25521.c: Added XFAIL. --- gcc/testsuite/gcc.dg/pr25521.c | 3 +-- gcc/testsuite/lib/target-supports.exp | 12 ++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c index 63363a03b9f..597a2fc25d8 100644 --- a/gcc/testsuite/gcc.dg/pr25521.c +++ b/gcc/testsuite/gcc.dg/pr25521.c @@ -6,5 +6,4 @@ const volatile int foo = 30; - -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { xfail { ! const_volatile_readonly_section } } } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 2a058c67c53..631d4593447 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -12196,3 +12196,15 @@ proc check_is_prog_name_available { prog } { return 1 } + +# returns 1 if target does selects a readonly section for const volatile variables. +proc check_effective_target_const_volatile_readonly_section { } { + + if { [istarget rx*-*-*] + || [istarget powerpc-*-*] + || [istarget rs6000*-*-*] + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { + return 0 + } + return 1 +} -- 2.30.2 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-02 17:52 ` [PATCH 2/2] Corrected pr25521.c target matching Cupertino Miranda @ 2022-12-05 18:47 ` Jeff Law 2022-12-07 15:45 ` Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Jeff Law @ 2022-12-05 18:47 UTC (permalink / raw) To: Cupertino Miranda, gcc-patches; +Cc: jose.marchesi On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: > This commit is a follow up of bugzilla #107181. > > The commit /a0aafbc/ changed the default implementation of the > SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the > placement of `const volatile' objects. > > However, the following targets use target-specific selection functions > and they choke on the testcase pr25521.c: > > *rx - target sets its const variables as '.section C,"a",@progbits'. That's presumably a constant section. We should instead twiddle the test to recognize that section. > > *powerpc - its 32bit version is eager to allocate globals in .sdata > sections. > > Normally, one can expect for the variable to be allocated in .srodata, > however, in case of powerpc-*-* or powerpc64-*-* (with -m32) > 'targetm.have_srodata_section == false' and the code in > categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. > > /* If the target uses small data sections, select it. */ > else if (targetm.in_small_data_p (decl)) > { > if (ret == SECCAT_BSS) > ret = SECCAT_SBSS; > else if targetm.have_srodata_section && ret == SECCAT_RODATA) > ret = SECCAT_SRODATA; > else > ret = SECCAT_SDATA; > } I'd just skip the test for 32bit ppc. There should be suitable effective-target tests you can use. jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-05 18:47 ` Jeff Law @ 2022-12-07 15:45 ` Cupertino Miranda 2022-12-15 10:14 ` Cupertino Miranda ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Cupertino Miranda @ 2022-12-07 15:45 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, jose.marchesi > On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >> This commit is a follow up of bugzilla #107181. >> The commit /a0aafbc/ changed the default implementation of the >> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >> placement of `const volatile' objects. >> However, the following targets use target-specific selection functions >> and they choke on the testcase pr25521.c: >> *rx - target sets its const variables as '.section C,"a",@progbits'. > That's presumably a constant section. We should instead twiddle the test to > recognize that section. Although @progbits is indeed a constant section, I believe it is more interesting to detect if the `rx' starts selecting more standard sections instead of the current @progbits. That was the reason why I opted to XFAIL instead of PASSing it. Can I keep it as such ? > >> *powerpc - its 32bit version is eager to allocate globals in .sdata >> sections. >> Normally, one can expect for the variable to be allocated in .srodata, >> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >> 'targetm.have_srodata_section == false' and the code in >> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >> /* If the target uses small data sections, select it. */ >> else if (targetm.in_small_data_p (decl)) >> { >> if (ret == SECCAT_BSS) >> ret = SECCAT_SBSS; >> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >> ret = SECCAT_SRODATA; >> else >> ret = SECCAT_SDATA; >> } > I'd just skip the test for 32bit ppc. There should be suitable effective-target > tests you can use. > > jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-07 15:45 ` Cupertino Miranda @ 2022-12-15 10:14 ` Cupertino Miranda 2022-12-22 17:22 ` [PING] " Cupertino Miranda 2023-01-13 15:13 ` Cupertino Miranda 2023-01-22 19:04 ` Jeff Law 2 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2022-12-15 10:14 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, jose.marchesi gentle ping Cupertino Miranda writes: >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> This commit is a follow up of bugzilla #107181. >>> The commit /a0aafbc/ changed the default implementation of the >>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>> placement of `const volatile' objects. >>> However, the following targets use target-specific selection functions >>> and they choke on the testcase pr25521.c: >>> *rx - target sets its const variables as '.section C,"a",@progbits'. >> That's presumably a constant section. We should instead twiddle the test to >> recognize that section. > > Although @progbits is indeed a constant section, I believe it is > more interesting to detect if the `rx' starts selecting more > standard sections instead of the current @progbits. > That was the reason why I opted to XFAIL instead of PASSing it. > Can I keep it as such ? > >> >>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>> sections. >>> Normally, one can expect for the variable to be allocated in .srodata, >>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>> 'targetm.have_srodata_section == false' and the code in >>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>> /* If the target uses small data sections, select it. */ >>> else if (targetm.in_small_data_p (decl)) >>> { >>> if (ret == SECCAT_BSS) >>> ret = SECCAT_SBSS; >>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>> ret = SECCAT_SRODATA; >>> else >>> ret = SECCAT_SDATA; >>> } >> I'd just skip the test for 32bit ppc. There should be suitable effective-target >> tests you can use. >> >> jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PING] Re: [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-15 10:14 ` Cupertino Miranda @ 2022-12-22 17:22 ` Cupertino Miranda 2023-01-02 10:43 ` Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2022-12-22 17:22 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Jeff Law, jose.marchesi, gcc-patches Cupertino Miranda via Gcc-patches writes: > gentle ping > > Cupertino Miranda writes: > >>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>> This commit is a follow up of bugzilla #107181. >>>> The commit /a0aafbc/ changed the default implementation of the >>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>> placement of `const volatile' objects. >>>> However, the following targets use target-specific selection functions >>>> and they choke on the testcase pr25521.c: >>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>> That's presumably a constant section. We should instead twiddle the test to >>> recognize that section. >> >> Although @progbits is indeed a constant section, I believe it is >> more interesting to detect if the `rx' starts selecting more >> standard sections instead of the current @progbits. >> That was the reason why I opted to XFAIL instead of PASSing it. >> Can I keep it as such ? >> >>> >>>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>>> sections. >>>> Normally, one can expect for the variable to be allocated in .srodata, >>>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>>> 'targetm.have_srodata_section == false' and the code in >>>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>>> /* If the target uses small data sections, select it. */ >>>> else if (targetm.in_small_data_p (decl)) >>>> { >>>> if (ret == SECCAT_BSS) >>>> ret = SECCAT_SBSS; >>>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>>> ret = SECCAT_SRODATA; >>>> else >>>> ret = SECCAT_SDATA; >>>> } >>> I'd just skip the test for 32bit ppc. There should be suitable effective-target >>> tests you can use. >>> >>> jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PING] Re: [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-22 17:22 ` [PING] " Cupertino Miranda @ 2023-01-02 10:43 ` Cupertino Miranda 0 siblings, 0 replies; 30+ messages in thread From: Cupertino Miranda @ 2023-01-02 10:43 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Jeff Law, jose.marchesi, gcc-patches PING PING Cupertino Miranda writes: > Cupertino Miranda via Gcc-patches writes: > >> gentle ping >> >> Cupertino Miranda writes: >> >>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>> This commit is a follow up of bugzilla #107181. >>>>> The commit /a0aafbc/ changed the default implementation of the >>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>> placement of `const volatile' objects. >>>>> However, the following targets use target-specific selection functions >>>>> and they choke on the testcase pr25521.c: >>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>> That's presumably a constant section. We should instead twiddle the test to >>>> recognize that section. >>> >>> Although @progbits is indeed a constant section, I believe it is >>> more interesting to detect if the `rx' starts selecting more >>> standard sections instead of the current @progbits. >>> That was the reason why I opted to XFAIL instead of PASSing it. >>> Can I keep it as such ? >>> >>>> >>>>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>>>> sections. >>>>> Normally, one can expect for the variable to be allocated in .srodata, >>>>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>>>> 'targetm.have_srodata_section == false' and the code in >>>>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>>>> /* If the target uses small data sections, select it. */ >>>>> else if (targetm.in_small_data_p (decl)) >>>>> { >>>>> if (ret == SECCAT_BSS) >>>>> ret = SECCAT_SBSS; >>>>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>>>> ret = SECCAT_SRODATA; >>>>> else >>>>> ret = SECCAT_SDATA; >>>>> } >>>> I'd just skip the test for 32bit ppc. There should be suitable effective-target >>>> tests you can use. >>>> >>>> jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-07 15:45 ` Cupertino Miranda 2022-12-15 10:14 ` Cupertino Miranda @ 2023-01-13 15:13 ` Cupertino Miranda 2023-01-22 19:04 ` Jeff Law 2 siblings, 0 replies; 30+ messages in thread From: Cupertino Miranda @ 2023-01-13 15:13 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, jose.marchesi Cupertino Miranda writes: >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> This commit is a follow up of bugzilla #107181. >>> The commit /a0aafbc/ changed the default implementation of the >>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>> placement of `const volatile' objects. >>> However, the following targets use target-specific selection functions >>> and they choke on the testcase pr25521.c: >>> *rx - target sets its const variables as '.section C,"a",@progbits'. >> That's presumably a constant section. We should instead twiddle the test to >> recognize that section. > > Although @progbits is indeed a constant section, I believe it is > more interesting to detect if the `rx' starts selecting more > standard sections instead of the current @progbits. > That was the reason why I opted to XFAIL instead of PASSing it. > Can I keep it as such ? > Jeff: Can you please give me an answer on this ? Cupertino >> >>> *powerpc - its 32bit version is eager to allocate globals in .sdata >>> sections. >>> Normally, one can expect for the variable to be allocated in .srodata, >>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32) >>> 'targetm.have_srodata_section == false' and the code in >>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata. >>> /* If the target uses small data sections, select it. */ >>> else if (targetm.in_small_data_p (decl)) >>> { >>> if (ret == SECCAT_BSS) >>> ret = SECCAT_SBSS; >>> else if targetm.have_srodata_section && ret == SECCAT_RODATA) >>> ret = SECCAT_SRODATA; >>> else >>> ret = SECCAT_SDATA; >>> } >> I'd just skip the test for 32bit ppc. There should be suitable effective-target >> tests you can use. >> >> jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2022-12-07 15:45 ` Cupertino Miranda 2022-12-15 10:14 ` Cupertino Miranda 2023-01-13 15:13 ` Cupertino Miranda @ 2023-01-22 19:04 ` Jeff Law 2023-01-24 12:24 ` Cupertino Miranda 2 siblings, 1 reply; 30+ messages in thread From: Jeff Law @ 2023-01-22 19:04 UTC (permalink / raw) To: Cupertino Miranda; +Cc: gcc-patches, jose.marchesi On 12/7/22 08:45, Cupertino Miranda wrote: > >> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>> This commit is a follow up of bugzilla #107181. >>> The commit /a0aafbc/ changed the default implementation of the >>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>> placement of `const volatile' objects. >>> However, the following targets use target-specific selection functions >>> and they choke on the testcase pr25521.c: >>> *rx - target sets its const variables as '.section C,"a",@progbits'. >> That's presumably a constant section. We should instead twiddle the test to >> recognize that section. > > Although @progbits is indeed a constant section, I believe it is > more interesting to detect if the `rx' starts selecting more > standard sections instead of the current @progbits. > That was the reason why I opted to XFAIL instead of PASSing it. > Can I keep it as such ? I'm not aware of any ongoing development for that port, so I would not let concerns about the rx port changing behavior dominate how we approach this problem. The rx port is using a different name for the section. That's valid thing to do and to the extent we can, we should support that in the test rather than (incorrectly IMHO) xfailing the test just becuase the name isn't what we expected. To avoid over-eagerly matching, I would probably search for "C," I wouldn't do that for the const or rodata sections as they often have a suffix like 1, 2, 4, 8 for different sized rodata sections. PPC32 is explicitly doing something different and placing those objects into an RW section. So for PPC32 it makes more sense to skip the test rather than xfail it. Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-01-22 19:04 ` Jeff Law @ 2023-01-24 12:24 ` Cupertino Miranda 2023-01-31 9:10 ` [PING] " Cupertino Miranda 2023-03-11 16:25 ` Jeff Law 0 siblings, 2 replies; 30+ messages in thread From: Cupertino Miranda @ 2023-01-24 12:24 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, jose.marchesi [-- Attachment #1: Type: text/plain, Size: 326 bytes --] Thank you for the comments and suggestions. I have changed the patch. Unfortunately in case of rx target I could not make scan-assembler-symbol-section to match. I believe it is because the .section and .global entries order is reversed in this target. Patch in inlined below. looking forward to your comments. Cupertino [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: a.patch --] [-- Type: text/x-diff, Size: 1240 bytes --] diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c index 63363a03b9f..82b4cd88ec0 100644 --- a/gcc/testsuite/gcc.dg/pr25521.c +++ b/gcc/testsuite/gcc.dg/pr25521.c @@ -2,9 +2,10 @@ sections. { dg-require-effective-target elf } - { dg-do compile } */ + { dg-do compile } + { dg-skip-if "" { ! const_volatile_readonly_section } } */ const volatile int foo = 30; - -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index c0694af2338..91aafd89909 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { return 1 } + +# returns 1 if target does selects a readonly section for const volatile variables. +proc check_effective_target_const_volatile_readonly_section { } { + + if { [istarget powerpc-*-*] + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { + return 0 + } + return 1 +} [-- Attachment #3: Type: text/plain, Size: 1762 bytes --] Jeff Law writes: > On 12/7/22 08:45, Cupertino Miranda wrote: >> >>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>> This commit is a follow up of bugzilla #107181. >>>> The commit /a0aafbc/ changed the default implementation of the >>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>> placement of `const volatile' objects. >>>> However, the following targets use target-specific selection functions >>>> and they choke on the testcase pr25521.c: >>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>> That's presumably a constant section. We should instead twiddle the test to >>> recognize that section. >> Although @progbits is indeed a constant section, I believe it is >> more interesting to detect if the `rx' starts selecting more >> standard sections instead of the current @progbits. >> That was the reason why I opted to XFAIL instead of PASSing it. >> Can I keep it as such ? > I'm not aware of any ongoing development for that port, so I would not let > concerns about the rx port changing behavior dominate how we approach this > problem. > > The rx port is using a different name for the section. That's valid thing to > do and to the extent we can, we should support that in the test rather than > (incorrectly IMHO) xfailing the test just becuase the name isn't what we > expected. > > To avoid over-eagerly matching, I would probably search for "C," I wouldn't do > that for the const or rodata sections as they often have a suffix like 1, 2, 4, > 8 for different sized rodata sections. > > PPC32 is explicitly doing something different and placing those objects into an > RW section. So for PPC32 it makes more sense to skip the test rather than xfail > it. > > Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PING] Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-01-24 12:24 ` Cupertino Miranda @ 2023-01-31 9:10 ` Cupertino Miranda 2023-02-07 9:53 ` Cupertino Miranda 2023-03-11 16:25 ` Jeff Law 1 sibling, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-01-31 9:10 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Jeff Law, jose.marchesi, gcc-patches Cupertino Miranda via Gcc-patches writes: > Thank you for the comments and suggestions. > I have changed the patch. > > Unfortunately in case of rx target I could not make > scan-assembler-symbol-section to match. I believe it is because the > .section and .global entries order is reversed in this target. > > Patch in inlined below. looking forward to your comments. > > Cupertino > > diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c > index 63363a03b9f..82b4cd88ec0 100644 > --- a/gcc/testsuite/gcc.dg/pr25521.c > +++ b/gcc/testsuite/gcc.dg/pr25521.c > @@ -2,9 +2,10 @@ > sections. > > { dg-require-effective-target elf } > - { dg-do compile } */ > + { dg-do compile } > + { dg-skip-if "" { ! const_volatile_readonly_section } } */ > > const volatile int foo = 30; > > - > -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ > +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ > +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index c0694af2338..91aafd89909 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { > > return 1 > } > + > +# returns 1 if target does selects a readonly section for const volatile variables. > +proc check_effective_target_const_volatile_readonly_section { } { > + > + if { [istarget powerpc-*-*] > + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { > + return 0 > + } > + return 1 > +} > > > Jeff Law writes: > >> On 12/7/22 08:45, Cupertino Miranda wrote: >>> >>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>> This commit is a follow up of bugzilla #107181. >>>>> The commit /a0aafbc/ changed the default implementation of the >>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>> placement of `const volatile' objects. >>>>> However, the following targets use target-specific selection functions >>>>> and they choke on the testcase pr25521.c: >>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>> That's presumably a constant section. We should instead twiddle the test to >>>> recognize that section. >>> Although @progbits is indeed a constant section, I believe it is >>> more interesting to detect if the `rx' starts selecting more >>> standard sections instead of the current @progbits. >>> That was the reason why I opted to XFAIL instead of PASSing it. >>> Can I keep it as such ? >> I'm not aware of any ongoing development for that port, so I would not let >> concerns about the rx port changing behavior dominate how we approach this >> problem. >> >> The rx port is using a different name for the section. That's valid thing to >> do and to the extent we can, we should support that in the test rather than >> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >> expected. >> >> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >> 8 for different sized rodata sections. >> >> PPC32 is explicitly doing something different and placing those objects into an >> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >> it. >> >> Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PING] Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-01-31 9:10 ` [PING] " Cupertino Miranda @ 2023-02-07 9:53 ` Cupertino Miranda 2023-02-17 14:33 ` Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-02-07 9:53 UTC (permalink / raw) To: Jeff Law; +Cc: Cupertino Miranda, jose.marchesi, gcc-patches Hi Jeff, Can you please confirm if the patch is Ok? Thanks, Cupertino > Cupertino Miranda via Gcc-patches writes: > >> Thank you for the comments and suggestions. >> I have changed the patch. >> >> Unfortunately in case of rx target I could not make >> scan-assembler-symbol-section to match. I believe it is because the >> .section and .global entries order is reversed in this target. >> >> Patch in inlined below. looking forward to your comments. >> >> Cupertino >> >> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >> index 63363a03b9f..82b4cd88ec0 100644 >> --- a/gcc/testsuite/gcc.dg/pr25521.c >> +++ b/gcc/testsuite/gcc.dg/pr25521.c >> @@ -2,9 +2,10 @@ >> sections. >> >> { dg-require-effective-target elf } >> - { dg-do compile } */ >> + { dg-do compile } >> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >> >> const volatile int foo = 30; >> >> - >> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >> index c0694af2338..91aafd89909 100644 >> --- a/gcc/testsuite/lib/target-supports.exp >> +++ b/gcc/testsuite/lib/target-supports.exp >> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >> >> return 1 >> } >> + >> +# returns 1 if target does selects a readonly section for const volatile variables. >> +proc check_effective_target_const_volatile_readonly_section { } { >> + >> + if { [istarget powerpc-*-*] >> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >> + return 0 >> + } >> + return 1 >> +} >> >> >> Jeff Law writes: >> >>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>> >>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>> This commit is a follow up of bugzilla #107181. >>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>> placement of `const volatile' objects. >>>>>> However, the following targets use target-specific selection functions >>>>>> and they choke on the testcase pr25521.c: >>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>> That's presumably a constant section. We should instead twiddle the test to >>>>> recognize that section. >>>> Although @progbits is indeed a constant section, I believe it is >>>> more interesting to detect if the `rx' starts selecting more >>>> standard sections instead of the current @progbits. >>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>> Can I keep it as such ? >>> I'm not aware of any ongoing development for that port, so I would not let >>> concerns about the rx port changing behavior dominate how we approach this >>> problem. >>> >>> The rx port is using a different name for the section. That's valid thing to >>> do and to the extent we can, we should support that in the test rather than >>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>> expected. >>> >>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>> 8 for different sized rodata sections. >>> >>> PPC32 is explicitly doing something different and placing those objects into an >>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>> it. >>> >>> Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PING] Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-02-07 9:53 ` Cupertino Miranda @ 2023-02-17 14:33 ` Cupertino Miranda 2023-02-27 10:17 ` Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-02-17 14:33 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Jeff Law, Cupertino Miranda, jose.marchesi, gcc-patches PING !!!!! Cupertino Miranda via Gcc-patches writes: > Hi Jeff, > > Can you please confirm if the patch is Ok? > > Thanks, > Cupertino > >> Cupertino Miranda via Gcc-patches writes: >> >>> Thank you for the comments and suggestions. >>> I have changed the patch. >>> >>> Unfortunately in case of rx target I could not make >>> scan-assembler-symbol-section to match. I believe it is because the >>> .section and .global entries order is reversed in this target. >>> >>> Patch in inlined below. looking forward to your comments. >>> >>> Cupertino >>> >>> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >>> index 63363a03b9f..82b4cd88ec0 100644 >>> --- a/gcc/testsuite/gcc.dg/pr25521.c >>> +++ b/gcc/testsuite/gcc.dg/pr25521.c >>> @@ -2,9 +2,10 @@ >>> sections. >>> >>> { dg-require-effective-target elf } >>> - { dg-do compile } */ >>> + { dg-do compile } >>> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >>> >>> const volatile int foo = 30; >>> >>> - >>> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >>> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >>> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >>> index c0694af2338..91aafd89909 100644 >>> --- a/gcc/testsuite/lib/target-supports.exp >>> +++ b/gcc/testsuite/lib/target-supports.exp >>> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >>> >>> return 1 >>> } >>> + >>> +# returns 1 if target does selects a readonly section for const volatile variables. >>> +proc check_effective_target_const_volatile_readonly_section { } { >>> + >>> + if { [istarget powerpc-*-*] >>> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >>> + return 0 >>> + } >>> + return 1 >>> +} >>> >>> >>> Jeff Law writes: >>> >>>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>>> >>>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>>> This commit is a follow up of bugzilla #107181. >>>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>>> placement of `const volatile' objects. >>>>>>> However, the following targets use target-specific selection functions >>>>>>> and they choke on the testcase pr25521.c: >>>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>>> That's presumably a constant section. We should instead twiddle the test to >>>>>> recognize that section. >>>>> Although @progbits is indeed a constant section, I believe it is >>>>> more interesting to detect if the `rx' starts selecting more >>>>> standard sections instead of the current @progbits. >>>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>>> Can I keep it as such ? >>>> I'm not aware of any ongoing development for that port, so I would not let >>>> concerns about the rx port changing behavior dominate how we approach this >>>> problem. >>>> >>>> The rx port is using a different name for the section. That's valid thing to >>>> do and to the extent we can, we should support that in the test rather than >>>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>>> expected. >>>> >>>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>>> 8 for different sized rodata sections. >>>> >>>> PPC32 is explicitly doing something different and placing those objects into an >>>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>>> it. >>>> >>>> Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PING] Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-02-17 14:33 ` Cupertino Miranda @ 2023-02-27 10:17 ` Cupertino Miranda 2023-03-09 9:51 ` [PING, PING] " Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-02-27 10:17 UTC (permalink / raw) To: Jeff Law; +Cc: Cupertino Miranda, jose.marchesi, gcc-patches Hi Jeff, Please, please, give me some feedback on this one. I just don't want to have to keep asking you for time on this small pending patches that I also have to keep track on. I realized your committed the other one. Thank you ! Best regards, Cupertino Cupertino Miranda writes: > PING !!!!! > > Cupertino Miranda via Gcc-patches writes: > >> Hi Jeff, >> >> Can you please confirm if the patch is Ok? >> >> Thanks, >> Cupertino >> >>> Cupertino Miranda via Gcc-patches writes: >>> >>>> Thank you for the comments and suggestions. >>>> I have changed the patch. >>>> >>>> Unfortunately in case of rx target I could not make >>>> scan-assembler-symbol-section to match. I believe it is because the >>>> .section and .global entries order is reversed in this target. >>>> >>>> Patch in inlined below. looking forward to your comments. >>>> >>>> Cupertino >>>> >>>> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >>>> index 63363a03b9f..82b4cd88ec0 100644 >>>> --- a/gcc/testsuite/gcc.dg/pr25521.c >>>> +++ b/gcc/testsuite/gcc.dg/pr25521.c >>>> @@ -2,9 +2,10 @@ >>>> sections. >>>> >>>> { dg-require-effective-target elf } >>>> - { dg-do compile } */ >>>> + { dg-do compile } >>>> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >>>> >>>> const volatile int foo = 30; >>>> >>>> - >>>> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >>>> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >>>> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >>>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >>>> index c0694af2338..91aafd89909 100644 >>>> --- a/gcc/testsuite/lib/target-supports.exp >>>> +++ b/gcc/testsuite/lib/target-supports.exp >>>> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >>>> >>>> return 1 >>>> } >>>> + >>>> +# returns 1 if target does selects a readonly section for const volatile variables. >>>> +proc check_effective_target_const_volatile_readonly_section { } { >>>> + >>>> + if { [istarget powerpc-*-*] >>>> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >>>> + return 0 >>>> + } >>>> + return 1 >>>> +} >>>> >>>> >>>> Jeff Law writes: >>>> >>>>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>>>> >>>>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>>>> This commit is a follow up of bugzilla #107181. >>>>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>>>> placement of `const volatile' objects. >>>>>>>> However, the following targets use target-specific selection functions >>>>>>>> and they choke on the testcase pr25521.c: >>>>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>>>> That's presumably a constant section. We should instead twiddle the test to >>>>>>> recognize that section. >>>>>> Although @progbits is indeed a constant section, I believe it is >>>>>> more interesting to detect if the `rx' starts selecting more >>>>>> standard sections instead of the current @progbits. >>>>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>>>> Can I keep it as such ? >>>>> I'm not aware of any ongoing development for that port, so I would not let >>>>> concerns about the rx port changing behavior dominate how we approach this >>>>> problem. >>>>> >>>>> The rx port is using a different name for the section. That's valid thing to >>>>> do and to the extent we can, we should support that in the test rather than >>>>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>>>> expected. >>>>> >>>>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>>>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>>>> 8 for different sized rodata sections. >>>>> >>>>> PPC32 is explicitly doing something different and placing those objects into an >>>>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>>>> it. >>>>> >>>>> Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PING, PING] Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-02-27 10:17 ` Cupertino Miranda @ 2023-03-09 9:51 ` Cupertino Miranda 0 siblings, 0 replies; 30+ messages in thread From: Cupertino Miranda @ 2023-03-09 9:51 UTC (permalink / raw) To: Jeff Law; +Cc: Cupertino Miranda, jose.marchesi, gcc-patches [PING] Cupertino Miranda writes: > Hi Jeff, > > Please, please, give me some feedback on this one. > I just don't want to have to keep asking you for time on this small > pending patches that I also have to keep track on. > > I realized your committed the other one. Thank you ! > > Best regards, > Cupertino > > > Cupertino Miranda writes: > >> PING !!!!! >> >> Cupertino Miranda via Gcc-patches writes: >> >>> Hi Jeff, >>> >>> Can you please confirm if the patch is Ok? >>> >>> Thanks, >>> Cupertino >>> >>>> Cupertino Miranda via Gcc-patches writes: >>>> >>>>> Thank you for the comments and suggestions. >>>>> I have changed the patch. >>>>> >>>>> Unfortunately in case of rx target I could not make >>>>> scan-assembler-symbol-section to match. I believe it is because the >>>>> .section and .global entries order is reversed in this target. >>>>> >>>>> Patch in inlined below. looking forward to your comments. >>>>> >>>>> Cupertino >>>>> >>>>> diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c >>>>> index 63363a03b9f..82b4cd88ec0 100644 >>>>> --- a/gcc/testsuite/gcc.dg/pr25521.c >>>>> +++ b/gcc/testsuite/gcc.dg/pr25521.c >>>>> @@ -2,9 +2,10 @@ >>>>> sections. >>>>> >>>>> { dg-require-effective-target elf } >>>>> - { dg-do compile } */ >>>>> + { dg-do compile } >>>>> + { dg-skip-if "" { ! const_volatile_readonly_section } } */ >>>>> >>>>> const volatile int foo = 30; >>>>> >>>>> - >>>>> -/* { dg-final { scan-assembler "\\.s\?rodata" } } */ >>>>> +/* { dg-final { scan-assembler {.section C,} { target { rx-*-* } } } } */ >>>>> +/* { dg-final { scan-assembler-symbol-section {^_?foo$} {^\.(const|s?rodata)} { target { ! "rx-*-*" } } } } */ >>>>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >>>>> index c0694af2338..91aafd89909 100644 >>>>> --- a/gcc/testsuite/lib/target-supports.exp >>>>> +++ b/gcc/testsuite/lib/target-supports.exp >>>>> @@ -12295,3 +12295,13 @@ proc check_is_prog_name_available { prog } { >>>>> >>>>> return 1 >>>>> } >>>>> + >>>>> +# returns 1 if target does selects a readonly section for const volatile variables. >>>>> +proc check_effective_target_const_volatile_readonly_section { } { >>>>> + >>>>> + if { [istarget powerpc-*-*] >>>>> + || [check-flags { "" { powerpc64-*-* } { -m32 } }] } { >>>>> + return 0 >>>>> + } >>>>> + return 1 >>>>> +} >>>>> >>>>> >>>>> Jeff Law writes: >>>>> >>>>>> On 12/7/22 08:45, Cupertino Miranda wrote: >>>>>>> >>>>>>>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote: >>>>>>>>> This commit is a follow up of bugzilla #107181. >>>>>>>>> The commit /a0aafbc/ changed the default implementation of the >>>>>>>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the >>>>>>>>> placement of `const volatile' objects. >>>>>>>>> However, the following targets use target-specific selection functions >>>>>>>>> and they choke on the testcase pr25521.c: >>>>>>>>> *rx - target sets its const variables as '.section C,"a",@progbits'. >>>>>>>> That's presumably a constant section. We should instead twiddle the test to >>>>>>>> recognize that section. >>>>>>> Although @progbits is indeed a constant section, I believe it is >>>>>>> more interesting to detect if the `rx' starts selecting more >>>>>>> standard sections instead of the current @progbits. >>>>>>> That was the reason why I opted to XFAIL instead of PASSing it. >>>>>>> Can I keep it as such ? >>>>>> I'm not aware of any ongoing development for that port, so I would not let >>>>>> concerns about the rx port changing behavior dominate how we approach this >>>>>> problem. >>>>>> >>>>>> The rx port is using a different name for the section. That's valid thing to >>>>>> do and to the extent we can, we should support that in the test rather than >>>>>> (incorrectly IMHO) xfailing the test just becuase the name isn't what we >>>>>> expected. >>>>>> >>>>>> To avoid over-eagerly matching, I would probably search for "C," I wouldn't do >>>>>> that for the const or rodata sections as they often have a suffix like 1, 2, 4, >>>>>> 8 for different sized rodata sections. >>>>>> >>>>>> PPC32 is explicitly doing something different and placing those objects into an >>>>>> RW section. So for PPC32 it makes more sense to skip the test rather than xfail >>>>>> it. >>>>>> >>>>>> Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-01-24 12:24 ` Cupertino Miranda 2023-01-31 9:10 ` [PING] " Cupertino Miranda @ 2023-03-11 16:25 ` Jeff Law 2023-03-13 17:52 ` Cupertino Miranda 1 sibling, 1 reply; 30+ messages in thread From: Jeff Law @ 2023-03-11 16:25 UTC (permalink / raw) To: Cupertino Miranda; +Cc: gcc-patches, jose.marchesi On 1/24/23 05:24, Cupertino Miranda wrote: > Thank you for the comments and suggestions. > I have changed the patch. > > Unfortunately in case of rx target I could not make > scan-assembler-symbol-section to match. I believe it is because the > .section and .global entries order is reversed in this target. > > Patch in inlined below. looking forward to your comments. Sorry for the long delays. I've installed this version. As a follow-up, can you update the documentation in doc/sourcebuild.texi to include the new check-effective-target test? Thanks, Jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-03-11 16:25 ` Jeff Law @ 2023-03-13 17:52 ` Cupertino Miranda 2023-03-13 17:57 ` Cupertino Miranda 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-03-13 17:52 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, jose.marchesi [-- Attachment #1: Type: text/plain, Size: 719 bytes --] > On 1/24/23 05:24, Cupertino Miranda wrote: >> Thank you for the comments and suggestions. >> I have changed the patch. >> Unfortunately in case of rx target I could not make >> scan-assembler-symbol-section to match. I believe it is because the >> .section and .global entries order is reversed in this target. >> Patch in inlined below. looking forward to your comments. > Sorry for the long delays. I've installed this version. > > As a follow-up, can you update the documentation in doc/sourcebuild.texi to > include the new check-effective-target test? Hi Jeff, Thank you for installing the patch. I have prepared the doc change you requested. Hopefully this is what you were expecting. Regards, Cupertino [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Added-item-entry-in-docs-for-the-new-check_effective.patch --] [-- Type: text/x-diff, Size: 896 bytes --] From ea2a0434014516c7f680d24f0e702b407be1b83e Mon Sep 17 00:00:00 2001 From: Cupertino Miranda <cupertino.miranda@oracle.com> Date: Mon, 13 Mar 2023 17:45:14 +0000 Subject: [PATCH] Added item entry in docs for the new check_effective_target. More precisely for: - check_effective_target_const_volatile_readonly_section --- gcc/doc/sourcebuild.texi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index be4318221cc..c614292f864 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -2777,6 +2777,9 @@ Target supports @option{-branch-cost=N}. @item cxa_atexit Target uses @code{__cxa_atexit}. +@item const_volatile_readonly_section +Target places const volatile variables in readonly sections. + @item default_packed @anchor{default_packed} Target has packed layout of structure members by default. -- 2.38.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-03-13 17:52 ` Cupertino Miranda @ 2023-03-13 17:57 ` Cupertino Miranda 2023-04-03 4:16 ` Jeff Law 0 siblings, 1 reply; 30+ messages in thread From: Cupertino Miranda @ 2023-03-13 17:57 UTC (permalink / raw) To: Cupertino Miranda; +Cc: Jeff Law, jose.marchesi, gcc-patches [-- Attachment #1: Type: text/plain, Size: 906 bytes --] Cupertino Miranda via Gcc-patches writes: >> On 1/24/23 05:24, Cupertino Miranda wrote: >>> Thank you for the comments and suggestions. >>> I have changed the patch. >>> Unfortunately in case of rx target I could not make >>> scan-assembler-symbol-section to match. I believe it is because the >>> .section and .global entries order is reversed in this target. >>> Patch in inlined below. looking forward to your comments. >> Sorry for the long delays. I've installed this version. >> >> As a follow-up, can you update the documentation in doc/sourcebuild.texi to >> include the new check-effective-target test? > > Hi Jeff, > > Thank you for installing the patch. > I have prepared the doc change you requested. > Hopefully this is what you were expecting. > > Regards, > Cupertino Just realized previous patch was in incorrect placement alphabetically. Please consider this one instead. Cupertino [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Added-item-entry-in-docs-for-the-new-check_effective.patch --] [-- Type: text/x-diff, Size: 855 bytes --] From 2a4f87294ad0af037b80e13d67678e52bc382c95 Mon Sep 17 00:00:00 2001 From: Cupertino Miranda <cupertino.miranda@oracle.com> Date: Mon, 13 Mar 2023 17:45:14 +0000 Subject: [PATCH] Added item entry in docs for the new check_effective_target. More precisely for: - check_effective_target_const_volatile_readonly_section --- gcc/doc/sourcebuild.texi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index be4318221cc..f08893aeb02 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -2774,6 +2774,9 @@ Target supports automatic stack alignment. @item branch_cost Target supports @option{-branch-cost=N}. +@item const_volatile_readonly_section +Target places const volatile variables in readonly sections. + @item cxa_atexit Target uses @code{__cxa_atexit}. -- 2.38.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] Corrected pr25521.c target matching. 2023-03-13 17:57 ` Cupertino Miranda @ 2023-04-03 4:16 ` Jeff Law 0 siblings, 0 replies; 30+ messages in thread From: Jeff Law @ 2023-04-03 4:16 UTC (permalink / raw) To: Cupertino Miranda; +Cc: jose.marchesi, gcc-patches On 3/13/23 11:57, Cupertino Miranda wrote: > > Cupertino Miranda via Gcc-patches writes: > >>> On 1/24/23 05:24, Cupertino Miranda wrote: >>>> Thank you for the comments and suggestions. >>>> I have changed the patch. >>>> Unfortunately in case of rx target I could not make >>>> scan-assembler-symbol-section to match. I believe it is because the >>>> .section and .global entries order is reversed in this target. >>>> Patch in inlined below. looking forward to your comments. >>> Sorry for the long delays. I've installed this version. >>> >>> As a follow-up, can you update the documentation in doc/sourcebuild.texi to >>> include the new check-effective-target test? >> >> Hi Jeff, >> >> Thank you for installing the patch. >> I have prepared the doc change you requested. >> Hopefully this is what you were expecting. >> >> Regards, >> Cupertino > > Just realized previous patch was in incorrect placement alphabetically. > Please consider this one instead. Installed. Thanks. jeff ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2023-04-03 4:16 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-02 17:52 [PATCH] `const volatile' sections selection - bugzilla #107181 Cupertino Miranda 2022-12-02 17:52 ` [PATCH 1/2] select .rodata for const volatile variables Cupertino Miranda 2022-12-05 18:06 ` Jeff Law 2022-12-07 15:12 ` Cupertino Miranda 2022-12-15 10:13 ` Cupertino Miranda 2022-12-22 17:21 ` [PING] " Cupertino Miranda 2023-01-02 10:42 ` Cupertino Miranda 2023-01-09 7:57 ` Richard Biener 2023-01-13 15:06 ` Cupertino Miranda 2023-01-19 9:59 ` Cupertino Miranda 2023-01-22 18:43 ` Jeff Law 2023-01-22 18:49 ` Jeff Law 2022-12-02 17:52 ` [PATCH 2/2] Corrected pr25521.c target matching Cupertino Miranda 2022-12-05 18:47 ` Jeff Law 2022-12-07 15:45 ` Cupertino Miranda 2022-12-15 10:14 ` Cupertino Miranda 2022-12-22 17:22 ` [PING] " Cupertino Miranda 2023-01-02 10:43 ` Cupertino Miranda 2023-01-13 15:13 ` Cupertino Miranda 2023-01-22 19:04 ` Jeff Law 2023-01-24 12:24 ` Cupertino Miranda 2023-01-31 9:10 ` [PING] " Cupertino Miranda 2023-02-07 9:53 ` Cupertino Miranda 2023-02-17 14:33 ` Cupertino Miranda 2023-02-27 10:17 ` Cupertino Miranda 2023-03-09 9:51 ` [PING, PING] " Cupertino Miranda 2023-03-11 16:25 ` Jeff Law 2023-03-13 17:52 ` Cupertino Miranda 2023-03-13 17:57 ` Cupertino Miranda 2023-04-03 4:16 ` Jeff Law
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).