Re: [PATCH][ARM] Enable code hoisting with -Os (PR80155)
Committed to ARM/arm-9-branch
On Wed, Sep 11, 2019 at 4:50 PM Wilco Dijkstra wrote:
>
> While code hoisting generally improves codesize, it can affect performance
> negatively. Benchmarking shows it doesn't help SPEC and negatively affects
> embedded benchmarks, so only enable code hoisting with -Os on Arm.
>
> Bootstrap OK, OK for commit?
>
> ChangeLog:
> 2019-09-11 Wilco Dijkstra
>
>
> PR tree-optimization/80155
> * common/config/arm/arm-common.c (arm_option_optimization_table):
> Enable -fcode-hoisting with -Os.
>
> --
> diff --git a/gcc/common/config/arm/arm-common.c
> b/gcc/common/config/arm/arm-common.c
> index
> 41a920f6dc96833e778faa8dbcc19beac483734c..b0d5fb300bf01acc1fb6f4631635f8a1bfe6441c
> 100644
> --- a/gcc/common/config/arm/arm-common.c
> +++ b/gcc/common/config/arm/arm-common.c
> @@ -39,6 +39,8 @@ static const struct default_options
> arm_option_optimization_table[] =
> /* Enable section anchors by default at -O1 or higher. */
> { OPT_LEVELS_1_PLUS, OPT_fsection_anchors, NULL, 1 },
> { OPT_LEVELS_1_PLUS, OPT_fsched_pressure, NULL, 1 },
> +/* Enable code hoisting only with -Os. */
> +{ OPT_LEVELS_SIZE, OPT_fcode_hoisting, NULL, 1 },
> { OPT_LEVELS_NONE, 0, NULL, 0 }
>};
>
>
[committed] Create ARM/Arm-9-branch
Branch created for Arm GCC releases. - Joey
Re: [PATCH][OBVIOUS] Close file on return from verify-intermediate
Committed as r264202. gcov-8 still fails at r264226 according to https://gcc.gnu.org/ml/gcc-testresults/2018-09/msg01478.html So it is confirmed that this patch doesn't resolve PR85871, as Mike hoped. Thanks, Joey On Mon, Sep 10, 2018 at 3:04 PM Martin Liška wrote: > > On 09/05/2018 03:29 PM, Joey Ye wrote: > > This is a fix to an obvious issue in gcov.exp, where proc > > verify-intermediate returns without closing the open file. > > > > This can be a possible fix to PR85871. gcov-8.C diffs to other gcov > > testcases that it invokes verify-intermediate. Not closing an open file may > > result in random failure quietly. > > > > It is only a possible fix as I failed to reproduce the PR85871 random > > failure in my local machine despite continuous testing of multiple days. So > > I cannot verify if this patch fixes the regression either. > > > > To verify, https://gcc.gnu.org/ml/gcc-testresults/ need to be watched > > whether gcov-8 regression will disappear completely one month after this > > patch committed to trunk. > > > > Tested with make check with no new regressions. > > > > OK to trunk? > > > > testsuite/ChangeLog: > > 2018-09-05 Joey Ye > > > > * lib/gcov.exp (verify-intermediate): Add missing close. > > > > Hi. > > Thanks for the fix, it's obvious. Please install the patch. > > Note that gcov-8.C is built multiple times with different -std=* options: > > PASS: g++.dg/gcov/gcov-8.C -std=gnu++98 (test for excess errors) > PASS: g++.dg/gcov/gcov-8.C -std=gnu++98 execution test > PASS: g++.dg/gcov/gcov-8.C -std=gnu++98 gcov > PASS: g++.dg/gcov/gcov-8.C -std=gnu++11 (test for excess errors) > PASS: g++.dg/gcov/gcov-8.C -std=gnu++11 execution test > PASS: g++.dg/gcov/gcov-8.C -std=gnu++11 gcov > PASS: g++.dg/gcov/gcov-8.C -std=gnu++14 (test for excess errors) > PASS: g++.dg/gcov/gcov-8.C -std=gnu++14 execution test > PASS: g++.dg/gcov/gcov-8.C -std=gnu++14 gcov > > That can cause the collisions seen in the PR. > > Martin
[PATCH][OBVIOUS] Close file on return from verify-intermediate
This is a fix to an obvious issue in gcov.exp, where proc verify-intermediate returns without closing the open file. This can be a possible fix to PR85871. gcov-8.C diffs to other gcov testcases that it invokes verify-intermediate. Not closing an open file may result in random failure quietly. It is only a possible fix as I failed to reproduce the PR85871 random failure in my local machine despite continuous testing of multiple days. So I cannot verify if this patch fixes the regression either. To verify, https://gcc.gnu.org/ml/gcc-testresults/ need to be watched whether gcov-8 regression will disappear completely one month after this patch committed to trunk. Tested with make check with no new regressions. OK to trunk? testsuite/ChangeLog: 2018-09-05 Joey Ye * lib/gcov.exp (verify-intermediate): Add missing close. gcov-20180905.patch Description: gcov-20180905.patch
Re: [PATCH AArch64]Fix test failure for pr84682-2.c
typo: s/reorg.c/recog.c/g
On Thu, Aug 30, 2018 at 11:20 AM Joey Ye wrote:
>
> Hi Bin & Richard,
>
> It is not as simple as keeping the assertion, which still fails even
> with the change in reorg.c. The testing result is as following:
>
> I. With Bin's patch version 2 (removing the assertion in aarch64.c and
> adding the check in reorg.c): pr84682-2.c passes
>
> II. With Richard's suggestion to keep the assertion in aarch64, but
> adding the check in reorg.c: pr84682-2.c fails
>
> Apparently there is a different path that reaches the assertion.
>
> With II:
> /build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c: In
> function 'b':
> /build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c:10:1:
> internal compiler error: in aarch64_classify_address, at
> config/aarch64/aarch64.c:5721
> 0xfa4071 aarch64_classify_address
> /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:5720
> 0xfa94f3 aarch64_legitimate_address_hook_p
> /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:6003
> 0xb85661 strict_memory_address_addr_space_p(machine_mode, rtx_def*,
> unsigned char)
> /build/trunk/src/gcc/gcc/reload.c:2177
> 0xb75da9 constrain_operands(int, unsigned long)
> /build/trunk/src/gcc/gcc/recog.c:2706
> 0xb761a0 extract_constrain_insn(rtx_insn*)
> /build/trunk/src/gcc/gcc/recog.c:2210
> 0xa6dd57 check_rtl
> /build/trunk/src/gcc/gcc/lra.c:2182
> 0xa737db lra(_IO_FILE*)
> /build/trunk/src/gcc/gcc/lra.c:2616
> 0xa25989 do_reload
> /build/trunk/src/gcc/gcc/ira.c:5469
> 0xa25989 execute
>
> Current trunk without any patch:
> /build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c: In
> function 'b':
> /build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c:9:3:
> internal compiler error: in aarch64_classify_address, at
> config/aarch64/aarch64.c:5721
> 0xfa4011 aarch64_classify_address
> /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:5720
> 0xfa9493 aarch64_legitimate_address_hook_p
> /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:6003
> 0xb74cf3 memory_address_addr_space_p(machine_mode, rtx_def*, unsigned char)
> /build/trunk/src/gcc/gcc/recog.c:1334
> 0xb74cf3 address_operand(rtx_def*, machine_mode)
> /build/trunk/src/gcc/gcc/recog.c:1073
> 0xb74cf3 asm_operand_ok(rtx_def*, char const*, char const**)
> /build/trunk/src/gcc/gcc/recog.c:1817
> 0x75e591 expand_asm_stmt
> /build/trunk/src/gcc/gcc/cfgexpand.c:3135
> 0x766d67 expand_gimple_stmt_1
> /build/trunk/src/gcc/gcc/cfgexpand.c:3572
> 0x766d67 expand_gimple_stmt
> /build/trunk/src/gcc/gcc/cfgexpand.c:3734
> 0x768ce7 expand_gimple_basic_block
>
> More places need to be patched.
>
> Thanks,
> Joey
> On Thu, Aug 30, 2018 at 2:02 AM Bin.Cheng wrote:
> >
> > On Thu, Aug 30, 2018 at 2:47 AM Richard Sandiford
> > wrote:
> > >
> > > Joey Ye writes:
> > > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > > > index 07c55b1..9e965ab 100644
> > > > --- a/gcc/config/aarch64/aarch64.c
> > > > +++ b/gcc/config/aarch64/aarch64.c
> > > > @@ -5674,9 +5674,6 @@ aarch64_classify_address (struct
> > > > aarch64_address_info *info,
> > > >&& (code != POST_INC && code != REG))
> > > > return false;
> > > >
> > > > - gcc_checking_assert (GET_MODE (x) == VOIDmode
> > > > -|| SCALAR_INT_MODE_P (GET_MODE (x)));
> > > > -
> > > >switch (code)
> > > > {
> > > > case REG:
> > > > diff --git a/gcc/recog.c b/gcc/recog.c
> > > > index 0a8fa2c..510aba2 100644
> > > > --- a/gcc/recog.c
> > > > +++ b/gcc/recog.c
> > > > @@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode)
> > > > int
> > > > address_operand (rtx op, machine_mode mode)
> > > > {
> > > > + /* Wrong mode for an address expr. */
> > > > + if (GET_MODE (op) != VOIDmode
> > > > + && ! SCALAR_INT_MODE_P (GET_MODE (op)))
> > > > +return false;
> > > > +
> > > >return memory_address_p (mode, op);
> > > > }
> > > >
> > >
> > > The address_operand part is OK, thanks.
> > >
> > > I think we should keep the assert in aarch64_classify_address, since
> > > IMO it's a bug for anything else to reach that point.
> >
> > Hi Joey,
> > Could you help me update the patch as suggested by Richard and commit
> > it please? My new assignment is still on the way.
> > Thanks very much!
> >
> > Thanks,
> > bin
> > >
> > > Richard
Re: [PATCH AArch64]Fix test failure for pr84682-2.c
Hi Bin & Richard,
It is not as simple as keeping the assertion, which still fails even
with the change in reorg.c. The testing result is as following:
I. With Bin's patch version 2 (removing the assertion in aarch64.c and
adding the check in reorg.c): pr84682-2.c passes
II. With Richard's suggestion to keep the assertion in aarch64, but
adding the check in reorg.c: pr84682-2.c fails
Apparently there is a different path that reaches the assertion.
With II:
/build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c: In
function 'b':
/build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c:10:1:
internal compiler error: in aarch64_classify_address, at
config/aarch64/aarch64.c:5721
0xfa4071 aarch64_classify_address
/build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:5720
0xfa94f3 aarch64_legitimate_address_hook_p
/build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:6003
0xb85661 strict_memory_address_addr_space_p(machine_mode, rtx_def*,
unsigned char)
/build/trunk/src/gcc/gcc/reload.c:2177
0xb75da9 constrain_operands(int, unsigned long)
/build/trunk/src/gcc/gcc/recog.c:2706
0xb761a0 extract_constrain_insn(rtx_insn*)
/build/trunk/src/gcc/gcc/recog.c:2210
0xa6dd57 check_rtl
/build/trunk/src/gcc/gcc/lra.c:2182
0xa737db lra(_IO_FILE*)
/build/trunk/src/gcc/gcc/lra.c:2616
0xa25989 do_reload
/build/trunk/src/gcc/gcc/ira.c:5469
0xa25989 execute
Current trunk without any patch:
/build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c: In
function 'b': /build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c:9:3:
internal compiler error: in aarch64_classify_address, at
config/aarch64/aarch64.c:5721
0xfa4011 aarch64_classify_address
/build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:5720
0xfa9493 aarch64_legitimate_address_hook_p
/build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:6003
0xb74cf3 memory_address_addr_space_p(machine_mode, rtx_def*, unsigned char)
/build/trunk/src/gcc/gcc/recog.c:1334
0xb74cf3 address_operand(rtx_def*, machine_mode)
/build/trunk/src/gcc/gcc/recog.c:1073
0xb74cf3 asm_operand_ok(rtx_def*, char const*, char const**)
/build/trunk/src/gcc/gcc/recog.c:1817
0x75e591 expand_asm_stmt
/build/trunk/src/gcc/gcc/cfgexpand.c:3135
0x766d67 expand_gimple_stmt_1
/build/trunk/src/gcc/gcc/cfgexpand.c:3572
0x766d67 expand_gimple_stmt
/build/trunk/src/gcc/gcc/cfgexpand.c:3734
0x768ce7 expand_gimple_basic_block
More places need to be patched.
Thanks,
Joey
On Thu, Aug 30, 2018 at 2:02 AM Bin.Cheng wrote:
>
> On Thu, Aug 30, 2018 at 2:47 AM Richard Sandiford
> wrote:
> >
> > Joey Ye writes:
> > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > > index 07c55b1..9e965ab 100644
> > > --- a/gcc/config/aarch64/aarch64.c
> > > +++ b/gcc/config/aarch64/aarch64.c
> > > @@ -5674,9 +5674,6 @@ aarch64_classify_address (struct
> > > aarch64_address_info *info,
> > >&& (code != POST_INC && code != REG))
> > > return false;
> > >
> > > - gcc_checking_assert (GET_MODE (x) == VOIDmode
> > > -|| SCALAR_INT_MODE_P (GET_MODE (x)));
> > > -
> > >switch (code)
> > > {
> > > case REG:
> > > diff --git a/gcc/recog.c b/gcc/recog.c
> > > index 0a8fa2c..510aba2 100644
> > > --- a/gcc/recog.c
> > > +++ b/gcc/recog.c
> > > @@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode)
> > > int
> > > address_operand (rtx op, machine_mode mode)
> > > {
> > > + /* Wrong mode for an address expr. */
> > > + if (GET_MODE (op) != VOIDmode
> > > + && ! SCALAR_INT_MODE_P (GET_MODE (op)))
> > > +return false;
> > > +
> > >return memory_address_p (mode, op);
> > > }
> > >
> >
> > The address_operand part is OK, thanks.
> >
> > I think we should keep the assert in aarch64_classify_address, since
> > IMO it's a bug for anything else to reach that point.
>
> Hi Joey,
> Could you help me update the patch as suggested by Richard and commit
> it please? My new assignment is still on the way.
> Thanks very much!
>
> Thanks,
> bin
> >
> > Richard
Re: [PATCH AArch64]Fix test failure for pr84682-2.c
Ping^2 for Bin
The ICE is still there annoyingly.
Thanks,
Joey
On Wed, May 16, 2018 at 9:21 AM Kyrill Tkachov
wrote:
>
> Hi Bin,
>
>
> On 22/03/18 11:07, Bin.Cheng wrote:
> > On Sat, Mar 17, 2018 at 8:54 AM, Richard Sandiford
> > wrote:
> > > Kyrill Tkachov writes:
> > >> Hi Bin,
> > >>
> > >> On 16/03/18 11:42, Bin Cheng wrote:
> > >>> Hi,
> > >>> This simple patch fixes test case failure for pr84682-2.c by returning
> > >>> false on wrong mode rtx in aarch64_classify_address, rather than assert.
> > >>>
> > >>> Bootstrap and test on aarch64. Is it OK?
> > >>>
> > >>> Thanks,
> > >>> bin
> > >>>
> > >>> 2018-03-16 Bin Cheng
> > >>>
> > >>> * config/aarch64/aarch64.c (aarch64_classify_address): Return
> > >>> false
> > >>> on wrong mode rtx, rather than assert.
> > >>
> > >> This looks ok to me in light of
> > >> https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00633.html
> > >> This function is used to validate inline asm operands too, not just
> > >> internally-generated addresses.
> > >> Therefore all kinds of garbage must be rejected gracefully rather than
> > >> ICEing.
> > >>
> > >> You'll need an approval from an AArch64 maintainer though.
> > >
> > > IMO we should make address_operand itself check something like:
> > >
> > > (GET_MODE (x) == VOIDmode || SCALAR_INT_MODE_P (GET_MODE (x)))
> > >
> > > Target-independent code fundamentally assumes that an address will not
> > > be a float, so I think the check should be in target-independent code
> > > rather than copied to each individual backend.
> > >
> > > This was only caught on aarch64 because we added the assert, but I think
> > > some backends ignore the mode of the address and so would actually accept
> > > simple float rtxes.
> > Hi Richard,
> > Thanks for the suggestion generalizing the fix. Here is the updated patch.
> > Bootstrap and test on x86_64 and AArch64, is it OK?
> >
>
> I guess you need a midend maintainer to ok this now.
> CC'ing Jeff...
>
> Thanks,
> Kyrill
>
> > Thanks,
> > bin
> >
> > 2018-03-22 Bin Cheng
> >
> > * recog.c (address_operand): Return false on wrong mode for address.
> > * config/aarch64/aarch64.c (aarch64_classify_address): Remove assert
> > since it's checked in general code now.
> >
> > >
> > > Thanks,
> > > Richard
>
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 07c55b1..9e965ab 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5674,9 +5674,6 @@ aarch64_classify_address (struct aarch64_address_info
*info,
&& (code != POST_INC && code != REG))
return false;
- gcc_checking_assert (GET_MODE (x) == VOIDmode
- || SCALAR_INT_MODE_P (GET_MODE (x)));
-
switch (code)
{
case REG:
diff --git a/gcc/recog.c b/gcc/recog.c
index 0a8fa2c..510aba2 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode)
int
address_operand (rtx op, machine_mode mode)
{
+ /* Wrong mode for an address expr. */
+ if (GET_MODE (op) != VOIDmode
+ && ! SCALAR_INT_MODE_P (GET_MODE (op)))
+return false;
+
return memory_address_p (mode, op);
}
RE: [ARM] mno-pic-data-is-text-relative & msingle-pic-base
Irfan, Thanks for the case sharing. It is a persuasive reason not to error out -mno-SPB. Nathan's patch changes default behaviour that -mSPB will be implied when -mno-PDITR is provided. So with this patch your project need to explicitly specify -mno-SPB to make it work as before. IMHO default behaviour should reflect the typical usage. Now I not so sure whether -mSPB should be typically used with -mno-PDITR. Irfan what's your opinion? Thanks, Joey > -Original Message- > From: Irfan Ahmad [mailto:h.irfanah...@gmail.com] > Sent: 05 August 2016 07:06 > To: Ramana Radhakrishnan; Nathan Sidwell; Joey Ye; GCC Patches > Cc: Richard Earnshaw > Subject: Re: [ARM] mno-pic-data-is-text-relative & msingle-pic-base > > Ramana, > > I saw some correspondence between you and Nathan on his patch > [https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html] (after I sent this > email) going in a direction that may eventually result in too tight than > necessary > coupling between these two switches, as your response hints at: > > > I am also slightly inclined to go further and error out if someone uses > > -mno- > PDITR with -mno-SPB on the command line, after all as you say -mno-PDITR > implies a non-fixed mapping while -mno-SPB implies there is some fixed mapping > some where currently in the compiler. I don't see how the twain can meet. That > can happen as a follow-up - the current patch is by itself a step improvement. > > Please see the alternative perspective as described below. > > Irfan Ahmad > [p.s. Sorry about repeat send. I accidentally sent it earlier in HTML format] > -- > - > On 05/08/2016 09:56, Irfan Ahmad wrote: > > Nathan, > > Sorry for jumping in a relatively old thread. I saw this in mailing list > archives > during a web search (I wasn't on this mailing list before). I decided to > speak up as > I would be an affected party if this patch (or some similar future patch) gets > accepted or worse yet, the feature involved gets disabled. > > > Apparently there are legitimate reasons one might want the -mno-PDITR > behaviour without -mSPB. I don't know what those are, perhaps Joey could > clarify? > > Yes, there are some practical use cases that require -mno-pic-data-is-text- > relative (-mno-PDITR) without -msingle-pic-base (-mSPB). > > These are based on two primary principles: > > 1. In the absence of lazy binding (that is almost always the case in embedded > systems), GOT is practically read-only - it needs to be modified only during > linking by the dynamic linker, after that it can be considered and marked > read- > only (e.g. read-only attribute set to be enforced by some MMU or MPU). > > 2. If you only need a simple dynamic object model - where you just need > dynamic loading and dynamic linking - but do not need to maintain multiple > data > states for the object like you need in a traditional shared object model, > then one > instance of GOT per dynamic object is enough. > > From #1: GOT is read-only so keeping it with CODE segment is a natural choice. > Now as there is no need to move it to RAM so there is no need for a mechanism > (-mSPB) that would enable controlling the GOT location independently of CODE > segment. > > From #2: Only one instance of GOT is required per dynamic object so there is > no > need for a mechanism (-mSPB) that would enable switching GOTs. > > So when both #1 and #2 are met, you only need -mno-pic-data-is-text-relative. > > Irfan Ahmad
RE: [ARM] no-data-is-text-relative & msingle-pic-base
Ramana, Nathan, My opinion was "there is nothing wrong to run application -mno-pic-data-is-text-relative without -msingle-pic-base in a system that offset of data and text it fixed. I am not convenience we should ban it." However, what Ramana is suggesting is to error out -mno-PDITR with explicit -mno-SPB, which I don't have a strong preference embrace it or not as it will be just a rare and wired command line combination. Thanks, Joey > -Original Message- > From: Nathan Sidwell [mailto:nathanmsidw...@gmail.com] On Behalf Of > Nathan Sidwell > Sent: 12 July 2016 17:07 > To: Ramana Radhakrishnan > Cc: GCC Patches; Joey Ye; nd > Subject: Re: [ARM] no-data-is-text-relative & msingle-pic-base > > On 07/12/16 12:01, Ramana Radhakrishnan wrote: > > > I am also slightly inclined to go further and error out if someone uses - > mno-PDITR with -mno-SPB on the command line, after all as you say -mno- > PDITR implies a non-fixed mapping while -mno-SPB implies there is some > fixed mapping some where currently in the compiler. I don't see how the > twain can meet. > > That was my original thought too, but Joey told me that such use cases > exist. > > nathan
RE: [ARM] mno-pic-data-is-text-relative & msingle-pic-base
> -Original Message- > From: Nathan Sidwell [mailto:nathanmsidw...@gmail.com] On Behalf Of > Nathan Sidwell > Sent: 09 May 2016 18:22 > To: Joey Ye; Richard Earnshaw; GCC Patches > Subject: Re: [ARM] mno-pic-data-is-text-relative & msingle-pic-base > > Joey, > > This patch will do what you intend it to do. However, I am not sure in part > related to VxWorks. The logic behind this patch is that -mno-pic-data-is- > text-relative should enable -msingle-pic-base because otherwise it will be > useless. The logic itself is orthogonal to OS. So I am not convinced the 'else > if' shouldn't be just 'if'. It should not change VxWorks behaviour if VxWorks > enables -msingle-pic-base explicitly. Or otherwise there is at least one use > case that -mno-pic-data-is-text-relative can be used without -msingle-pic- > base, which breaks the logic that this whole patch stands on. > > VxWorks has two modes of code generation -- kernel and RTP. RTPs don't > have a fixed mapping between code and data (and use special sequence to > initialize the PIC register, using vxworks-specific relocs). Kernel mode > doesn't support PIC code generation -- see config/vxworks.c > > So I don't think there's a problem. No other commit. OK for me though I cannot approve it. - Joey
RE: [ARM] mno-pic-data-is-text-relative & msingle-pic-base
Nathan, This patch will do what you intend it to do. However, I am not sure in part related to VxWorks. The logic behind this patch is that -mno-pic-data-is-text-relative should enable -msingle-pic-base because otherwise it will be useless. The logic itself is orthogonal to OS. So I am not convinced the 'else if' shouldn't be just 'if'. It should not change VxWorks behaviour if VxWorks enables -msingle-pic-base explicitly. Or otherwise there is at least one use case that -mno-pic-data-is-text-relative can be used without -msingle-pic-base, which breaks the logic that this whole patch stands on. Thanks, Joey > -Original Message- > From: Nathan Sidwell [mailto:nathanmsidw...@gmail.com] On Behalf Of > Nathan Sidwell > Sent: 09 May 2016 15:07 > To: Richard Earnshaw; GCC Patches > Cc: Joey Ye > Subject: [ARM] mno-pic-data-is-text-relative & msingle-pic-base > > This patch comes from an off-list conversation between Joey & me. The > context is with RTOSs not all singing & dancing dynamic objects and OSes. > > currently, the documentation for -mno-pic-data-is-text-relative (-mno-PDITR) > says 'Assume that each data segments are relative to text segment at load > time. > Therefore, it permits addressing data using PC-relative operations. > This option is on by default for targets other than VxWorks RTP.' > > However, if you use just this option, you still end up with a pic-register > init > sequence that presumes a fixed mapping. That's a surprise. Joey tells me > its expected use is with -msingle-pic-base (-mSPB), which reserves a global > register to point at the (single) GOT. That's what I had expected the -mno- > PDITR option to have implied. > > Apparently there are legitimate reasons one might want the -mno-PDITR > behaviour without -mSPB. I don't know what those are, perhaps Joey could > clarify? > > Anyway, IMHO that is the rare case and the more common case is that one > would want to have -mnoPDITR imply -mSPB. (The reverse probably doesn't > apply.) > > This patch does 3 things. > 1) have -mno-PDITR imply -mSPB, unless one has explictly provided -m[no- > ]SPB. > 2) clarified the -m[no-]PDITR documentation. > 3) Added some testcases -- there didn't appear to be any. > > ok? > > nathan
[patch] [testsuite, arm] Missing test case for thumb2 pop single
Find a missing test case for
https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00789.html left at the corner.
Merged with the latest trunk. New test case does not fail on
thumb1/thumb2/arm targets.
ChangeLog:
2015-07-24: Joey Ye joey...@arm.com
* gcc.target/arm/thumb2-pop-single.c: New test.
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-pop-single.c
b/gcc/testsuite/gcc.target/arm/thumb2-pop-single.c
new file mode 100644
index 000..f86c633
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/thumb2-pop-single.c
@@ -0,0 +1,14 @@
+/* Verify if thumb2 save/restore lr unnecessarily in case of tail call. */
+/* Verify if thumb2 generates pop to restore a single register. */
+/* { dg-do compile { target arm_thumb2 } } */
+/* { dg-options -Os } */
+/* { dg-final { scan-assembler-not push.*lr } } */
+/* { dg-final { scan-assembler pop\[\\t \]+\{r\[4-7\]\} } } */
+extern int
+bar (int, int, int, int);
+
+int
+foo (int a, int b, int c, int d)
+{
+ return bar (b, a, c, d);
+}
Re: ldm/stm bus error
In this case ldm is loading at alignment address. It is just loaded
more than sizeof a. So it can be the bus that does not permit
accessing memory beyond address range of a. Such a case I don't
believe compiler is doing wrong.
On Mon, May 18, 2015 at 4:50 PM, Richard Earnshaw
richard.earns...@foss.arm.com wrote:
On 18/05/15 10:05, Umesh Kalappa wrote:
Hi All,
Getting a bus/hard error for the below case ,make sense since ldm/stm
expects the address to be word aligned .
bash-4.1$ cat test.c
struct test
{
char c;
int i;
} __attribute__((packed));
struct test a,b;
int main()
{
a =b ; //here compiler is not sure that a or b is word aligned
return a.i;
}
bash-4.1$ arm-eabi-gcc -v
Using built-in specs.
COLLECT_GCC=arm-eabi-gcc
COLLECT_LTO_WRAPPER=/nobackup/ukalappa/build/gcc/mv-ga/c4.7.0-p1/x86_64-linux/libexec/gcc/arm-eabi/4.7.0/lto-wrapper
Target: arm-eabi
Configured with: /nobackup/ukalappa/src/gcc/mv-ga/gcc/configure
--srcdir=/nobackup/ukalappa/src/gcc/mv-ga/gcc --build=x86_64-linux
--target=arm-eabi --host=x86_64-linux
--prefix=/nobackup/ukalappa/build/gcc/mv-ga/c4.7.0-p1
--exec-prefix=/nobackup/ukalappa/build/gcc/mv-ga/c4.7.0-p1/x86_64-linux
--with-pkgversion='Cisco GCC c4.7.0-p1' --with-cisco-patch-level=1
--with-cisco-patch-level-minor=0
--with-bugurl=http://wwwin.cisco.com/it/services/
--disable-maintainer-mode --enable-languages=c,c++ --disable-nls
Thread model: single
gcc version 4.7.0
bash-4.1$ ./arm-eabi-gcc -march=armv7 -mthumb -S test.c
bash-4.1$ cat test.s
.syntax unified
.arch armv7
.fpu softvfp
.eabi_attribute 20, 1
.eabi_attribute 21, 1
.eabi_attribute 23, 3
.eabi_attribute 24, 1
.eabi_attribute 25, 1
.eabi_attribute 26, 1
.eabi_attribute 30, 6
.eabi_attribute 34, 1
.eabi_attribute 18, 4
.thumb
.file test.c
.comm a,5,4
.comm b,5,4
The above two lines create (common) instances of a and b that are 4-byte
aligned, so the LDM should not be faulting in this case, unless your
binutils have ignored the alignment constraints.
I don't think the compiler has done the wrong thing here.
R.
.text
.align 2
.global main
.thumb
.thumb_func
.type main, %function
main:
@ args = 0, pretend = 0, frame = 0
@ frame_needed = 1, uses_anonymous_args = 0
@ link register save eliminated.
push{r7}
add r7, sp, #0
movwr3, #:lower16:a
movtr3, #:upper16:a
movwr2, #:lower16:b
movtr2, #:upper16:b
ldmia r2, {r0, r1} //Bus error
str r0, [r3]
addsr3, r3, #4
strbr1, [r3]
movwr3, #:lower16:a
movtr3, #:upper16:a
ldr r3, [r3, #1]@ unaligned
mov r0, r3
mov sp, r7
pop {r7}
bx lr
.size main, .-main
Arm states that ldm/stm should be word aligned and generating ldm/stm
in the above case is the compiler error/bug ,do you guys agree with me
or i'm missing something here ?
Thank you
~Umesh
Re: [patch, arm] Minor optimization on thumb2 tail call
Ping
On Wed, Nov 19, 2014 at 10:43 AM, Joey Ye joey...@arm.com wrote:
Current thumb2 -Os generates suboptimal code for following tail call case:
int f4(int b, int a, int c, int d);
int g(int a, int b, int c, int d)
{ return f4(b, a, c, d); }
arm-none-eabi-gcc -Os -mthumb -mcpu=cortex-m3 test.c
push
{r4, lr}
mov r4, r1
mov r1, r0
mov r0, r4
pop {r4, lr}
b f4
There are two issues: The first one is that saving/restoring lr is not
necessary, as there is no return via pop pc. The second one is that even if
we managed to avoid lr push/pop, ldmia.w sp!, {r4} is still emitted as there
is a missing pattern for pop single and code size is not optimal.
This patch fixes these two issues and introduces a shared test case. CSiBE
thumb2 -Os shows cross board code size reduction, except for one case with 4
bytes regression. The case is like:
void f ()
{
if ()
...
else if ()
...
else g();
}
There are N=2 non-sibcall returns and S=1 sibcall return. Originally the
non-sibcall returns are just pop {r4, r5, pc}, now they become
b.n .Lreturn
.Lreturn:
pop {r4, r5}
bx lr
The one byte save from sibcall return does not win the non-sibcall return
regressions back. In general scenario, number of N non-sibcall returns use
b.n branching to merged tail, number of S sibcalls save 2 bytes by avoid
poping lr. It results in 4-2*S bytes regression. In the worst scenario, each
non-sibcall return has to use b.w branching to merged tail, resulting in
(N-S)*2 bytes regression. The worst scenario is rare, according to CSiBE.
The general regression scenario can only regress 2 bytes at most. So I would
not introduce additional complexity to handle the regression case.
Make check cortex-m3: pass
thumb2 bootstrap (O2/Os): pass
* config/arm/arm.c (arm_compute_save_reg_mask):
Do not save lr in case of tail call.
* config/arm/thumb2.md (*thumb2_pop_single): New pattern.
* gcc.target/arm/thumb2-pop-single.c: New test.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 4f04707..20d0b9e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19190,6 +19190,7 @@ arm_compute_save_reg_mask (void)
|| (save_reg_mask
optimize_size
ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL
+ !crtl-tail_call_emit
!crtl-calls_eh_return))
save_reg_mask |= 1 LR_REGNUM;
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 64acfea..29cfb17 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -267,6 +267,17 @@
(set_attr type multiple)]
)
+;; Pop a single register as its size is preferred over a post-incremental
load
+(define_insn *thumb2_pop_single
+ [(set (match_operand:SI 0 low_register_operand =r)
+(mem:SI (post_inc:SI (reg:SI SP_REGNUM]
+ TARGET_THUMB2 (reload_in_progress || reload_completed)
+ pop\t{%0}
+ [(set_attr type load1)
+ (set_attr length 2)
+ (set_attr predicable yes)]
+)
+
;; We have two alternatives here for memory loads (and similarly for
stores)
;; to reflect the fact that the permissible constant pool ranges differ
;; between ldr instructions taking low regs and ldr instructions taking
high
Re: [PATCH, ARM] Fix PR63718, Thumb1 bootstrap -- disable fuse-caller-save for Thumb1
This work around brings thumb1 bootstrap back. Though I cannot
approve, I would like it in stage 3. A complete fix in thumb1 pattern
will be welcomed.
Thanks,
Joey
On Thu, Nov 20, 2014 at 7:54 PM, Tom de Vries tom_devr...@mentor.com wrote:
Richard,
This patch fixes PR63718, which currently breaks Thumb1 bootstrap.
The problem is that in Thumb1 mode, we emit the epilogue in RTL, but the
last insn - epilogue_insns - does not accurately model the corresponding
insns
emitted in the asm file. F.i., the asm file may contain an insn:
...
pop {r0}
while the corresponding RTL pattern looks like this:
...
(jump_insn (unspec_volatile [
(return)
] VUNSPEC_EPILOGUE))
...
As a consequence, the epilogue may clobber registers without
fuse-caller-save being able to analyze that.
Adding the missing clobbers to epilogue_insns is not trivial, and probably
not a good idea for stage3. The patch works around the problem by disabling
fuse-caller-save in Thumb1 mode.
Build and reg-tested on arm-none-eabi.
OK for stage3?
Thanks,
- Tom
Re: [PATCH, ARM, libgcc] New aeabi_idiv function for armv6-m
OK applying to arm/embedded-4_9-branch, though you still need
maintainer approval into trunk.
- Joey
On Wed, Nov 26, 2014 at 11:43 AM, Hale Wang hale.w...@arm.com wrote:
Hi,
This patch ports the aeabi_idiv routine from Linaro Cortex-Strings
(https://git.linaro.org/toolchain/cortex-strings.git), which was contributed
by ARM under Free BSD license.
The new aeabi_idiv routine is used to replace the one in
libgcc/config/arm/lib1funcs.S. This replacement happens within the Thumb1
wrapper. The new routine is under LGPLv3 license.
The main advantage of this version is that it can improve the performance of
the aeabi_idiv function for Thumb1. This solution will also increase the
code size. So it will only be used if __OPTIMIZE_SIZE__ is not defined.
Make check passed for armv6-m.
OK for trunk?
Thanks,
Hale Wang
libgcc/ChangeLog:
2014-11-26 Hale Wang hale.w...@arm.com
* config/arm/lib1funcs.S: Add new wrapper.
===
diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index b617137..de66c81 100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -306,34 +306,12 @@ LSYM(Lend_fde):
#ifdef __ARM_EABI__
.macro THUMB_LDIV0 name signed
#if defined(__ARM_ARCH_6M__)
- .ifc \signed, unsigned
- cmp r0, #0
- beq 1f
- mov r0, #0
- mvn r0, r0 @ 0x
-1:
- .else
- cmp r0, #0
- beq 2f
- blt 3f
+
+ push{r0, lr}
mov r0, #0
- mvn r0, r0
- lsr r0, r0, #1 @ 0x7fff
- b 2f
-3: mov r0, #0x80
- lsl r0, r0, #24 @ 0x8000
-2:
- .endif
- push{r0, r1, r2}
- ldr r0, 4f
- adr r1, 4f
- add r0, r1
- str r0, [sp, #8]
- @ We know we are not on armv4t, so pop pc is safe.
- pop {r0, r1, pc}
- .align 2
-4:
- .word __aeabi_idiv0 - 4b
+ bl SYM(__aeabi_idiv0)
+ pop {r1, pc}
+
#elif defined(__thumb2__)
.syntax unified
.ifc \signed, unsigned
@@ -927,7 +905,158 @@ LSYM(Lover7):
add dividend, work
.endif
LSYM(Lgot_result):
-.endm
+.endm
+
+#if defined(__prefer_thumb__) !defined(__OPTIMIZE_SIZE__)
+.macro BranchToDiv n, label
+ lsr curbit, dividend, \n
+ cmp curbit, divisor
+ blo \label
+.endm
+
+.macro DoDiv n
+ lsr curbit, dividend, \n
+ cmp curbit, divisor
+ bcc 1f
+ lsl curbit, divisor, \n
+ sub dividend, dividend, curbit
+
+1: adc result, result
+.endm
+
+.macro THUMB1_Div_Positive
+ mov result, #0
+ BranchToDiv #1, LSYM(Lthumb1_div1)
+ BranchToDiv #4, LSYM(Lthumb1_div4)
+ BranchToDiv #8, LSYM(Lthumb1_div8)
+ BranchToDiv #12, LSYM(Lthumb1_div12)
+ BranchToDiv #16, LSYM(Lthumb1_div16)
+LSYM(Lthumb1_div_large_positive):
+ mov result, #0xff
+ lsl divisor, divisor, #8
+ rev result, result
+ lsr curbit, dividend, #16
+ cmp curbit, divisor
+ blo 1f
+ asr result, #8
+ lsl divisor, divisor, #8
+ beq LSYM(Ldivbyzero_waypoint)
+
+1: lsr curbit, dividend, #12
+ cmp curbit, divisor
+ blo LSYM(Lthumb1_div12)
+ b LSYM(Lthumb1_div16)
+LSYM(Lthumb1_div_loop):
+ lsr divisor, divisor, #8
+LSYM(Lthumb1_div16):
+ Dodiv #15
+ Dodiv #14
+ Dodiv #13
+ Dodiv #12
+LSYM(Lthumb1_div12):
+ Dodiv #11
+ Dodiv #10
+ Dodiv #9
+ Dodiv #8
+ bcs LSYM(Lthumb1_div_loop)
+LSYM(Lthumb1_div8):
+ Dodiv #7
+ Dodiv #6
+ Dodiv #5
+LSYM(Lthumb1_div5):
+ Dodiv #4
+LSYM(Lthumb1_div4):
+ Dodiv #3
+LSYM(Lthumb1_div3):
+ Dodiv #2
+LSYM(Lthumb1_div2):
+ Dodiv #1
+LSYM(Lthumb1_div1):
+ sub divisor, dividend, divisor
+ bcs 1f
+ cpy divisor, dividend
+
+1: adc result, result
+ cpy dividend, result
+ RET
+
+LSYM(Ldivbyzero_waypoint):
+ b LSYM(Ldiv0)
+.endm
+
+.macro THUMB1_Div_Negative
+ lsr result, divisor, #31
+ beq 1f
+ neg divisor, divisor
+
+1: asr curbit, dividend, #32
+ bcc 2f
+ neg dividend, dividend
+
+2: eor curbit, result
+ mov result, #0
+ cpy ip, curbit
+ BranchToDiv #4, LSYM(Lthumb1_div_negative4)
+ BranchToDiv #8, LSYM(Lthumb1_div_negative8)
+LSYM(Lthumb1_div_large):
+ mov result, #0xfc
+ lsl divisor, divisor, #6
+ rev result, result
+ lsr curbit, dividend, #8
+ cmp curbit, divisor
+ blo
[patch, arm] Minor optimization on thumb2 tail call
Current thumb2 -Os generates suboptimal code for following tail call case:
int f4(int b, int a, int c, int d);
int g(int a, int b, int c, int d)
{ return f4(b, a, c, d); }
arm-none-eabi-gcc -Os -mthumb -mcpu=cortex-m3 test.c
push
{r4, lr}
mov r4, r1
mov r1, r0
mov r0, r4
pop {r4, lr}
b f4
There are two issues: The first one is that saving/restoring lr is not
necessary, as there is no return via pop pc. The second one is that even if
we managed to avoid lr push/pop, ldmia.w sp!, {r4} is still emitted as there
is a missing pattern for pop single and code size is not optimal.
This patch fixes these two issues and introduces a shared test case. CSiBE
thumb2 -Os shows cross board code size reduction, except for one case with 4
bytes regression. The case is like:
void f ()
{
if ()
...
else if ()
...
else g();
}
There are N=2 non-sibcall returns and S=1 sibcall return. Originally the
non-sibcall returns are just pop {r4, r5, pc}, now they become
b.n .Lreturn
.Lreturn:
pop {r4, r5}
bx lr
The one byte save from sibcall return does not win the non-sibcall return
regressions back. In general scenario, number of N non-sibcall returns use
b.n branching to merged tail, number of S sibcalls save 2 bytes by avoid
poping lr. It results in 4-2*S bytes regression. In the worst scenario, each
non-sibcall return has to use b.w branching to merged tail, resulting in
(N-S)*2 bytes regression. The worst scenario is rare, according to CSiBE.
The general regression scenario can only regress 2 bytes at most. So I would
not introduce additional complexity to handle the regression case.
Make check cortex-m3: pass
thumb2 bootstrap (O2/Os): pass
* config/arm/arm.c (arm_compute_save_reg_mask):
Do not save lr in case of tail call.
* config/arm/thumb2.md (*thumb2_pop_single): New pattern.
* gcc.target/arm/thumb2-pop-single.c: New test.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 4f04707..20d0b9e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19190,6 +19190,7 @@ arm_compute_save_reg_mask (void)
|| (save_reg_mask
optimize_size
ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL
+ !crtl-tail_call_emit
!crtl-calls_eh_return))
save_reg_mask |= 1 LR_REGNUM;
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 64acfea..29cfb17 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -267,6 +267,17 @@
(set_attr type multiple)]
)
+;; Pop a single register as its size is preferred over a post-incremental
load
+(define_insn *thumb2_pop_single
+ [(set (match_operand:SI 0 low_register_operand =r)
+(mem:SI (post_inc:SI (reg:SI SP_REGNUM]
+ TARGET_THUMB2 (reload_in_progress || reload_completed)
+ pop\t{%0}
+ [(set_attr type load1)
+ (set_attr length 2)
+ (set_attr predicable yes)]
+)
+
;; We have two alternatives here for memory loads (and similarly for
stores)
;; to reflect the fact that the permissible constant pool ranges differ
;; between ldr instructions taking low regs and ldr instructions taking
high
Re: Strange ARM optimisation issue in (android-ndk) gcc 4.8
This doesn't seem like a bug to me, unless they run with unexpected
results. My guess of the reason of different generated code can be the
mapping of registers and variables. s0, s1, s2 are not symmetric as
they are mapping to different parameter registers. GCC may choose
different sequence, in order to reduce number of register copying from
incoming registers to outgoing registers (parameter of foo).
Thanks,
Joey
On Mon, Nov 17, 2014 at 10:57 PM, Andrea Martino ciac...@gmail.com wrote:
Hi all,
Today I noticed something strange with the way gcc optimises ternary
operations in c (and c++). Consider the following example where I call
foo(int) passing the clamped value of (s0, s1, s2):
/**/
#include algorithm
extern C {
void foo(int i);
}
void bar(int s0, int s1, int s2)
{
foo(std::max(s0, std::min(s1, s2)));
}
/**/
When I compile the above class with -O1 -S, the generated assembly
contains 2 conditional moves and 0 branches:
stmfd sp!, {r3, lr}
.save {r3, lr}
cmp r2, r1
movge r2, r1
cmp r0, r2
movlt r0, r2
bl foo(PLT)
ldmfd sp!, {r3, pc}
If I replace the call to foo with one of the following:
foo(std::min(std::max(s0, s1), s2));
foo(s1 s2 ? (s1 s0 ? s1 : s0) : s2);
foo(s1 s2 ? (s1 s0 ? s0 : s1) : s2);
foo(s1 s2 ? s2 : (s1 s0 ? s1 : s0));
foo(s1 s2 ? s2 : (s1 s0 ? s0 : s1));
the generated output is the same. On the other side, when I replace
the foo call with one of the following:
foo(s0 s1 ? (s2 s1 ? s2 : s1) : s0);
foo(s0 s1 ? (s2 s1 ? s1 : s2) : s0);
foo(s0 s1 ? s0 : (s2 s1 ? s2 : s1));
foo(s0 s1 ? s0 : (s2 s1 ? s1 : s2));
The generated assembly contains 1 branch instruction + 2 conditional moves:
stmfd sp!, {r3, lr}
.save {r3, lr}
cmp r0, r1
bge .L2
cmp r1, r2
movlt r0, r1
movge r0, r2
.L2:
bl foo(PLT)
ldmfd sp!, {r3, pc}
I expected all the different combinations of clamp should generate
the same assembly.
Is there a reason for this? Is this a GCC bug?
Thanks
Andrea
Re: [PATCH, trivial][AArch64] Fix mode iterator for *aarch64_simd_ld1rmode pattern
Is there a case or PR to demonstrate the issue? If yes, better to
include it as a test case.
Thanks,
Joey
On Thu, Nov 13, 2014 at 2:14 PM, Yangfei (Felix) felix.y...@huawei.com wrote:
Hi,
We find that the VALLDI mode iterator used in *aarch64_simd_ld1rmode
pattern is not appropriate.
The reason is that it's impossible to get a new operand of DImode by
vec_duplicating an operand of the same mode.
So this patch just excludes the DImode and uses VALL instead.
Reg-tested for aarch64-linux-gnu with QEMU. OK for the trunk?
Index: gcc/ChangeLog
===
--- gcc/ChangeLog (revision 217394)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2014-11-13 Felix Yang felix.y...@huawei.com
+ Jiji Jiang jiangj...@huawei.com
+
+ * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1rmode): Use
+ VALL mode iterator instead of VALLDI.
+
2014-11-11 Andrew Pinski apin...@cavium.com
Bug target/61997
Index: gcc/config/aarch64/aarch64-simd.md
===
--- gcc/config/aarch64/aarch64-simd.md (revision 217394)
+++ gcc/config/aarch64/aarch64-simd.md (working copy)
@@ -4936,8 +4936,8 @@
})
(define_insn *aarch64_simd_ld1rmode
- [(set (match_operand:VALLDI 0 register_operand =w)
- (vec_duplicate:VALLDI
+ [(set (match_operand:VALL 0 register_operand =w)
+ (vec_duplicate:VALL
(match_operand:VEL 1 aarch64_simd_struct_operand Utv)))]
TARGET_SIMD
ld1r\\t{%0.Vtype}, %1
Re: 答复: [PATCH, trivial][AArch64] Fix mode iterator for *aarch64_simd_ld1rmode pattern
Can a new case be rewritten then?
- Joey
On Fri, Nov 14, 2014 at 9:32 AM, Yangfei (Felix) felix.y...@huawei.com wrote:
No, we noticed this issue when improving the vld1(q?)_dup intrinsics. Thanks.
Is there a case or PR to demonstrate the issue? If yes, better to include it
as a test
case.
Thanks,
Joey
On Thu, Nov 13, 2014 at 2:14 PM, Yangfei (Felix) felix.y...@huawei.com
wrote:
Hi,
We find that the VALLDI mode iterator used in *aarch64_simd_ld1rmode
pattern is not appropriate.
The reason is that it's impossible to get a new operand of DImode by
vec_duplicating an operand of the same mode.
So this patch just excludes the DImode and uses VALL instead.
Reg-tested for aarch64-linux-gnu with QEMU. OK for the trunk?
Index: gcc/ChangeLog
=
==
--- gcc/ChangeLog (revision 217394)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2014-11-13 Felix Yang felix.y...@huawei.com
+ Jiji Jiang jiangj...@huawei.com
+
+ * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1rmode):
Use
+ VALL mode iterator instead of VALLDI.
+
2014-11-11 Andrew Pinski apin...@cavium.com
Bug target/61997
Index: gcc/config/aarch64/aarch64-simd.md
=
==
--- gcc/config/aarch64/aarch64-simd.md (revision 217394)
+++ gcc/config/aarch64/aarch64-simd.md (working copy)
@@ -4936,8 +4936,8 @@
})
(define_insn *aarch64_simd_ld1rmode
- [(set (match_operand:VALLDI 0 register_operand =w)
- (vec_duplicate:VALLDI
+ [(set (match_operand:VALL 0 register_operand =w)
+ (vec_duplicate:VALL
(match_operand:VEL 1 aarch64_simd_struct_operand
Utv)))]
TARGET_SIMD
ld1r\\t{%0.Vtype}, %1
Re: More useful support for low-end ARM architecture
On Wed, Nov 12, 2014 at 1:47 AM, Joern Rennecke joern.renne...@embecosm.com wrote: On 11 November 2014 16:22, Joey Ye joey.ye...@gmail.com wrote: * Expensive effort. Either supporting none, or supporting all. There are large number of MCUs from ARM eco-system partners. Supporting all of them is a large project. * Maintance nightmare. Having GCC developers to input and maintain vendor specific configurations is inefficient and error-prone. * Failed schedule dependence. Having -mmcu actually means whenever there is a new MCU introduced into market, GCC has to be updated to describe it. Given GCC's release cycle (once per year) and the frequency of MCU release (could be tens each year), GCC will never be able to catch up. These kinds of issues were why I re-implemented the avr -mmcu option with the SELF_SPEC mechanism to read a device-specific spec file. As long as a new mcu can be described with the existing toolchain facilities (e.g. a new combination of existing I/O support, some different parameters describing RAM / flash sizes), a new spec file can be distributed together with a few hardware-specific library/crt files to support a new mcu with an existing compiler. Indeed. Took a look at the patch and I have to say it is a quite smart idea. Further more all the board/MCU specific configurations are already described in CMSIS and variant of GUI based toolchain for ARM MCU. Replicating then in GCC does not sounds a right thing to do for me. Yes, better to have a script that translates this info into a suitable spec file and copies any required files in the appropriate installation places. Not sure what kind of Copyright/licensing issues that would entail, though I would think of a separate project, where people can add new NCU configuration there independent to GCC. Thanks, Joey
Re: More useful support for low-end ARM architecture
Markus,
-mmcu probably will not work for ARM architectured MCUs. Reason are
* Confusion. -mcpu is encouraged and already widely used for ARM
architectures. Introducing -mmcu will be very confusing.
* Expensive effort. Either supporting none, or supporting all. There
are large number of MCUs from ARM eco-system partners. Supporting all
of them is a large project.
* Maintance nightmare. Having GCC developers to input and maintain
vendor specific configurations is inefficient and error-prone.
* Failed schedule dependence. Having -mmcu actually means whenever
there is a new MCU introduced into market, GCC has to be updated to
describe it. Given GCC's release cycle (once per year) and the
frequency of MCU release (could be tens each year), GCC will never be
able to catch up.
Further more all the board/MCU specific configurations are already
described in CMSIS and variant of GUI based toolchain for ARM MCU.
Replicating then in GCC does not sounds a right thing to do for me.
From the link you shared I also noticed some other discussion
regarding makefile, libraries and other options, which are very
interesting but off scope of GCC mail list. Shall we please continue
the discuss at https://answers.launchpad.net/gcc-arm-embedded/+question/257326
?
Thanks,
Joey
On Sun, Nov 9, 2014 at 3:42 AM, Markus Hitter m...@jump-ing.de wrote:
Hello gcc folks,
recently I started to expand a project of mine running mainly on AVR ATmega
to low end ARM chips. To my enlightment, gcc supports these thingies already.
To my disappointment, this support is pretty complicated. One faces at least
a much steeper learning curve that on AVR. Accordingly I suggested on the
avr-libc mailing list to do similar work for ARM, Cortex-M0 to Cortex-M4. At
least four people expressed interest, it looks like arm-libc is about to be
born.
To those not knowing what this is, I talk here about all-in-one CPUs (MCUs)
with memory and some peripherals already on the chip. Program memory can be
as low as 8 kB, RAM as low as 1 kB. Usually they're programmed bare-metal,
this is, without any operating system.
If you want to take a look at a simple Hello World application, here is one:
https://bugs.launchpad.net/gcc-arm-embedded/+bug/1387906
Looking at its Makefile, it requires quite a number of flags, among them nine
-I with custom paths, --specs, -T and also auto-generated C files. Lots of
stuff average programmers probably don't even know it exists. One of the
interested persons on the avr-libc mailing list explained what's missing,
much better than I could:
I think what the other responders missed is that avr-libc (via its
integration with binutils and gcc) gives you two key pieces of
functionality:
-mmcu=atmega88
#include avr/io.h
You *also* get classic libc functionality (printf, etc) that's provided
on ARM by newlib etc, but that's not the big deal IMO.
The #include is *relatively* easy, [... no topic for gcc ...]
The -mmcu= functionality is even more deeply useful, although less
easily repeatable on ARM. It brings in the relevant linker script,
startup code, vector tables, and all the other infrastructure. *THAT*
is what makes it possible to write a program like:
#include avr/io.h
int main() {
DDRD = 0x01;PORTD = 0x01;
}
# avr-gcc -mmcu=atmega88 -o test test.c
# avrdude
Writing a program for your random ARM chip requires digging *deeply*
into the various websites or IDEs of the manufacturer, trying to find
the right files (the filenames alone of which vary in strange ways),
probably determining the right way to alter them because the only
example you found was for a different chip in the same line, and then
hoping you've got everything glued together properly.
I want to be able to write the above program (modified for the right
GPIO) and run:
# arm-none-eabi-gcc -mmcu=stm32f405 -o test test.c
This is why I joined here, we'd like to get -mmcu for all the ARM flavours.
It should pick up a linker script which works in most cases on its own. It
should also bring in startup code, so code writers don't have to mess with
stuff happening before main(). And not to forget, pre-set #defines like
__ARM_LPC1114__, __ARM_STM32F405__, etc.
- How would we proceed in general?
- Many flavours at once, or better start with one or two, adding more when
these work?
- Did AVR support make things we should not repeat?
Thanks for discussing,
Markus
P.S.: arm-libc discussion so far can be followed here:
http://lists.nongnu.org/archive/html/avr-libc-dev/2014-11/threads.html
--
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/
Re: [PATCH 3/5] IPA ICF pass
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63747 is likely caused by
this patch. compare_gimple_switch does not check CASE_LOW and
CASE_HIGH, resulting merging functions not identical.
Interestingly in the first a few versions of this patch CASE_LOW/HIGH
were checked. But last versions only checks CASE_LABEL. What was the
intention?
Thanks,
Joey
On Thu, Oct 23, 2014 at 5:18 AM, Jiong Wang
wong.kwongyuan.to...@gmail.com wrote:
PR 63574 ICE building libjava (segfault) on arm-linux-gnueabihf is
caused by this commit.
from the backtrace, the ICF pass is trying to compare two label tree
node without type info.
while looks like compare_operand expect the type info always be not
empty before invoking func_checker::compatible_types_p.
I think we should add a similiar t1/t2 check at the start of
func_checker::compatible_types_p, just
like what has been done at the head of func_checker::compare_operand.
And I verified if we add that check, the crash gone away.
Regards,
Jiong
2014-10-15 18:03 GMT+01:00 Martin Liška mli...@suse.cz:
On 10/14/2014 06:04 PM, Jan Hubicka wrote:
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index fb41b01..2de98b4 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -172,6 +172,12 @@ public:
/* Dump referring in list to FILE. */
void dump_referring (FILE *);
+ /* Get number of references for this node. */
+ inline unsigned get_references_count (void)
+ {
+return ref_list.references ? ref_list.references-length () : 0;
+ }
Probably better called num_references() (like we have num_edge in
basic-block.h)
@@ -8068,6 +8069,19 @@ it may significantly increase code size
(see @option{--param ipcp-unit-growth=@var{value}}).
This flag is enabled by default at @option{-O3}.
+@item -fipa-icf
+@opindex fipa-icf
+Perform Identical Code Folding for functions and read-only variables.
+The optimization reduces code size and may disturb unwind stacks by
replacing
+a function by equivalent one with a different name. The optimization
works
+more effectively with link time optimization enabled.
+
+Nevertheless the behavior is similar to Gold Linker ICF optimization,
GCC ICF
+works on different levels and thus the optimizations are not same -
there are
+equivalences that are found only by GCC and equivalences found only by
Gold.
+
+This flag is enabled by default at @option{-O2}.
... and -Os?
+case ARRAY_REF:
+case ARRAY_RANGE_REF:
+ {
+ x1 = TREE_OPERAND (t1, 0);
+ x2 = TREE_OPERAND (t2, 0);
+ y1 = TREE_OPERAND (t1, 1);
+ y2 = TREE_OPERAND (t2, 1);
+
+ if (!compare_operand (array_ref_low_bound (t1),
+ array_ref_low_bound (t2)))
+ return return_false_with_msg ();
+ if (!compare_operand (array_ref_element_size (t1),
+ array_ref_element_size (t2)))
+ return return_false_with_msg ();
+ if (!compare_operand (x1, x2))
+ return return_false_with_msg ();
+ return compare_operand (y1, y2);
+ }
No need for {...} if there are no local vars.
+bool
+func_checker::compare_function_decl (tree t1, tree t2)
+{
+ bool ret = false;
+
+ if (t1 == t2)
+return true;
+
+ symtab_node *n1 = symtab_node::get (t1);
+ symtab_node *n2 = symtab_node::get (t2);
+
+ if (m_ignored_source_nodes != NULL m_ignored_target_nodes != NULL)
+{
+ ret = m_ignored_source_nodes-contains (n1)
+m_ignored_target_nodes-contains (n2);
+
+ if (ret)
+ return true;
+}
+
+ /* If function decl is WEAKREF, we compare targets. */
+ cgraph_node *f1 = cgraph_node::get (t1);
+ cgraph_node *f2 = cgraph_node::get (t2);
+
+ if(f1 f2 f1-weakref f2-weakref)
+ret = f1-alias_target == f2-alias_target;
+
+ return ret;
Comparing aliases is bit more complicated than just handling weakrefs. I
have
patch for symtab_node::equivalent_address_p somewhre in queue. lets just
drop
the fancy stuff for the moment and compare f1f2 for equivalence.
+ ret = compare_decl (t1, t2);
Why functions are not compared with compare_decl while variables are?
+
+ return return_with_debug (ret);
+}
+
+void
+func_checker::parse_labels (sem_bb *bb)
+{
+ for (gimple_stmt_iterator gsi = gsi_start_bb (bb-bb); !gsi_end_p
(gsi);
+ gsi_next (gsi))
+{
+ gimple stmt = gsi_stmt (gsi);
+
+ if (gimple_code (stmt) == GIMPLE_LABEL)
+ {
+ tree t = gimple_label_label (stmt);
+ gcc_assert (TREE_CODE (t) == LABEL_DECL);
+
+ m_label_bb_map.put (t, bb-bb-index);
+ }
+}
+}
+
+/* Basic block equivalence comparison function that returns true if
+ basic blocks BB1 and BB2 (from functions FUNC1 and FUNC2) correspond.
+
+ In general, a collection of equivalence dictionaries is built for
types
+ like SSA names, declarations (VAR_DECL, PARM_DECL, ..). This
infrastructure
+ is utilized by
Re: [PATCH, PR61605, 2/2] Use fuse-caller-save info in pass_cprop_hardreg
Tom, This patch broke arm thumb1 bootstrap. Please check details at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63718 Best Regards Joey On Fri, Oct 17, 2014 at 5:57 AM, Jeff Law l...@redhat.com wrote: On 10/16/14 03:14, Tom de Vries wrote: Eric, this patch is the second half of the fix for PR61605. It make sure in pass_cprop_hardreg that, if available we use the call-specific information provided by fuse-caller-save, rather than the generic regs_invalidated_by_call info. The 2 patches together allow an insn to removed from the gcc.target/i386/fuse-caller-save.c testcase, which is updated accordingly in this patch. Bootstrapped and reg-tested on x86_64. OK for trunk? Thanks, - Tom 0002-Use-fuse-caller-save-info-in-cprop-hardreg.patch 2014-10-13 Tom de Vriest...@codesourcery.com PR rtl-optimization/61605 * regcprop.c (copyprop_hardreg_forward_1): Use regs_invalidated_by_this_call instead of regs_invalidated_by_call. * gcc.target/i386/fuse-caller-save.c: Update addition check. Add movl absence check. OK. Jeff
Re: plugins header file
Sounds a good idea to me, here is the list I'm using: #include params.h #include flags.h #include tree.h #include tree-pass.h #include basic-block.h #include function.h #include hash-table.h #include tree-ssa-alias.h #include tree-cfg.h #include tree-ssa-operands.h #include tree-inline.h #include gimple-expr.h #include is-a.h #include gimple.h #include tree-phinodes.h #include gimple-iterator.h #include gimple-ssa.h #include ssa-iterators.h #include tree-into-ssa.h #include cfgloop.h #include context.h However, it does not automatically solve the missing header file issue in Makefile, any idea to solve this problem? - Joey On Tue, Sep 16, 2014 at 2:18 AM, Andrew MacLeod amacl...@redhat.com wrote: During the re-architecture session at Cauldron, I mentioned the possibility of introducing a plugin-headers.h. This would be a file which plugins could use which would protect them somewhat from header file restructuring. The idea is that it includes all the common things plugins need, (like gimple.h, rtl.h, most-of-the-world.h, etc etc),. When header files are restructured, that file would also be adjusted so that the correct include order is still maintained. This could also give plugins a little more stability across releases since header files do come and go.. I am about to start another round of flattening and shuffling, so figured this might be a good time to introduce it. Any of you plugin users have a list of includes you want to see in it, or better yet, provide me with a plugin-headers.h? ( Out of curiosity, is there a reason gcc-plugins.h doesn't include a pile of these common things? or is that simply to avoid bringing in the world?) Or would you rather just continue to deal with the pain of header file name changing/content shuffling? or is there a different solution proposal? Andrew
[patch][plugin] Fix PR59335 - missing plugin headers again
Trunk fails to build plugin again due to missing plugin header files. This patch fixes it. OK to trunk? ChangeLog: PR plugin/59335 * Makefile.in (PLUGIN_HEADERS): Add wide-int.h, signop.h, hash-map.h, hash-set.h. diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 63124f8..8e7aada 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -3170,7 +3170,8 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ tree-parloops.h tree-ssa-address.h tree-ssa-coalesce.h tree-ssa-dom.h \ tree-ssa-loop.h tree-ssa-loop-ivopts.h tree-ssa-loop-manip.h \ tree-ssa-loop-niter.h tree-ssa-ter.h tree-ssa-threadedge.h \ - tree-ssa-threadupdate.h inchash.h + tree-ssa-threadupdate.h inchash.h wide-int.h signop.h hash-map.h \ + hash-set.h # generate the 'build fragment' b-header-vars s-header-vars: Makefile
Re: selective linking of floating point support for *printf / *scanf
On Sat, Aug 30, 2014 at 12:26 PM, Thomas Preud'homme thomas.preudho...@arm.com wrote: From: Grissiom [mailto:chaos.pro...@gmail.com] Sent: Friday, August 29, 2014 11:51 PM Yes, it does. The namespace reserved for the implementation is _[_A-Z]. The namespace _[a-z] is still available for the user. Which means the user can declare their own _printf_float, and WE (as the implementation) MUST NOT INTERFERE with it. Since WE are the implementation, we should use the namespace reserved for us, namely __printf_float. Mmmh indeed. I checked C99 and section 7.1.3 paragraph 1 third clause states: All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use. Next clause express how single underscore not followed by a capital letter is reserved: All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces. Apparently newlib is not following this specification very well, as there are symbols like _abc_r defined every where in current newlib. I am not implying the spec should not be followed, but is newlib designed to have a loose spec for the single underscore? - Joey Since here we are talking about linkage, _printf_float is not safe according to the standard. Sigh. Ok I need to think about it. Thank you all for pointing out the problem with the current scheme. Best regards, Thomas
Re: Is escaping of a temp variable valid?
On Fri, Aug 15, 2014 at 6:33 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Fri, Aug 15, 2014 at 10:45 AM, Joey Ye joey.ye...@gmail.com wrote:
Running into an unexpected result with GCC with following case, but
not sure if it is a valid C++ case.
#define nullptr 0
enum nonetype { none };
templatetypename T
class class_zoo {
public:
const T *data;
int length;
class_zoo (nonetype) : data (nullptr), length (0) {}
class_zoo (const T e) : data (e), length (1) {}
Capturing a const referece via a pointer is error-prone as for
example literal constants class_zooconst int * zoo(0)
have associated objects that live only throughout the function
call.
Thanks for confirming this. But do you imply capturing a non-const
reference via a pointer is safe, which I would assume it unsafe
either?
- Joey
So clearly your testcase is invalid.
Richard.
Re: RFD: selective linking of floating point support for *printf / *scanf
Joern, there is https://sourceware.org/ml/newlib/2014/msg00166.html, which is already in newlib mainline. I think it solves the same issue in a slight different approach. Does it work for you? Thanks, Joey On Thu, Aug 14, 2014 at 4:52 PM, Joern Rennecke joern.renne...@embecosm.com wrote: For embedded targets with small memories and static linking, the size of functions like *printf and their dependencies is painful, particularily for targets that need software floating point. avr-libc has long had a printf / scanf implementation that by default does not include floating point support. There's a library that can be liked to provide the floating-point enabled functions, but the required functions have to be pulled in manually with -Wl,-u if they are otherwise only referenced from libc, lest these symbols got resolved with the integer-only implementations from libc itself. All in all, a rather unsatisfying state of affairs when trying to run the gcc regression test suite. Newlib also has an integer-only printf implementation, but in this case, the default is the other way round - you have to use functions with nonstandard names to use the integer-only implementations. And a half-hearted approach to use this can easily end up with linking in both the integer-only version and the floating-point enabled one, resulting in increased executable size instead of a saving. I think we can do better with integrated compiler/linker support. Trying to do a perfect job i of course impossible because of Rice's theorem, but it doesn't have to be perfect to be useful. Just looking statically at each *printf statement, we can look at the format strings and/or the passed arguments. Floating point arguments are easier to check for by the compiler than parsing the format string. There is already code that parses the format strings for the purpose of warnings, but it would be a somewhat intrusive change to add this functionality there, and the information is not available where a variable format is used anyway. In a standards-conformant application, floating point formats can only be used with floating point arguments, so checking for the latter seems most effective. So my idea is to make the compile emit special calls when there are no floating point arguments. A library that provides the floating point enabled *printf/*scanf precedes libc in link order. Libc contains the integer-only implementations of *scanf/*printf, in two parts: entry points with the special function name, which in the same object file also contain a reference to the ordinary function name, and another object file with the ordinary symbol and the integer-only implementation. Thus, if any application translation unit has pulled in a floating-point enabled implementation, this is the one that'll be used. Otherwise, the integer-only one will be used. Use of special sections and alphasorting of these in the linker script ensures that the integer-only entry points appear in the right place at the start of the chosen implementation. If vfprintf is used I've implemented this for AVR with these commits: https://github.com/embecosm/avr-gcc/commit/3b3bfe33fe29b6d29d8fb96e5d57ee025adf7af0 https://github.com/embecosm/avr-libc/commit/c55eba74838635613c8b80d86a85ed605a79d337 https://github.com/embecosm/avr-binutils-gdb/commit/72b3a1ea3659577198838a7149c6882a079da403 Although it could use some more testing, and thought how to best introduce the change as to avoid getting broken toolchains when components are out-of-sync. Now Joerg Wunsch suggested we might want to facto out more pieces, like the long long support. This quickly leads to a combinatorial explosion. If we want to support a more modular *printf / *scanf, than maybe a different approach is warranted. Say, if we could give a symbol and section attribute and/or pragma to individual case labels of a switch, and put the different pieces into separate object files (maybe with a bit of objcopy massaging). The symbols references to trigger the inclusion of the case objects could be generated by the gcc backend by processing suitably annotated function calls. E.g. we might put something into CALL_FUNCTION_USAGE, or play with TARGET_ENCODE_SECTION_INFO.
Re: [PATCH] Don't set dir prefix twice (PR middle-end/60484)
OK to 4.8 then? On Thu, Aug 14, 2014 at 6:36 PM, Richard Biener richard.guent...@gmail.com wrote: On Thu, Aug 14, 2014 at 7:34 AM, Joey Ye joey.ye...@gmail.com wrote: PR60484 is marked as 4.7/4.8 regression and it is reported against 4.8 recently by an user. OK backporting to 4.7/4.8? The 4.7 branch is closed. Richard. - Joey On Sat, Mar 15, 2014 at 1:43 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Fri, 14 Mar 2014, Marek Polacek wrote: This patch makes sure that we set the directory prefix of dump_base_name only once, otherwise we'd end up with invalid path, resulting in error: could not open dump file ... This happened because finish_options is called for every optimize attribute and once more for command line options and every time it added the directory prefix. Regtested/bootstrapped on x86_64-linux, ok for trunk? OK, though I think it might be better to use separate fields of gcc_options for the originally specified name and the prefixed version. -- Joseph S. Myers jos...@codesourcery.com
Is escaping of a temp variable valid?
Running into an unexpected result with GCC with following case, but
not sure if it is a valid C++ case.
#define nullptr 0
enum nonetype { none };
templatetypename T
class class_zoo {
public:
const T *data;
int length;
class_zoo (nonetype) : data (nullptr), length (0) {}
class_zoo (const T e) : data (e), length (1) {}
};
int bar (class_zooconst int * p1 = none,
class_zooconst unsigned * p2 = none)
{
if (*p1.data==nullptr) return 1;
return 0;
}
int foo (int *b) // If changing int to const int here, zoo will
return 0 (pass)
{
class_zooconst int * zoo(b);
return bar(zoo);
}
int g = 678;
int main ()
{
return foo (g);
}
$ g++ main.cpp -fstack-protector -o m
$ ./m
$ echo $?
1
Expand shows:
D.2320 = b_2(D);
class_zooconst int*::class_zoo (zoo, D.2320);
D.2320 ={v} {CLOBBER};--- D.2320 is dead, but it is address
escapes to zoo
--- with
stack-protector, D.2322 reuses stack slot of D.2320, thus overwrite
D.2320
class_zooconst unsigned int*::class_zoo (D.2322, 0);
_8 = bar (zoo, D.2322); D.2320 is accessed via its
address, but value changed now
_9 = _8;
D.2322 ={v} {CLOBBER};
zoo ={v} {CLOBBER};
;;succ: 3
;; basic block 3, loop depth 0
;;pred: 2
L2:
return _9;
;;succ: EXIT
}
Partition 0: size 16 align 16
D.2322 D.2320 --- GCC believes they are not
conflict, which is what I not sure of here
Partition 2: size 16 align 16
zoo
Questions
1. Is this a correct test case?
2. If not, why escaping of D.2320's address to a local is not
accounted as conflicting to other local variables?
Thanks,
Joey
Re: [PATCH] Don't set dir prefix twice (PR middle-end/60484)
PR60484 is marked as 4.7/4.8 regression and it is reported against 4.8 recently by an user. OK backporting to 4.7/4.8? - Joey On Sat, Mar 15, 2014 at 1:43 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Fri, 14 Mar 2014, Marek Polacek wrote: This patch makes sure that we set the directory prefix of dump_base_name only once, otherwise we'd end up with invalid path, resulting in error: could not open dump file ... This happened because finish_options is called for every optimize attribute and once more for command line options and every time it added the directory prefix. Regtested/bootstrapped on x86_64-linux, ok for trunk? OK, though I think it might be better to use separate fields of gcc_options for the originally specified name and the prefixed version. -- Joseph S. Myers jos...@codesourcery.com
Re: [resend] implement assembly variant routines for IEEE floating-point under libgcc for cortex-m0
Mallikarjun, Thanks for trying this. I agree with you that current C implementation is far from optimal. It is in our road-map with a different approach, which will not simply port current functions from armv7-m to armv6-m. This task is low priority due to less common usage of fp in Cortex-M0*, so it may take years to show up in mainline. Thanks, Joey On Mon, Aug 4, 2014 at 2:40 PM, Mallikarjun Goudar mallikarjun.go...@gmail.com wrote: Hi All, sorry, sending previous mail with proper subject line. For cortex-m0 (armv6-m) target, i observed that ieee floating-point functions under libgcc are implemented in C. This takes lot of memory if one uses floating-point in their applciations. I agree that usage of fp in cortex-m0 devices might be minimal. I wanted to check if its good idea to implement these fp functions in assembly. As we have already assembly variant functions for cortex-m4 (armv7-m) targets, It would be nice to port those to armv6-m. I have ported couple of routines as an exercise. And results looked good. I see improvement in size and speed. I ran some fp tests on hardware board to see the performance. I would be interested in porting remaining functions and contribute to the community. I am open to discussion on this. Please reply if you feel its good idea to implement this feature. If anyone has already working on this, please let me know. Thanks, Mallikarjuna
Re: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion
If f2d need fix, then please fix d2f too as current implementation for
both behave similarly.
- Joey
On Mon, May 19, 2014 at 5:23 AM, Aurelien Jarno aurel...@aurel32.net wrote:
On ARM soft-float, the float to double conversion doesn't convert a sNaN
to qNaN as the IEEE Std 754 standard mandates:
Under default exception handling, any operation signaling an invalid
operation exception and for which a floating-point result is to be
delivered shall deliver a quiet NaN.
Given the soft float ARM code ignores exceptions and always provides a
result, a float to double conversion of a signaling NaN should return a
quiet NaN. Fix this in extendsfdf2.
2014-05-18 Aurelien Jarno aurel...@aurel32.net
PR target/61219
* config/arm/ieee754-df.S (extendsfdf2): Convert sNaN to qNaN.
Index: libgcc/config/arm/ieee754-df.S
===
--- libgcc/config/arm/ieee754-df.S (revision 210588)
+++ libgcc/config/arm/ieee754-df.S (working copy)
@@ -473,11 +473,15 @@
eorne xh, xh, #0x3800 @ fixup exponent otherwise.
RETc(ne)@ and return it.
- teq r2, #0 @ if actually 0
- do_it ne, e
- teqne r3, #0xff00 @ or INF or NAN
+ bicsr2, r2, #0xff00 @ isolate mantissa
+ do_it eq @ if 0, that is ZERO or INF,
RETc(eq)@ we are done already.
+ teq r3, #0xff00 @ check for NAN
+ do_it eq, t
+ orreq xh, xh, #0x0008 @ change to quiet NAN
+ RETc(eq)@ and return it.
+
@ value was denormalized. We can normalize it now.
do_push {r4, r5, lr}
mov r4, #0x380 @ setup corresponding exponent
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net
RE: [patch] Shorten Windows path
Ping
-Original Message-
From: Joey Ye [mailto:joey...@arm.com]
Sent: Tuesday, April 01, 2014 6:18 PM
To: 'Ian Lance Taylor'
Cc: gcc-patches
Subject: RE: [patch] Shorten Windows path
Ian, thanks for your comments. Please find answers and new version below:
-Original Message-
From: Ian Lance Taylor [mailto:i...@google.com]
Sent: 25 March 2014 21:09
To: Joey Ye
Cc: gcc-patches
Subject: Re: [patch] Shorten Windows path
On Tue, Mar 25, 2014 at 1:58 AM, Joey Ye joey...@arm.com wrote:
Ping
This code looks different on mainline.
Writing if ( do_canonical ) is not GCC style.
Fixed
This patch does not respect the configure option --disable-canonical-
system-
headers.
Solved by put is under the control of default
ENABLE_CANONICAL_SYSTEM_HEADERS
Also I personally don't actually know what the consequences would be.
Are there any downsides to canonicalizing header names?
Since 4.8 system headers are by default canonicalized. This version only
additionally canonical non-system headers. I can't think of any downsides.
Ian
ChangeLog.libcpp:
* files.c (find_file_in_dir): Always try to shorten for DOS non-system
headers.
* init.c (ENABLE_CANONICAL_SYSTEM_HEADERS): Default enabled for DOS.
diff --git a/libcpp/files.c b/libcpp/files.c
index 7e88778..ad68682 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -387,8 +387,14 @@ find_file_in_dir (cpp_reader *pfile, _cpp_file *file,
bool *invalid_pch)
char *copy;
void **pp;
- /* We try to canonicalize system headers. */
- if (CPP_OPTION (pfile, canonical_system_headers) file-dir-sysp)
+ /* We try to canonicalize system headers. For DOS based file
+ * system, we always try to shorten non-system headers, as DOS
+ * has a tighter constraint on max path length. */
+ if (CPP_OPTION (pfile, canonical_system_headers) file-dir-sysp
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ || !file-dir-sysp
+#endif
+ )
{
char * canonical_path = maybe_shorter_path (path);
if (canonical_path)
diff --git a/libcpp/init.c b/libcpp/init.c
index f10413a..b809515 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -27,8 +27,12 @@ along with this program; see the file COPYING3. If not
see
#include filenames.h
#ifndef ENABLE_CANONICAL_SYSTEM_HEADERS
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+#define ENABLE_CANONICAL_SYSTEM_HEADERS 1
+#else
#define ENABLE_CANONICAL_SYSTEM_HEADERS 0
#endif
+#endif
static void init_library (void);
static void mark_named_operators (cpp_reader *, int);
RE: [patch, testsuite] Fix fragile case nsdmi-union5
-Original Message- From: Mike Stump [mailto:mikest...@comcast.net] Sent: Monday, April 21, 2014 11:39 PM To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch, testsuite] Fix fragile case nsdmi-union5 On Apr 17, 2014, at 10:28 PM, Joey Ye joey...@arm.com wrote: Resulting from discussion here: http://gcc.gnu.org/ml/gcc/2014-04/msg00125.html Not checked in, and no Ok? asked. You should do one or the other. :-) I'll assume Ok? Ok. Just committed.
[patch, testsuite] Fix fragile case nsdmi-union5
Resulting from discussion here:
http://gcc.gnu.org/ml/gcc/2014-04/msg00125.html
ChangeLog:
* g++.dg/cpp0x/nsdmi-union5.C: Change to runtime test.
Index: gcc/testsuite/g++.dg/cpp0x/nsdmi-union5.C
===
--- gcc/testsuite/g++.dg/cpp0x/nsdmi-union5.C (revision 209462)
+++ gcc/testsuite/g++.dg/cpp0x/nsdmi-union5.C (working copy)
@@ -1,6 +1,5 @@
// PR c++/58701
-// { dg-require-effective-target c++11 }
-// { dg-final { scan-assembler 7 } }
+// { dg-do run { target c++11 } }
static union
{
@@ -9,3 +8,10 @@
int i = 7;
};
};
+
+extern C void abort(void);
+int main()
+{
+ if (i != 7) abort();
+ return 0;
+}
Fragile test case nsdmi-union5.C
Ran into a fragile test case:
FAIL: g+.dg/cpp0x/nsdmi-union5.C -std=c+11 scan-assembler 7
$ cat nsdmi-union5.C
// PR c++/58701
// { dg-require-effective-target c++11 }
// { dg-final { scan-assembler 7 } }
static union
{
union
{
int i = 7;
};
};
Two issues make it very fragile. It only seems to pass with -O0, as the code
will be optimized away with any non-O0 levels. This is somewhat acceptable,
but following is annoying: It scans digit 7 in resulting asm, which will
pass whenever
* Any CPU name, file name, svn/git revision number and ARM eabi_attribute
contain digit 7.
* All GCC x.7 versions
* Any GCC built in July or on 7th/17th/27th of the month, or in 2017
Actually I ran into this issue when I checked my test result with a
reference. Unfortunately one has digit 7 in revision number and one has not.
I tend to just not scan anything and makes it a do-compile case against ICE.
But I'm not sure if there was a reason to scan the digit. Please comment.
Thanks,
Joey
RE: [patch] Disable if_conversion2 for Og
-Original Message-
From: Joey Ye [mailto:joey...@arm.com]
Sent: Tuesday, April 15, 2014 6:37 PM
To: 'Richard Biener'
Cc: GCC Patches
Subject: RE: [patch] Disable if_conversion2 for Og
Ok for trunk and branches after a while. Why does if-conversion not have
the same problem? On the GIMPLE part we avoid all kinds of if-conversion
with -Og.
I think if-conversion should be disabled for Og too, but I don't have a case
to
show that it is harmful. If GIMPLE avoids all if-conversion, it is nature to
do
the same for RTL. I'll test and send another patch to also disable
if-conversion.
New patch tested with more regressions with -Og, which are expected.
FAIL: gcc.target/arm/its.c scan-assembler-times \\tit 2
FAIL: gcc.target/arm/pr40956.c scan-assembler-times mov[t ]*r., #0 1
FAIL: gcc.target/arm/pr42835.c scan-assembler-times moveq[t ]*r.,[t ]*#
1
FAIL: gcc.target/arm/pr42835.c scan-assembler-times movne[t ]*r.,[t ]*#
1
FAIL: gcc.target/arm/shiftable.c scan-assembler sub.*[al]sl #6
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler asreq
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler lslne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler asrne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler lslne
FAIL: gcc.target/arm/vseleqdf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vseleqsf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselgedf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselgesf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselgtdf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselgtsf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselledf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vsellesf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselltdf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselltsf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselnedf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselnesf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselvcdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselvcsf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselvsdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselvssf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1
OK?
ChangeLog:
* opts.c (OPT_fif_conversion, OPT_fif_conversion2): Disable for Og.
diff --git a/gcc/opts.c b/gcc/opts.c
index fdc903f..3f3db1a 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -431,8 +431,8 @@ static const struct default_options default_options_table[]
=
{ OPT_LEVELS_1_PLUS, OPT_fguess_branch_probability, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fcprop_registers, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fforward_propagate, NULL, 1 },
-{ OPT_LEVELS_1_PLUS, OPT_fif_conversion, NULL, 1 },
-{ OPT_LEVELS_1_PLUS, OPT_fif_conversion2, NULL, 1 },
+{ OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion, NULL, 1 },
+{ OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion2, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fipa_pure_const, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fipa_reference, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fipa_profile, NULL, 1 },
RE: [patch] Disable if_conversion2 for Og
-Original Message- From: Richard Earnshaw Sent: Wednesday, April 16, 2014 5:44 PM To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Disable if_conversion2 for Og Arguably, this is a bug in gdb. The debugger should understand when a breakpointed conditional instruction is not going to execute and silently continue. That preserves the illusion of not executing the code without requiring the compiler to de-optimize things. R. Or compiler just optimizes it, and emits generic DWARFx information to help GDB handle it in more target independently? - Joey
RE: [patch] Disable if_conversion2 for Og
-Original Message- From: Richard Earnshaw Sent: Wednesday, April 16, 2014 6:04 PM To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Disable if_conversion2 for Og On 16/04/14 11:02, Joey Ye wrote: -Original Message- From: Richard Earnshaw Sent: Wednesday, April 16, 2014 5:44 PM To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Disable if_conversion2 for Og Arguably, this is a bug in gdb. The debugger should understand when a breakpointed conditional instruction is not going to execute and silently continue. That preserves the illusion of not executing the code without requiring the compiler to de-optimize things. R. Or compiler just optimizes it, and emits generic DWARFx information to help GDB handle it in more target independently? - Joey I'm not sure extra dwarf info would help much. The debugger still has to understand that the breakpoint has not really been hit. R. Yes, it is inevitable. But without extra dwarf info it will be even more painful: each time setting break-point or break-point hits it has to decode the break-pointed instructions and its context to search for conditional execution and IT blocks. - Joey
RE: [patch] Disable if_conversion2 for Og
-Original Message- From: Richard Earnshaw Sent: Wednesday, April 16, 2014 6:21 PM To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Disable if_conversion2 for Og On 16/04/14 11:17, Joey Ye wrote: -Original Message- From: Richard Earnshaw Sent: Wednesday, April 16, 2014 6:04 PM To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Disable if_conversion2 for Og On 16/04/14 11:02, Joey Ye wrote: -Original Message- From: Richard Earnshaw Sent: Wednesday, April 16, 2014 5:44 PM To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Disable if_conversion2 for Og Arguably, this is a bug in gdb. The debugger should understand when a breakpointed conditional instruction is not going to execute and silently continue. That preserves the illusion of not executing the code without requiring the compiler to de-optimize things. R. Or compiler just optimizes it, and emits generic DWARFx information to help GDB handle it in more target independently? - Joey I'm not sure extra dwarf info would help much. The debugger still has to understand that the breakpoint has not really been hit. R. Yes, it is inevitable. But without extra dwarf info it will be even more painful: each time setting break-point or break-point hits it has to decode the break- pointed instructions and its context to search for conditional execution and IT blocks. For thumb code it can get the conditional information it needs from the IT state in the PSR; for ARM code it has to look no further than the instruction itself. R. The thing is, debugger has to do this for every breakpoint, even though more of them are not conditional execution, which isn't efficient.
RE: [patch] Disable if_conversion2 for Og
-Original Message-
From: Richard Biener [mailto:richard.guent...@gmail.com]
Sent: Tuesday, April 15, 2014 4:05 PM
To: Joey Ye
Cc: GCC Patches
Subject: Re: [patch] Disable if_conversion2 for Og
On Tue, Apr 15, 2014 at 3:59 AM, Joey Ye joey...@arm.com wrote:
If-converstion is harmful to optimized debugging as it generates
conditional execution instructions with line number information, which
resulted in a dillusion to developers that both then-else branches are
executed.
For example:
test.c:
1: unsigned oldest_sequence;
2:
3: unsigned foo(unsigned seq_number)
4: {
5: if ((seq_number + 5) 10)
6:seq_number += 100;
7: else
8: seq_number = oldest_sequence;
if (seq_number oldest_sequence)
seq_number = oldest_sequence;
return seq_number;
}
$ arm-none-eabi-gcc -mthumb -mcpu=cortex-m3 -Og -g3
gets:
.loc 1 5 0
addsr3, r0, #5
cmp r3, #9
.loc 1 6 0 - line 6, then branch
iteels
addls r0, r0, #100
.LVL1:
.loc 1 8 0 - line 8, else branch. Both branches seems to
be executed in GDB
ldrhi r3, .L5
ldrhi r0, [r3]
The reason is that if_conversion2 is still enabled in Og. The patch
simply disables it for Og.
Tests:
* -Og bootstrap passed.
* Make check default (no additional option): No regression.
* Make check with -Og: expected regressions. Cases relying on
if-conversion2 failed.
FAIL: gcc.target/arm/its.c scan-assembler-times \\tit 2
FAIL: gcc.target/arm/pr40956.c scan-assembler-times mov[t ]*r.,
#0 1
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler asreq
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler lslne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler asrne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler lslne
OK to trunk and 4.8/4.9 branch?
Ok for trunk and branches after a while. Why does if-conversion not have
the same problem? On the GIMPLE part we avoid all kinds of if-conversion
with -Og.
I think if-conversion should be disabled for Og too, but I don't have a case to
show that it is harmful. If GIMPLE avoids all if-conversion, it is nature to do
the same for RTL. I'll test and send another patch to also disable
if-conversion.
[patch] Disable if_conversion2 for Og
If-converstion is harmful to optimized debugging as it generates conditional
execution instructions with line number information, which resulted in a
dillusion to developers that both then-else branches are executed.
For example:
test.c:
1: unsigned oldest_sequence;
2:
3: unsigned foo(unsigned seq_number)
4: {
5: if ((seq_number + 5) 10)
6:seq_number += 100;
7: else
8: seq_number = oldest_sequence;
if (seq_number oldest_sequence)
seq_number = oldest_sequence;
return seq_number;
}
$ arm-none-eabi-gcc -mthumb -mcpu=cortex-m3 -Og -g3
gets:
.loc 1 5 0
addsr3, r0, #5
cmp r3, #9
.loc 1 6 0 - line 6, then branch
iteels
addls r0, r0, #100
.LVL1:
.loc 1 8 0 - line 8, else branch. Both branches seems to
be executed in GDB
ldrhi r3, .L5
ldrhi r0, [r3]
The reason is that if_conversion2 is still enabled in Og. The patch simply
disables it for Og.
Tests:
* -Og bootstrap passed.
* Make check default (no additional option): No regression.
* Make check with -Og: expected regressions. Cases relying on if-conversion2
failed.
FAIL: gcc.target/arm/its.c scan-assembler-times \\tit 2
FAIL: gcc.target/arm/pr40956.c scan-assembler-times mov[t ]*r., #0 1
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler asreq
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler lslne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler asrne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler lslne
OK to trunk and 4.8/4.9 branch?
ChangeLog:
* opts.c (OPT_fif_conversion2): Disable for Og.
diff --git a/gcc/opts.c b/gcc/opts.c
index fdc903f..e076253 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -432,7 +432,7 @@ static const struct default_options
default_options_table[] =
{ OPT_LEVELS_1_PLUS, OPT_fcprop_registers, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fforward_propagate, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fif_conversion, NULL, 1 },
-{ OPT_LEVELS_1_PLUS, OPT_fif_conversion2, NULL, 1 },
+{ OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion2, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fipa_pure_const, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fipa_reference, NULL, 1 },
{ OPT_LEVELS_1_PLUS, OPT_fipa_profile, NULL, 1 },
RE: [patch] Shorten Windows path
Ian, thanks for your comments. Please find answers and new version below:
-Original Message-
From: Ian Lance Taylor [mailto:i...@google.com]
Sent: 25 March 2014 21:09
To: Joey Ye
Cc: gcc-patches
Subject: Re: [patch] Shorten Windows path
On Tue, Mar 25, 2014 at 1:58 AM, Joey Ye joey...@arm.com wrote:
Ping
This code looks different on mainline.
Writing if ( do_canonical ) is not GCC style.
Fixed
This patch does not respect the configure option --disable-canonical-system-
headers.
Solved by put is under the control of default ENABLE_CANONICAL_SYSTEM_HEADERS
Also I personally don't actually know what the consequences would be.
Are there any downsides to canonicalizing header names?
Since 4.8 system headers are by default canonicalized. This version only
additionally canonical non-system headers. I can't think of any downsides.
Ian
ChangeLog.libcpp:
* files.c (find_file_in_dir): Always try to shorten for DOS non-system
headers.
* init.c (ENABLE_CANONICAL_SYSTEM_HEADERS): Default enabled for DOS.
diff --git a/libcpp/files.c b/libcpp/files.c
index 7e88778..ad68682 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -387,8 +387,14 @@ find_file_in_dir (cpp_reader *pfile, _cpp_file *file, bool
*invalid_pch)
char *copy;
void **pp;
- /* We try to canonicalize system headers. */
- if (CPP_OPTION (pfile, canonical_system_headers) file-dir-sysp)
+ /* We try to canonicalize system headers. For DOS based file
+ * system, we always try to shorten non-system headers, as DOS
+ * has a tighter constraint on max path length. */
+ if (CPP_OPTION (pfile, canonical_system_headers) file-dir-sysp
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ || !file-dir-sysp
+#endif
+)
{
char * canonical_path = maybe_shorter_path (path);
if (canonical_path)
diff --git a/libcpp/init.c b/libcpp/init.c
index f10413a..b809515 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -27,8 +27,12 @@ along with this program; see the file COPYING3. If not see
#include filenames.h
#ifndef ENABLE_CANONICAL_SYSTEM_HEADERS
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+#define ENABLE_CANONICAL_SYSTEM_HEADERS 1
+#else
#define ENABLE_CANONICAL_SYSTEM_HEADERS 0
#endif
+#endif
static void init_library (void);
static void mark_named_operators (cpp_reader *, int);
RE: [patch] Shorten Windows path
Ping
-Original Message-
From: Joey Ye [mailto:joey...@arm.com]
Sent: 19 February 2014 15:45
To: gcc-patches@gcc.gnu.org; Ian Lance Taylor (i...@google.com)
Subject: [patch] Shorten Windows path
Max length of path on Windows is 255, which is easy to exceed in a
complicated project. Ultimate solution may be complex but canonizing the
path and skipping the ..s in path is helpful.
Relative discussion in gcc-patches:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00582.html
OK to trunk stage 1?
ChangeLog.libcpp:
* files.c (find_file_in_dir): Always try to shorten for DOS.
diff --git a/libcpp/files.c b/libcpp/files.c index 7e88778..9dcc71f 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -386,9 +386,18 @@ find_file_in_dir (cpp_reader *pfile, _cpp_file *file,
bool *invalid_pch)
hashval_t hv;
char *copy;
void **pp;
+ bool do_canonical;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ /* For DOS based file system, we always try to shorten file path
+ * to as it has a shorter constraint on max path length. */
+ do_canonical = true;
+#else
/* We try to canonicalize system headers. */
- if (CPP_OPTION (pfile, canonical_system_headers)
file-dir-sysp)
+ do_canonical = (CPP_OPTION (pfile, canonical_system_headers)
+ file-dir-sysp); #endif
+ if ( do_canonical )
{
char * canonical_path = maybe_shorter_path (path);
if (canonical_path)
Re: [PATCH] Fix incorrect byte swap detection (PR tree-optimization/60454)
4.8 also has this bug. OK to backport? On Tue, Mar 11, 2014 at 6:59 PM, Jakub Jelinek ja...@redhat.com wrote: On Tue, Mar 11, 2014 at 06:48:37PM +0800, Thomas Preud'homme wrote: I also added a typedef unsigned uint32_t for when sizeof(unsigned) == 4. I hope it's right. In theory you could have __CHAR_BIT__ different from 8 and what you care about is that uint32_t has exactly 32 bits, so the check would need to be if (sizeof (uint32_t) * __CHAR_BIT__ != 32) return 0; + if (fake_swap32 (0x12345678) != 0x78567E12) +__builtin_abort (); Also, for int16 targets where __UINT32_TYPE__ is supposedly unsigned long, I think you would need to use: if (fake_swap32 (0x12345678UL) != 0x78567E12UL) __builtin_abort (); (the C standard guarantees that unsigned long is at least 32-bit and unsigned int at least 16-bit). Ok with those changes. Do you have write access, or will somebody from your coworkers commit it for you? Are you covered by ARM GCC Copyright assignment? Jakub
RE: [patch] [arm] Fix PR60169 - thumb1 far jump
Ping. OK for trunk and 4.8?
-Original Message-
From: Joey Ye [mailto:joey...@arm.com]
Sent: 21 February 2014 19:32
To: gcc-patches@gcc.gnu.org
Subject: [patch] [arm] Fix PR60169 - thumb1 far jump
Patch http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01229.html introduced
this ICE:
1. thumb1 estimate if far_jump is used based on function insn size 2.
During
reload, after stack layout finalized, it does reload_as_needed. It however
increases insn size that changes estimation result of far_jump, which in
return need to save lr and change stack layout again. While there is not
chance to change, GCC crashes.
Solution:
Do not change estimation result of far_jump if reload_in_progress or
reload_completed is true.
Not likely need to fix lra according to Vlad:
http://gcc.gnu.org/ml/gcc/2014-02/msg00355.html
ChangeLog:
* config/arm/arm.c (thumb_far_jump_used_p): Don't change
if reload in progress or completed.
* gcc.target/arm/thumb1-far-jump-3.c: New case.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index b562986..2cf362c 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -26255,6 +26255,11 @@ thumb_far_jump_used_p (void)
return 0;
}
+ /* We should not change far_jump_used during or after reload, as there is
+ no chance to change stack frame layout. */
+ if (reload_in_progress || reload_completed)
+return 0;
+
/* Check to see if the function contains a branch
insn with the far jump attribute set. */
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
diff --git a/gcc/testsuite/gcc.target/arm/thumb1-far-jump-3.c
b/gcc/testsuite/gcc.target/arm/thumb1-far-jump-3.c
new file mode 100644
index 000..90559ba
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/thumb1-far-jump-3.c
@@ -0,0 +1,108 @@
+/* Catch reload ICE on target thumb1 with far jump optimization.
+ * It is also a valid case for non-thumb1 target. */
+
+/* Add -mno-lra option as it is only reproducable with reload. It will
+ be removed after reload is completely removed. */
+/* { dg-options -mno-lra -fomit-frame-pointer } */
+/* { dg-do compile } */
+
+#define C 2
+#define A 4
+#define RGB (C | A)
+#define GRAY (A)
+
+typedef unsigned long uint_32;
+typedef unsigned char byte;
+typedef byte* bytep;
+
+typedef struct ss
+{
+ uint_32 w;
+ uint_32 r;
+ byte c;
+ byte b;
+ byte p;
+} info;
+
+typedef info * infop;
+
+void
+foo(infop info, bytep row)
+{
+ uint_32 iw = info-w;
+ if (info-c == RGB)
+ {
+ if (info-b == 8)
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save;
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save;
+ }
+ }
+
+ else
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save[2];
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save[0] = *(--sp);
+save[1] = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save[0];
+*(--dp) = save[1];
+ }
+ }
+ }
+ else if (info-c == GRAY)
+ {
+ if (info-b == 8)
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save;
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save;
+ }
+ }
+ else
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save[2];
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save[0] = *(--sp);
+save[1] = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save[0];
+*(--dp) = save[1];
+ }
+ }
+ }
+}
[PATCH] [libgcc,arm] Fix PR 60166 - NAN fraction bits
This patch is a mirror copy from approved patch in glibc: http://sourceware.org/ml/libc-alpha/2014-02/msg00741.html OK to trunk, 4.8 and 4.7? ChangeLog.libgcc: * config/arm/sfp-machine.h (_FP_NANFRAC_H, _FP_NANFRAC_S, _FP_NANFRAC_D, _FP_NANFRAC_Q): Set to zero. diff --git a/libgcc/config/arm/sfp-machine.h b/libgcc/config/arm/sfp-machine.h index bb34895..8d45320 100644 --- a/libgcc/config/arm/sfp-machine.h +++ b/libgcc/config/arm/sfp-machine.h @@ -19,10 +19,12 @@ typedef int __gcc_CMPtype __attribute__ ((mode (__libgcc_cmp_return__))); #define _FP_DIV_MEAT_D(R,X,Y) _FP_DIV_MEAT_2_udiv(D,R,X,Y) #define _FP_DIV_MEAT_Q(R,X,Y) _FP_DIV_MEAT_4_udiv(Q,R,X,Y) -#define _FP_NANFRAC_H ((_FP_QNANBIT_H 1) - 1) -#define _FP_NANFRAC_S ((_FP_QNANBIT_S 1) - 1) -#define _FP_NANFRAC_D ((_FP_QNANBIT_D 1) - 1), -1 -#define _FP_NANFRAC_Q ((_FP_QNANBIT_Q 1) - 1), -1, -1, -1 +/* According to RTABI, QNAN is only with the most significant bit of the + significand set, and all other significand bits zero. */ +#define _FP_NANFRAC_H 0 +#define _FP_NANFRAC_S 0 +#define _FP_NANFRAC_D 0, 0 +#define _FP_NANFRAC_Q 0, 0, 0, 0 #define _FP_NANSIGN_H 0 #define _FP_NANSIGN_S 0 #define _FP_NANSIGN_D 0
RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8
Ping ^ 5 -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: 19 February 2014 17:22 To: 'ja...@redhat.com'; gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^ 4 -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Friday, February 14, 2014 9:58 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^3 These fixes are very important to 4.8 ARM embedded users, as they rely on strict volatile bitfields a lot. Please let them in 4.8. -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Saturday, February 08, 2014 10:42 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^ 2 OK to 4.8? -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Monday, January 20, 2014 10:47 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Thursday, January 16, 2014 16:28 To: gcc-patches@gcc.gnu.org Subject: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 4.8 has a number of strict-volatile-bitfields issues that can be fixed by following patches. trunk@205899, 205898, 205897, 205896, 203003 Tested on x86_64 and arm without regression. OK to 4.8? 2013-09-28 Sandra Loosemore san...@codesourcery.com gcc/ * expr.h (extract_bit_field): Remove packedp parameter. * expmed.c (extract_fixed_bit_field): Remove packedp parameter from forward declaration. (store_split_bit_field): Remove packedp arg from calls to extract_fixed_bit_field. (extract_bit_field_1): Remove packedp parameter and packedp argument from recursive calls and calls to extract_fixed_bit_field. (extract_bit_field): Remove packedp parameter and corresponding arg to extract_bit_field_1. (extract_fixed_bit_field): Remove packedp parameter. Remove code to issue warnings. (extract_split_bit_field): Remove packedp arg from call to extract_fixed_bit_field. * expr.c (emit_group_load_1): Adjust calls to extract_bit_field. (copy_blkmode_from_reg): Likewise. (copy_blkmode_to_reg): Likewise. (read_complex_part): Likewise. (store_field): Likewise. (expand_expr_real_1): Likewise. * calls.c (store_unaligned_arguments_into_pseudos): Adjust call to extract_bit_field. * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust call to extract_bit_field. * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust call to extract_bit_field. * doc/invoke.texi (Code Gen Options): Remove mention of warnings and special packedp behavior from -fstrict-volatile-bitfields documentation. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de * expr.c (expand_assignment): Remove dependency on flag_strict_volatile_bitfields. Always set the memory access mode. (expand_expr_real_1): Likewise. 2013-12-11 Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 gcc/ * expmed.c (strict_volatile_bitfield_p): New function. (store_bit_field_1): Don't special-case strict volatile bitfields here. (store_bit_field): Handle strict volatile bitfields here instead. (store_fixed_bit_field): Don't special-case strict volatile bitfields here. (extract_bit_field_1): Don't special-case strict volatile bitfields here. (extract_bit_field): Handle strict volatile bitfields here instead. (extract_fixed_bit_field): Don't special-case strict volatile bitfields here. Simplify surrounding code to resemble that in store_fixed_bit_field. * doc/invoke.texi (Code Gen Options): Update -fstrict-volatile-bitfields description. gcc/testsuite/ * gcc.dg/pr23623.c: New test. * gcc.dg/pr48784-1.c: New test. * gcc.dg/pr48784-2.c: New test. * gcc.dg/pr56341-1.c: New test. * gcc.dg/pr56341-2.c: New test. * gcc.dg/pr56997-1.c: New test. * gcc.dg/pr56997-2.c: New test
RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8
Committed to ARM/embedded-4_8-branch Still pending to gcc-4_8-branch. -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: 27 February 2014 13:53 To: 'ja...@redhat.com'; 'gcc-patches@gcc.gnu.org' Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^ 5 -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: 19 February 2014 17:22 To: 'ja...@redhat.com'; gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^ 4 -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Friday, February 14, 2014 9:58 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^3 These fixes are very important to 4.8 ARM embedded users, as they rely on strict volatile bitfields a lot. Please let them in 4.8. -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Saturday, February 08, 2014 10:42 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^ 2 OK to 4.8? -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Monday, January 20, 2014 10:47 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Thursday, January 16, 2014 16:28 To: gcc-patches@gcc.gnu.org Subject: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 4.8 has a number of strict-volatile-bitfields issues that can be fixed by following patches. trunk@205899, 205898, 205897, 205896, 203003 Tested on x86_64 and arm without regression. OK to 4.8? 2013-09-28 Sandra Loosemore san...@codesourcery.com gcc/ * expr.h (extract_bit_field): Remove packedp parameter. * expmed.c (extract_fixed_bit_field): Remove packedp parameter from forward declaration. (store_split_bit_field): Remove packedp arg from calls to extract_fixed_bit_field. (extract_bit_field_1): Remove packedp parameter and packedp argument from recursive calls and calls to extract_fixed_bit_field. (extract_bit_field): Remove packedp parameter and corresponding arg to extract_bit_field_1. (extract_fixed_bit_field): Remove packedp parameter. Remove code to issue warnings. (extract_split_bit_field): Remove packedp arg from call to extract_fixed_bit_field. * expr.c (emit_group_load_1): Adjust calls to extract_bit_field. (copy_blkmode_from_reg): Likewise. (copy_blkmode_to_reg): Likewise. (read_complex_part): Likewise. (store_field): Likewise. (expand_expr_real_1): Likewise. * calls.c (store_unaligned_arguments_into_pseudos): Adjust call to extract_bit_field. * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust call to extract_bit_field. * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust call to extract_bit_field. * doc/invoke.texi (Code Gen Options): Remove mention of warnings and special packedp behavior from -fstrict-volatile-bitfields documentation. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de * expr.c (expand_assignment): Remove dependency on flag_strict_volatile_bitfields. Always set the memory access mode. (expand_expr_real_1): Likewise. 2013-12-11 Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 gcc/ * expmed.c (strict_volatile_bitfield_p): New function. (store_bit_field_1): Don't special-case strict volatile bitfields here. (store_bit_field): Handle strict volatile bitfields here instead. (store_fixed_bit_field): Don't special-case strict volatile bitfields here. (extract_bit_field_1): Don't special-case strict volatile bitfields here. (extract_bit_field): Handle strict volatile bitfields here instead. (extract_fixed_bit_field): Don't special-case strict volatile bitfields here. Simplify surrounding code to resemble that in store_fixed_bit_field. * doc/invoke.texi (Code Gen
[patch] [arm] Fix PR60169 - thumb1 far jump
Patch http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01229.html introduced
this ICE:
1. thumb1 estimate if far_jump is used based on function insn size
2. During reload, after stack layout finalized, it does reload_as_needed. It
however increases insn size that changes estimation result of far_jump,
which in return need to save lr and change stack layout again. While there
is not chance to change, GCC crashes.
Solution:
Do not change estimation result of far_jump if reload_in_progress or
reload_completed is true.
Not likely need to fix lra according to Vlad:
http://gcc.gnu.org/ml/gcc/2014-02/msg00355.html
ChangeLog:
* config/arm/arm.c (thumb_far_jump_used_p): Don't change
if reload in progress or completed.
* gcc.target/arm/thumb1-far-jump-3.c: New case.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index b562986..2cf362c 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -26255,6 +26255,11 @@ thumb_far_jump_used_p (void)
return 0;
}
+ /* We should not change far_jump_used during or after reload, as there is
+ no chance to change stack frame layout. */
+ if (reload_in_progress || reload_completed)
+return 0;
+
/* Check to see if the function contains a branch
insn with the far jump attribute set. */
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
diff --git a/gcc/testsuite/gcc.target/arm/thumb1-far-jump-3.c
b/gcc/testsuite/gcc.target/arm/thumb1-far-jump-3.c
new file mode 100644
index 000..90559ba
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/thumb1-far-jump-3.c
@@ -0,0 +1,108 @@
+/* Catch reload ICE on target thumb1 with far jump optimization.
+ * It is also a valid case for non-thumb1 target. */
+
+/* Add -mno-lra option as it is only reproducable with reload. It will
+ be removed after reload is completely removed. */
+/* { dg-options -mno-lra -fomit-frame-pointer } */
+/* { dg-do compile } */
+
+#define C 2
+#define A 4
+#define RGB (C | A)
+#define GRAY (A)
+
+typedef unsigned long uint_32;
+typedef unsigned char byte;
+typedef byte* bytep;
+
+typedef struct ss
+{
+ uint_32 w;
+ uint_32 r;
+ byte c;
+ byte b;
+ byte p;
+} info;
+
+typedef info * infop;
+
+void
+foo(infop info, bytep row)
+{
+ uint_32 iw = info-w;
+ if (info-c == RGB)
+ {
+ if (info-b == 8)
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save;
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save;
+ }
+ }
+
+ else
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save[2];
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save[0] = *(--sp);
+save[1] = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save[0];
+*(--dp) = save[1];
+ }
+ }
+ }
+ else if (info-c == GRAY)
+ {
+ if (info-b == 8)
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save;
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save;
+ }
+ }
+ else
+ {
+ bytep sp = row + info-r;
+ bytep dp = sp;
+ byte save[2];
+ uint_32 i;
+
+ for (i = 0; i iw; i++)
+ {
+save[0] = *(--sp);
+save[1] = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = *(--sp);
+*(--dp) = save[0];
+*(--dp) = save[1];
+ }
+ }
+ }
+}
RE: [patch] [arm] Fix PR60169 - thumb1 far jump
OK to trunk and 4.8? -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: 2014年2月21日 19:32 To: gcc-patches@gcc.gnu.org Subject: [patch] [arm] Fix PR60169 - thumb1 far jump Patch http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01229.html introduced this ICE: 1. thumb1 estimate if far_jump is used based on function insn size 2. During reload, after stack layout finalized, it does reload_as_needed. It however increases insn size that changes estimation result of far_jump, which in return need to save lr and change stack layout again. While there is not chance to change, GCC crashes. Solution: Do not change estimation result of far_jump if reload_in_progress or reload_completed is true. Not likely need to fix lra according to Vlad: http://gcc.gnu.org/ml/gcc/2014-02/msg00355.html ChangeLog: * config/arm/arm.c (thumb_far_jump_used_p): Don't change if reload in progress or completed. * gcc.target/arm/thumb1-far-jump-3.c: New case.
Stack layout change during lra
Vlad,
When fixing PR60169, I found that reload fail to assert
verify_initial_elim_offsets ()
if (insns_need_reload != 0 || something_needs_elimination
|| something_needs_operands_changed)
{
HOST_WIDE_INT old_frame_size = get_frame_size ();
reload_as_needed (global);
gcc_assert (old_frame_size == get_frame_size ());
gcc_assert (verify_initial_elim_offsets ());
}
The reason is that stack layout changes during reload_as_needed as a result
of a thumb1 backend heuristic.
I have a patch to make sure the heuristic doesn't change stack layout during
and after reload, and the assertion disappeared. However, I'm not sure if it
will also be a problem in lra. Here is the question more specific:
Is that any chance during lra_in_progress that: stack layout can no longer
be altered, but insns can still be added or changed?
Thanks,
Joey
RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8
Ping ^ 4 -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Friday, February 14, 2014 9:58 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^3 These fixes are very important to 4.8 ARM embedded users, as they rely on strict volatile bitfields a lot. Please let them in 4.8. -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Saturday, February 08, 2014 10:42 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^ 2 OK to 4.8? -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Monday, January 20, 2014 10:47 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Thursday, January 16, 2014 16:28 To: gcc-patches@gcc.gnu.org Subject: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 4.8 has a number of strict-volatile-bitfields issues that can be fixed by following patches. trunk@205899, 205898, 205897, 205896, 203003 Tested on x86_64 and arm without regression. OK to 4.8? 2013-09-28 Sandra Loosemore san...@codesourcery.com gcc/ * expr.h (extract_bit_field): Remove packedp parameter. * expmed.c (extract_fixed_bit_field): Remove packedp parameter from forward declaration. (store_split_bit_field): Remove packedp arg from calls to extract_fixed_bit_field. (extract_bit_field_1): Remove packedp parameter and packedp argument from recursive calls and calls to extract_fixed_bit_field. (extract_bit_field): Remove packedp parameter and corresponding arg to extract_bit_field_1. (extract_fixed_bit_field): Remove packedp parameter. Remove code to issue warnings. (extract_split_bit_field): Remove packedp arg from call to extract_fixed_bit_field. * expr.c (emit_group_load_1): Adjust calls to extract_bit_field. (copy_blkmode_from_reg): Likewise. (copy_blkmode_to_reg): Likewise. (read_complex_part): Likewise. (store_field): Likewise. (expand_expr_real_1): Likewise. * calls.c (store_unaligned_arguments_into_pseudos): Adjust call to extract_bit_field. * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust call to extract_bit_field. * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust call to extract_bit_field. * doc/invoke.texi (Code Gen Options): Remove mention of warnings and special packedp behavior from -fstrict-volatile-bitfields documentation. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de * expr.c (expand_assignment): Remove dependency on flag_strict_volatile_bitfields. Always set the memory access mode. (expand_expr_real_1): Likewise. 2013-12-11 Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 gcc/ * expmed.c (strict_volatile_bitfield_p): New function. (store_bit_field_1): Don't special-case strict volatile bitfields here. (store_bit_field): Handle strict volatile bitfields here instead. (store_fixed_bit_field): Don't special-case strict volatile bitfields here. (extract_bit_field_1): Don't special-case strict volatile bitfields here. (extract_bit_field): Handle strict volatile bitfields here instead. (extract_fixed_bit_field): Don't special-case strict volatile bitfields here. Simplify surrounding code to resemble that in store_fixed_bit_field. * doc/invoke.texi (Code Gen Options): Update -fstrict-volatile-bitfields description. gcc/testsuite/ * gcc.dg/pr23623.c: New test. * gcc.dg/pr48784-1.c: New test. * gcc.dg/pr48784-2.c: New test. * gcc.dg/pr56341-1.c: New test. * gcc.dg/pr56341-2.c: New test. * gcc.dg/pr56997-1.c: New test. * gcc.dg/pr56997-2.c: New test. * gcc.dg/pr56997-3.c: New test. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 * expmed.c (strict_volatile_bitfield_p): Add bitregion_start
[patch] Shorten Windows path
Max length of path on Windows is 255, which is easy to exceed in a
complicated project. Ultimate solution may be complex but canonizing the
path and skipping the ..s in path is helpful.
Relative discussion in gcc-patches:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00582.html
OK to trunk stage 1?
ChangeLog.libcpp:
* files.c (find_file_in_dir): Always try to shorten for DOS.
diff --git a/libcpp/files.c b/libcpp/files.c
index 7e88778..9dcc71f 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -386,9 +386,18 @@ find_file_in_dir (cpp_reader *pfile, _cpp_file *file,
bool *invalid_pch)
hashval_t hv;
char *copy;
void **pp;
+ bool do_canonical;
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ /* For DOS based file system, we always try to shorten file path
+ * to as it has a shorter constraint on max path length. */
+ do_canonical = true;
+#else
/* We try to canonicalize system headers. */
- if (CPP_OPTION (pfile, canonical_system_headers) file-dir-sysp)
+ do_canonical = (CPP_OPTION (pfile, canonical_system_headers)
+ file-dir-sysp);
+#endif
+ if ( do_canonical )
{
char * canonical_path = maybe_shorter_path (path);
if (canonical_path)
max_path_joey-140109-1.patch
Description: Binary data
RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8
Ping ^3 These fixes are very important to 4.8 ARM embedded users, as they rely on strict volatile bitfields a lot. Please let them in 4.8. -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Saturday, February 08, 2014 10:42 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping ^ 2 OK to 4.8? -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Monday, January 20, 2014 10:47 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Thursday, January 16, 2014 16:28 To: gcc-patches@gcc.gnu.org Subject: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 4.8 has a number of strict-volatile-bitfields issues that can be fixed by following patches. trunk@205899, 205898, 205897, 205896, 203003 Tested on x86_64 and arm without regression. OK to 4.8? 2013-09-28 Sandra Loosemore san...@codesourcery.com gcc/ * expr.h (extract_bit_field): Remove packedp parameter. * expmed.c (extract_fixed_bit_field): Remove packedp parameter from forward declaration. (store_split_bit_field): Remove packedp arg from calls to extract_fixed_bit_field. (extract_bit_field_1): Remove packedp parameter and packedp argument from recursive calls and calls to extract_fixed_bit_field. (extract_bit_field): Remove packedp parameter and corresponding arg to extract_bit_field_1. (extract_fixed_bit_field): Remove packedp parameter. Remove code to issue warnings. (extract_split_bit_field): Remove packedp arg from call to extract_fixed_bit_field. * expr.c (emit_group_load_1): Adjust calls to extract_bit_field. (copy_blkmode_from_reg): Likewise. (copy_blkmode_to_reg): Likewise. (read_complex_part): Likewise. (store_field): Likewise. (expand_expr_real_1): Likewise. * calls.c (store_unaligned_arguments_into_pseudos): Adjust call to extract_bit_field. * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust call to extract_bit_field. * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust call to extract_bit_field. * doc/invoke.texi (Code Gen Options): Remove mention of warnings and special packedp behavior from -fstrict-volatile-bitfields documentation. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de * expr.c (expand_assignment): Remove dependency on flag_strict_volatile_bitfields. Always set the memory access mode. (expand_expr_real_1): Likewise. 2013-12-11 Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 gcc/ * expmed.c (strict_volatile_bitfield_p): New function. (store_bit_field_1): Don't special-case strict volatile bitfields here. (store_bit_field): Handle strict volatile bitfields here instead. (store_fixed_bit_field): Don't special-case strict volatile bitfields here. (extract_bit_field_1): Don't special-case strict volatile bitfields here. (extract_bit_field): Handle strict volatile bitfields here instead. (extract_fixed_bit_field): Don't special-case strict volatile bitfields here. Simplify surrounding code to resemble that in store_fixed_bit_field. * doc/invoke.texi (Code Gen Options): Update -fstrict-volatile-bitfields description. gcc/testsuite/ * gcc.dg/pr23623.c: New test. * gcc.dg/pr48784-1.c: New test. * gcc.dg/pr48784-2.c: New test. * gcc.dg/pr56341-1.c: New test. * gcc.dg/pr56341-2.c: New test. * gcc.dg/pr56997-1.c: New test. * gcc.dg/pr56997-2.c: New test. * gcc.dg/pr56997-3.c: New test. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 * expmed.c (strict_volatile_bitfield_p): Add bitregion_start and bitregion_end parameters. Test for compliance with C++ memory model. (store_bit_field): Adjust call to strict_volatile_bitfield_p. Add fallback logic for cases where -fstrict-volatile-bitfields is supposed to apply, but cannot. (extract_bit_field): Likewise. Use narrow_bit_field_mem
RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8
Ping ^ 2 OK to 4.8? -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Monday, January 20, 2014 10:47 To: gcc-patches@gcc.gnu.org Subject: RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 Ping -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Thursday, January 16, 2014 16:28 To: gcc-patches@gcc.gnu.org Subject: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 4.8 has a number of strict-volatile-bitfields issues that can be fixed by following patches. trunk@205899, 205898, 205897, 205896, 203003 Tested on x86_64 and arm without regression. OK to 4.8? 2013-09-28 Sandra Loosemore san...@codesourcery.com gcc/ * expr.h (extract_bit_field): Remove packedp parameter. * expmed.c (extract_fixed_bit_field): Remove packedp parameter from forward declaration. (store_split_bit_field): Remove packedp arg from calls to extract_fixed_bit_field. (extract_bit_field_1): Remove packedp parameter and packedp argument from recursive calls and calls to extract_fixed_bit_field. (extract_bit_field): Remove packedp parameter and corresponding arg to extract_bit_field_1. (extract_fixed_bit_field): Remove packedp parameter. Remove code to issue warnings. (extract_split_bit_field): Remove packedp arg from call to extract_fixed_bit_field. * expr.c (emit_group_load_1): Adjust calls to extract_bit_field. (copy_blkmode_from_reg): Likewise. (copy_blkmode_to_reg): Likewise. (read_complex_part): Likewise. (store_field): Likewise. (expand_expr_real_1): Likewise. * calls.c (store_unaligned_arguments_into_pseudos): Adjust call to extract_bit_field. * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust call to extract_bit_field. * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust call to extract_bit_field. * doc/invoke.texi (Code Gen Options): Remove mention of warnings and special packedp behavior from -fstrict-volatile-bitfields documentation. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de * expr.c (expand_assignment): Remove dependency on flag_strict_volatile_bitfields. Always set the memory access mode. (expand_expr_real_1): Likewise. 2013-12-11 Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 gcc/ * expmed.c (strict_volatile_bitfield_p): New function. (store_bit_field_1): Don't special-case strict volatile bitfields here. (store_bit_field): Handle strict volatile bitfields here instead. (store_fixed_bit_field): Don't special-case strict volatile bitfields here. (extract_bit_field_1): Don't special-case strict volatile bitfields here. (extract_bit_field): Handle strict volatile bitfields here instead. (extract_fixed_bit_field): Don't special-case strict volatile bitfields here. Simplify surrounding code to resemble that in store_fixed_bit_field. * doc/invoke.texi (Code Gen Options): Update -fstrict-volatile-bitfields description. gcc/testsuite/ * gcc.dg/pr23623.c: New test. * gcc.dg/pr48784-1.c: New test. * gcc.dg/pr48784-2.c: New test. * gcc.dg/pr56341-1.c: New test. * gcc.dg/pr56341-2.c: New test. * gcc.dg/pr56997-1.c: New test. * gcc.dg/pr56997-2.c: New test. * gcc.dg/pr56997-3.c: New test. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 * expmed.c (strict_volatile_bitfield_p): Add bitregion_start and bitregion_end parameters. Test for compliance with C++ memory model. (store_bit_field): Adjust call to strict_volatile_bitfield_p. Add fallback logic for cases where -fstrict-volatile-bitfields is supposed to apply, but cannot. (extract_bit_field): Likewise. Use narrow_bit_field_mem and extract_fixed_bit_field_1 to do the extraction. (extract_fixed_bit_field): Revert to previous mode selection algorithm. Call extract_fixed_bit_field_1 to do the real work. (extract_fixed_bit_field_1): New function. testsuite: * gcc.dg/pr23623.c: Update to test interaction with C++ memory model. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de PR middle-end/59134 * expmed.c
RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8
Ping -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Thursday, January 16, 2014 16:28 To: gcc-patches@gcc.gnu.org Subject: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8 4.8 has a number of strict-volatile-bitfields issues that can be fixed by following patches. trunk@205899, 205898, 205897, 205896, 203003 Tested on x86_64 and arm without regression. OK to 4.8? 2013-09-28 Sandra Loosemore san...@codesourcery.com gcc/ * expr.h (extract_bit_field): Remove packedp parameter. * expmed.c (extract_fixed_bit_field): Remove packedp parameter from forward declaration. (store_split_bit_field): Remove packedp arg from calls to extract_fixed_bit_field. (extract_bit_field_1): Remove packedp parameter and packedp argument from recursive calls and calls to extract_fixed_bit_field. (extract_bit_field): Remove packedp parameter and corresponding arg to extract_bit_field_1. (extract_fixed_bit_field): Remove packedp parameter. Remove code to issue warnings. (extract_split_bit_field): Remove packedp arg from call to extract_fixed_bit_field. * expr.c (emit_group_load_1): Adjust calls to extract_bit_field. (copy_blkmode_from_reg): Likewise. (copy_blkmode_to_reg): Likewise. (read_complex_part): Likewise. (store_field): Likewise. (expand_expr_real_1): Likewise. * calls.c (store_unaligned_arguments_into_pseudos): Adjust call to extract_bit_field. * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust call to extract_bit_field. * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust call to extract_bit_field. * doc/invoke.texi (Code Gen Options): Remove mention of warnings and special packedp behavior from -fstrict-volatile-bitfields documentation. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de * expr.c (expand_assignment): Remove dependency on flag_strict_volatile_bitfields. Always set the memory access mode. (expand_expr_real_1): Likewise. 2013-12-11 Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 gcc/ * expmed.c (strict_volatile_bitfield_p): New function. (store_bit_field_1): Don't special-case strict volatile bitfields here. (store_bit_field): Handle strict volatile bitfields here instead. (store_fixed_bit_field): Don't special-case strict volatile bitfields here. (extract_bit_field_1): Don't special-case strict volatile bitfields here. (extract_bit_field): Handle strict volatile bitfields here instead. (extract_fixed_bit_field): Don't special-case strict volatile bitfields here. Simplify surrounding code to resemble that in store_fixed_bit_field. * doc/invoke.texi (Code Gen Options): Update -fstrict-volatile-bitfields description. gcc/testsuite/ * gcc.dg/pr23623.c: New test. * gcc.dg/pr48784-1.c: New test. * gcc.dg/pr48784-2.c: New test. * gcc.dg/pr56341-1.c: New test. * gcc.dg/pr56341-2.c: New test. * gcc.dg/pr56997-1.c: New test. * gcc.dg/pr56997-2.c: New test. * gcc.dg/pr56997-3.c: New test. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 * expmed.c (strict_volatile_bitfield_p): Add bitregion_start and bitregion_end parameters. Test for compliance with C++ memory model. (store_bit_field): Adjust call to strict_volatile_bitfield_p. Add fallback logic for cases where -fstrict-volatile-bitfields is supposed to apply, but cannot. (extract_bit_field): Likewise. Use narrow_bit_field_mem and extract_fixed_bit_field_1 to do the extraction. (extract_fixed_bit_field): Revert to previous mode selection algorithm. Call extract_fixed_bit_field_1 to do the real work. (extract_fixed_bit_field_1): New function. testsuite: * gcc.dg/pr23623.c: Update to test interaction with C++ memory model. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de PR middle-end/59134 * expmed.c (store_bit_field): Use narrow_bit_field_mem and store_fixed_bit_field_1 for -fstrict-volatile-bitfields. (store_fixed_bit_field): Split up. Call store_fixed_bit_field_1 to do the real work. (store_fixed_bit_field_1): New function. (store_split_bit_field): Limit the unit size to the memory
[PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8
4.8 has a number of strict-volatile-bitfields issues that can be fixed by following patches. trunk@205899, 205898, 205897, 205896, 203003 Tested on x86_64 and arm without regression. OK to 4.8? 2013-09-28 Sandra Loosemore san...@codesourcery.com gcc/ * expr.h (extract_bit_field): Remove packedp parameter. * expmed.c (extract_fixed_bit_field): Remove packedp parameter from forward declaration. (store_split_bit_field): Remove packedp arg from calls to extract_fixed_bit_field. (extract_bit_field_1): Remove packedp parameter and packedp argument from recursive calls and calls to extract_fixed_bit_field. (extract_bit_field): Remove packedp parameter and corresponding arg to extract_bit_field_1. (extract_fixed_bit_field): Remove packedp parameter. Remove code to issue warnings. (extract_split_bit_field): Remove packedp arg from call to extract_fixed_bit_field. * expr.c (emit_group_load_1): Adjust calls to extract_bit_field. (copy_blkmode_from_reg): Likewise. (copy_blkmode_to_reg): Likewise. (read_complex_part): Likewise. (store_field): Likewise. (expand_expr_real_1): Likewise. * calls.c (store_unaligned_arguments_into_pseudos): Adjust call to extract_bit_field. * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust call to extract_bit_field. * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust call to extract_bit_field. * doc/invoke.texi (Code Gen Options): Remove mention of warnings and special packedp behavior from -fstrict-volatile-bitfields documentation. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de * expr.c (expand_assignment): Remove dependency on flag_strict_volatile_bitfields. Always set the memory access mode. (expand_expr_real_1): Likewise. 2013-12-11 Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 gcc/ * expmed.c (strict_volatile_bitfield_p): New function. (store_bit_field_1): Don't special-case strict volatile bitfields here. (store_bit_field): Handle strict volatile bitfields here instead. (store_fixed_bit_field): Don't special-case strict volatile bitfields here. (extract_bit_field_1): Don't special-case strict volatile bitfields here. (extract_bit_field): Handle strict volatile bitfields here instead. (extract_fixed_bit_field): Don't special-case strict volatile bitfields here. Simplify surrounding code to resemble that in store_fixed_bit_field. * doc/invoke.texi (Code Gen Options): Update -fstrict-volatile-bitfields description. gcc/testsuite/ * gcc.dg/pr23623.c: New test. * gcc.dg/pr48784-1.c: New test. * gcc.dg/pr48784-2.c: New test. * gcc.dg/pr56341-1.c: New test. * gcc.dg/pr56341-2.c: New test. * gcc.dg/pr56997-1.c: New test. * gcc.dg/pr56997-2.c: New test. * gcc.dg/pr56997-3.c: New test. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de Sandra Loosemore san...@codesourcery.com PR middle-end/23623 PR middle-end/48784 PR middle-end/56341 PR middle-end/56997 * expmed.c (strict_volatile_bitfield_p): Add bitregion_start and bitregion_end parameters. Test for compliance with C++ memory model. (store_bit_field): Adjust call to strict_volatile_bitfield_p. Add fallback logic for cases where -fstrict-volatile-bitfields is supposed to apply, but cannot. (extract_bit_field): Likewise. Use narrow_bit_field_mem and extract_fixed_bit_field_1 to do the extraction. (extract_fixed_bit_field): Revert to previous mode selection algorithm. Call extract_fixed_bit_field_1 to do the real work. (extract_fixed_bit_field_1): New function. testsuite: * gcc.dg/pr23623.c: Update to test interaction with C++ memory model. 2013-12-11 Bernd Edlinger bernd.edlin...@hotmail.de PR middle-end/59134 * expmed.c (store_bit_field): Use narrow_bit_field_mem and store_fixed_bit_field_1 for -fstrict-volatile-bitfields. (store_fixed_bit_field): Split up. Call store_fixed_bit_field_1 to do the real work. (store_fixed_bit_field_1): New function. (store_split_bit_field): Limit the unit size to the memory mode size, to prevent recursion. testsuite: * gcc.c-torture/compile/pr59134.c: New test. * gnat.dg/misaligned_volatile.adb: New test. all.diff Description: Binary data
Re: [GCC, ARM] Backport trunk patch to 4.8 to reclassify ARM preload insn
ChangeLog is messed up with other one. On Thu, Jan 16, 2014 at 3:33 PM, Terry Guo terry@arm.com wrote: Hi, Current 4.8 branch will assign alu_reg attribute to the type of arm preload insn, which is clearly wrong. The attached patch intends to back port trunk patch to reclassify the type attribute as load1. With this back port, the 4.8 bug PR59826 can be fixed too. Tested with gcc regression test on QEMU Cortex-M3, no new regressions. Is it OK to back port this patch http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00322.html? BR, Terry
Re: Still fails with strict-volatile-bitfields
Bernd,
If that's the case, can you please firstly fix invoke.texi where the
behavior of strict-volatile-bitfields is described? At least my
interpretation of current doc doesn't explain the behavior of the case
we are discussing. Also it should be a generic definition rather than
target specific one.
A related issue is that how would we expect users to build their
original code with volatile bitfields usage? From the approach I
understand they have to explicitly add -fstrict-volatile-bitfields for
4.9 (by default it is disabled now), and probably
-fstrict-volatile-bitfields=aapcs (to allow C++ memory model conflict)
later. Neither of them are obvious to users. How about a configure
option to set default?
Thanks,
Joey
On Fri, Jan 10, 2014 at 10:20 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
On Fri, 10 Jan 2014 14:45:12, Richard Biener wrote:
On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote:
On 10/01/14 08:49, Bernd Edlinger wrote:
On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote:
On 09/01/14 08:26, Bernd Edlinger wrote:
Hi,
On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote:
Sandra, Bernd,
Can you take a look at
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734
It seems a siimple case still doesn't work as expected. Did I miss
anything?
Thanks,
Joey
Yes,
this is a major case where the C++ memory model is
in conflict with AAPCS.
Does the compiler warn about this? And if so, is the warning on by
default? I think it's essential that we don't have quiet changes in
behaviour without such warnings.
R.
No. This example was working in 4.6 and broken in 4.7 and 4.8.
Well, 4.7 should have warned about that.
4.9 does simply not change that, because it would violate the C++
memory model. Maybe that should go into release notes.
That's no excuse for not generating a warning at compile time when the
situation is encountered. Users of this feature are experiencing a
quiet change of behaviour and that's unacceptable, even if the compiler
is doing what it was intended to do; that doesn't necessarily match the
user's expectations. There should always be a way to rewrite the C11
intended semantics in a way that doesn't violate the strict volatile
bitfields semantics.
Hmm...
You mean we should have a (ugly) warning enabled by default in 4.9 (at
expmed.c)
if a bit-field access would be perfectly aligned and so, but is in conflict
with the
C++ memory model, and -fstrict-volatile-bitfields is in effect.
Maybe only once per compilation?
I'd say you want a warning for the structure declaration instead
Accesses to XYZ will not follow AACPS due to C++ memory model
constraints.
Richard.
Yes, that would be the way how we wanted to implement the
-Wportable-volatility warning, except that it would be on by default
if -fstrict-volatile-bitfields is in effect.
And it would not only warn if the member is declared volatile,
because the structure can be declared volatile later.
Except that I did not even start to implement it that way, that
would be quite late for 4.9 now?
Bernd.
At the same time we had all kinds of invalid code generation,
starting at 4.6, especially with packed structures in C and Ada(!),
(writing not all bits, and using unaligned accesses)
and that is fixed in 4.9.
I'm not worried about packed structures. You can't sensibly implement
the strict volatile bitfields rules when things aren't aligned.
R.
Bernd.
You can get the expected code, by changing the struct
like this:
struct str {
volatile unsigned f1: 8;
unsigned dummy:24;
};
If it is written this way the C++ memory model allows
QImode, HImode, SImode. And -fstrict-volatile-bitfields
demands SImode, so the conflict is resolved. This dummy
member makes only a difference on the C level, and is
completely invisible in the generated code.
If -fstrict-volatile-bitfields is now given, we use SImode,
if -fno-strict-volatile-bitfields is given, we give GCC the
freedom to choose the access mode, maybe QImode if that is
faster.
In the _very_ difficult process to find an solution
that seems to be acceptable to all maintainers, we came to
the solution, that we need to adhere to the C++ memory
model by default. And we may not change the default
setting of -fstruct-volatile-bitfields at the same time!
As a future extension we discussed the possibility
to add a new option -fstrict-volatile-bitfields=aapcs
that explicitly allows us to break the C++ memory model.
But I did not yet try to implement this, as I feel that
would certainly not be accepted as we are in Phase3 now.
As another future extension there was the discussion
about the -Wportable-volatility warning, which I see now
as a warning that analyzes the structure layout and
warns about any structures that are not well-formed,
in the sense, that a bit-field fails to define all
bits of the container.
Still fails with strict-volatile-bitfields
Sandra, Bernd, Can you take a look at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 It seems a siimple case still doesn't work as expected. Did I miss anything? Thanks, Joey
[PATCH] [doc] Update plugin doc
Update plugin document after switching to C++, also make it more friendly to
cross-build.
ChangeLog:
2014-01-08 Joey Ye joey...@arm.com
doc/plugin.texi (Building GCC plugins): Update to C++.
OK to trunk?
diff --git a/gcc/doc/plugins.texi b/gcc/doc/plugins.texi
index fc2d754..e668de6 100644
--- a/gcc/doc/plugins.texi
+++ b/gcc/doc/plugins.texi
@@ -465,18 +465,18 @@ integer numbers, so a plugin could ensure it is built
for GCC 4.7 with
The following GNU Makefile excerpt shows how to build a simple plugin:
@smallexample
-GCC=gcc
-PLUGIN_SOURCE_FILES= plugin1.c plugin2.c
-PLUGIN_OBJECT_FILES= $(patsubst %.c,%.o,$(PLUGIN_SOURCE_FILES))
-GCCPLUGINS_DIR:= $(shell $(GCC) -print-file-name=plugin)
-CFLAGS+= -I$(GCCPLUGINS_DIR)/include -fPIC -O2
-
-plugin.so: $(PLUGIN_OBJECT_FILES)
- $(GCC) -shared $^ -o $@@
+HOST_GCC=g++
+TARGET_GCC=gcc
+PLUGIN_SOURCE_FILES= plugin1.c plugin2.cc
+GCCPLUGINS_DIR:= $(shell $(TARGET_GCC) -print-file-name=plugin)
+CXXFLAGS+= -I$(GCCPLUGINS_DIR)/include -fPIC -fno-rtti -O2
+
+plugin.so: $(PLUGIN_SOURCE_FILES)
+ $(HOST_GCC) -shared $(CXXFLAGS) $^ -o $@@
@end smallexample
-A single source file plugin may be built with @code{gcc -I`gcc
--print-file-name=plugin`/include -fPIC -shared -O2 plugin.c -o
+A single source file plugin may be built with @code{g++ -I`gcc
+-print-file-name=plugin`/include -fPIC -shared -fno-rtti -O2 plugin.c -o
plugin.so}, using backquote shell syntax to query the @file{plugin}
directory.
[patch] [plugin] Fix PR 59335 plugin build
Fix trunk plugin build by adding missing headers and remove headers no longer exist. Test passed: - arm-none-eabi build --enable-plugins - build test plugin - x86_64 bootstrap --enable-plugins OK to trunk? ChangeLog.gcc 2013-11-19 Joey Ye joey...@arm.com PR plugin/59335 * Makefile.in (tree-cfg.h, tree-into-ssa.h, fold-const.h, gimple-ssa.h, gimple-iterator.h, varasm.h, context.h): Add missing headers for plugin. (tree-flow.h, tree-flow-inline.h): Remove as they no longer exist. diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 459b1ba..55f1ace 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -882,13 +882,14 @@ TREE_CORE_H = tree-core.h coretypes.h all-tree.def tree.def \ $(VEC_H) treestruct.def $(HASHTAB_H) \ double-int.h alias.h $(SYMTAB_H) $(FLAGS_H) \ $(REAL_H) $(FIXED_VALUE_H) -TREE_H = tree.h $(TREE_CORE_H) tree-check.h +TREE_H = tree.h $(TREE_CORE_H) tree-check.h tree-cfg.h tree-into-ssa.h REGSET_H = regset.h $(BITMAP_H) hard-reg-set.h BASIC_BLOCK_H = basic-block.h $(PREDICT_H) $(VEC_H) $(FUNCTION_H) \ cfg-flags.def cfghooks.h GIMPLE_H = gimple.h gimple.def gsstruct.def pointer-set.h $(VEC_H) \ $(GGC_H) $(BASIC_BLOCK_H) $(TREE_H) tree-ssa-operands.h \ - tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H) is-a.h + tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H) is-a.h \ + gimple-ssa.h gimple-iterator.h GCOV_IO_H = gcov-io.h gcov-iov.h auto-host.h RECOG_H = recog.h EMIT_RTL_H = emit-rtl.h @@ -929,7 +930,7 @@ CPP_ID_DATA_H = $(CPPLIB_H) $(srcdir)/../libcpp/include/cpp-id-data.h CPP_INTERNAL_H = $(srcdir)/../libcpp/internal.h $(CPP_ID_DATA_H) TREE_DUMP_H = tree-dump.h $(SPLAY_TREE_H) $(DUMPFILE_H) TREE_PASS_H = tree-pass.h $(TIMEVAR_H) $(DUMPFILE_H) -TREE_FLOW_H = tree-flow.h tree-flow-inline.h tree-ssa-operands.h \ +TREE_FLOW_H = tree-ssa-operands.h \ $(BITMAP_H) sbitmap.h $(BASIC_BLOCK_H) $(GIMPLE_H) \ $(HASHTAB_H) $(CGRAPH_H) $(IPA_REFERENCE_H) \ tree-ssa-alias.h @@ -3119,7 +3120,7 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ cppdefault.h flags.h $(MD5_H) params.def params.h prefix.h tree-inline.h \ $(GIMPLE_PRETTY_PRINT_H) realmpfr.h \ $(IPA_PROP_H) $(TARGET_H) $(RTL_H) $(TM_P_H) $(CFGLOOP_H) $(EMIT_RTL_H) \ - version.h stringpool.h + version.h stringpool.h varasm.h fold-const.h $(CONTEXT_H) # generate the 'build fragment' b-header-vars s-header-vars: Makefilediff --git a/gcc/Makefile.in b/gcc/Makefile.in index 459b1ba..55f1ace 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -882,13 +882,14 @@ TREE_CORE_H = tree-core.h coretypes.h all-tree.def tree.def \ $(VEC_H) treestruct.def $(HASHTAB_H) \ double-int.h alias.h $(SYMTAB_H) $(FLAGS_H) \ $(REAL_H) $(FIXED_VALUE_H) -TREE_H = tree.h $(TREE_CORE_H) tree-check.h +TREE_H = tree.h $(TREE_CORE_H) tree-check.h tree-cfg.h tree-into-ssa.h REGSET_H = regset.h $(BITMAP_H) hard-reg-set.h BASIC_BLOCK_H = basic-block.h $(PREDICT_H) $(VEC_H) $(FUNCTION_H) \ cfg-flags.def cfghooks.h GIMPLE_H = gimple.h gimple.def gsstruct.def pointer-set.h $(VEC_H) \ $(GGC_H) $(BASIC_BLOCK_H) $(TREE_H) tree-ssa-operands.h \ - tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H) is-a.h + tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H) is-a.h \ + gimple-ssa.h gimple-iterator.h GCOV_IO_H = gcov-io.h gcov-iov.h auto-host.h RECOG_H = recog.h EMIT_RTL_H = emit-rtl.h @@ -929,7 +930,7 @@ CPP_ID_DATA_H = $(CPPLIB_H) $(srcdir)/../libcpp/include/cpp-id-data.h CPP_INTERNAL_H = $(srcdir)/../libcpp/internal.h $(CPP_ID_DATA_H) TREE_DUMP_H = tree-dump.h $(SPLAY_TREE_H) $(DUMPFILE_H) TREE_PASS_H = tree-pass.h $(TIMEVAR_H) $(DUMPFILE_H) -TREE_FLOW_H = tree-flow.h tree-flow-inline.h tree-ssa-operands.h \ +TREE_FLOW_H = tree-ssa-operands.h \ $(BITMAP_H) sbitmap.h $(BASIC_BLOCK_H) $(GIMPLE_H) \ $(HASHTAB_H) $(CGRAPH_H) $(IPA_REFERENCE_H) \ tree-ssa-alias.h @@ -3119,7 +3120,7 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ cppdefault.h flags.h $(MD5_H) params.def params.h prefix.h tree-inline.h \ $(GIMPLE_PRETTY_PRINT_H) realmpfr.h \ $(IPA_PROP_H) $(TARGET_H) $(RTL_H) $(TM_P_H) $(CFGLOOP_H) $(EMIT_RTL_H) \ - version.h stringpool.h + version.h stringpool.h varasm.h fold-const.h $(CONTEXT_H) # generate the 'build fragment' b-header-vars s-header-vars: Makefile
Re: [arm-embedded] Backport trunk thumb1 pic fix to embedded-4_8-branch
Terry, this is a bug fix to pic register. I feel it should also be in gcc-4_8-branch. - Joey On Wed, Nov 27, 2013 at 11:45 AM, Terry Guo terry@arm.com wrote: Hi, This patch back ported trunk fix at r205391 to arm/embedded-4_8-branch. BR, Terry gcc/ChangeLog.arm 2013-11-27 Terry Guo terry@arm.com Backport mainline r205391 2013-11-26 Terry Guo terry@arm.com * config/arm/arm.c (require_pic_register): Handle high pic base register for thumb-1. (arm_load_pic_register): Also initialize high pic base register. * doc/invoke.texi: Update documentation for option -mpic-register. gcc/testsuite/ChangeLog.arm 2013-11-27 Terry Guo terry@arm.com Backport mainline r205391 2013-11-26 Terry Guo terry@arm.com * gcc.target/arm/thumb1-pic-high-reg.c: New case. * gcc.target/arm/thumb1-pic-single-base.c: New case.
RE: [PATCH, libgcc] Disable JCR section when java is not enabled
Ping, as wasting 8 bytes of RAM isn't ignorable on embedded system. OK to trunk stage 1? -Original Message- From: Tom Tromey [mailto:tro...@redhat.com] Sent: Thursday, October 10, 2013 21:32 To: Jakub Jelinek Cc: Joey Ye; p...@bothner.com; a...@redhat.com; H.J. Lu; gcc-patches; 'Ian Lance Taylor' Subject: Re: [PATCH, libgcc] Disable JCR section when java is not enabled Jakub Given the state of gcj that it is now only rarely used and most Jakub people just use OpenJDK instead, wouldn't it be a good idea to Jakub just require that gcj code is linked using gcj driver or, if Jakub linked in any other driver, just using a special non-default Jakub option (-flink-jcr or similar), that would be automatically set Jakub by gcj driver, move this JCR stuff out of the normal crt* files Jakub and put it into crtjava*.o instead, and only link in if Jakub -flink-jcr is passed or gcj driver used? Or treat -lgcj as that Jakub magic switch? The irony of the situation is that this would require significantly more work than has gone into gcj in the past N years. Jakub Also, looking at crtstuff.c makes me wonder where are classes Jakub deregistered, there are only calls to _Jv_RegisterClasses, but Jakub never to to deregistration, wonder what happens if you dlclose a Jakub shared library with registered classes. I think we never implemented class GC for compiled classes, though it's hard to remember. Tom jcr_disable_non_java-130910.patch Description: Binary data
RE: [patch] [arm] ARM Cortex-M3/M4 tuning
Sorry about this. I should have run x86 make check.
- Joey
-Original Message-
From: Richard Biener [mailto:richard.guent...@gmail.com]
Sent: Thursday, November 14, 2013 22:16
To: H.J. Lu
Cc: Joey Ye; Janis Johnson; GCC Patches; Ramana Radhakrishnan
Subject: Re: [patch] [arm] ARM Cortex-M3/M4 tuning
On Thu, Nov 14, 2013 at 1:35 PM, H.J. Lu hjl.to...@gmail.com wrote:
On Thu, Nov 14, 2013 at 2:24 AM, Joey Ye joey...@arm.com wrote:
In mainline and arm/embedded-4_8-branch now.
-Original Message-
From: Janis Johnson [mailto:janis_john...@mentor.com]
Sent: Thursday, November 14, 2013 1:45
To: Joey Ye; jani...@codesourcery.com
Cc: gcc-patches@gcc.gnu.org; Ramana Radhakrishnan
Subject: Re: [patch] [arm] ARM Cortex-M3/M4 tuning
On 11/12/2013 10:20 PM, Joey Ye wrote:
Janis, can you please take a look at test case changes.
Thanks,
Joey
They look fine.
I got
ERROR: (DejaGnu) proc { scan-tree-dump-times Threaded 1 vrp1 } ||
{ arm_cortex_m } does not exist.
on Linux/x86-64.
me too, this stops testing tree-ssa.exp at this point which is bad. FIxed
as
attached.
Richard.
--
H.J.
RE: [patch] [arm] New option for PIC offset unfixed
-Original Message-
From: Richard Earnshaw
Sent: Thursday, November 14, 2013 0:57
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [patch] [arm] New option for PIC offset unfixed
So you are suggesting change like this:
+ Target Report Var(arm_pic_data_is_text_relative) Init(-1)
+ if (arm_pic_data_is_text_relative 0 TARGET_VXWORKS_RTP)
+ arm_pic_data_is_text_relative = 0;
+ else
+ arm_pic_data_is_text_relative = 1;
No, use the global_options_set structure to find out if the user has set
the
value.
Thank pointing this out. Here is the latest patch with global_options_set
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7757e86..3af8293 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2504,6 +2504,10 @@ arm_option_override (void)
arm_pic_register = pic_register;
}
+ if (TARGET_VXWORKS_RTP
+ !global_options_set.x_arm_pic_data_is_text_relative)
+arm_pic_data_is_text_relative = 0;
+
/* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores. */
if (fix_cm3_ldrd == 2)
{
@@ -6020,7 +6024,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode,
rtx reg)
|| (GET_CODE (orig) == SYMBOL_REF
SYMBOL_REF_LOCAL_P (orig)))
NEED_GOT_RELOC
- !TARGET_VXWORKS_RTP)
+ arm_pic_data_is_text_relative)
insn = arm_pic_static_addr (orig, reg);
else
{
@@ -21498,7 +21502,7 @@ arm_assemble_integer (rtx x, unsigned int size, int
aligned_p)
{
/* See legitimize_pic_address for an explanation of the
TARGET_VXWORKS_RTP check. */
- if (TARGET_VXWORKS_RTP
+ if (!arm_pic_data_is_text_relative
|| (GET_CODE (x) == SYMBOL_REF !SYMBOL_REF_LOCAL_P (x)))
fputs ((GOT), asm_out_file);
else
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 1781b75..dbd841e 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -568,6 +568,10 @@ extern int prefer_neon_for_64bits;
#define NEED_PLT_RELOC 0
#endif
+#ifndef TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
+#define TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE 1
+#endif
+
/* Nonzero if we need to refer to the GOT with a PC-relative
offset. In other words, generate
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 9b74038..fa0839a 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -158,6 +158,10 @@ mlong-calls
Target Report Mask(LONG_CALLS)
Generate call insns as indirect calls, if necessary
+mpic-data-is-text-relative
+Target Report Var(arm_pic_data_is_text_relative)
Init(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE)
+Assume data segments are relative to text segment.
+
mpic-register=
Target RejectNegative Joined Var(arm_pic_register_string)
Specify the register to be used for PIC addressing
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 863e518..fbe77e6 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12120,6 +12120,12 @@ before execution begins.
Specify the register to be used for PIC addressing. The default is R10
unless stack-checking is enabled, when R9 is used.
+@item -mpic-data-is-text-relative
+@opindex mpic-data-is-text-relative
+Assume that each data segments are relative to text segment at load time.
+Therefore, it permits addressing data using PC-relative operations.
+This option is on by default for targets other than VxWorks RTP.
+
@item -mpoke-function-name
@opindex mpoke-function-name
Write the name of each function into the text section, directly
RE: [patch] [arm] New option for PIC offset unfixed
-Original Message-
From: Richard Earnshaw
Sent: Thursday, November 14, 2013 18:00
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [patch] [arm] New option for PIC offset unfixed
On 14/11/13 08:23, Joey Ye wrote:
-Original Message-
From: Richard Earnshaw
Sent: Thursday, November 14, 2013 0:57
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [patch] [arm] New option for PIC offset unfixed
So you are suggesting change like this:
+ Target Report Var(arm_pic_data_is_text_relative) Init(-1)
+ if (arm_pic_data_is_text_relative 0 TARGET_VXWORKS_RTP)
+ arm_pic_data_is_text_relative = 0;
+ else
+ arm_pic_data_is_text_relative = 1;
No, use the global_options_set structure to find out if the user has
set
the
value.
Thank pointing this out. Here is the latest patch with
global_options_set
This is OK.
Thanks!
However, don't you also need to fix the other references to
TARGET_VXWORKS_RTP; eg pic_offset_arm in arm.md?
The only reference of TARGET_VXWORKS_RTP in arm.md is define_insn
pic_offset_arm, which is used in arm_load_pic_register as
if (TARGET_VXWORKS_RTP)
{
...
emit_insn (gen_pic_offset_arm (pic_reg, pic_reg, pic_tmp));
}
Apparently it is a VxWorks specific pattern, I don't think it should be
changed, though I'm not 100% sure.
- Joey
RE: [patch] [arm] ARM Cortex-M3/M4 tuning
In mainline and arm/embedded-4_8-branch now. -Original Message- From: Janis Johnson [mailto:janis_john...@mentor.com] Sent: Thursday, November 14, 2013 1:45 To: Joey Ye; jani...@codesourcery.com Cc: gcc-patches@gcc.gnu.org; Ramana Radhakrishnan Subject: Re: [patch] [arm] ARM Cortex-M3/M4 tuning On 11/12/2013 10:20 PM, Joey Ye wrote: Janis, can you please take a look at test case changes. Thanks, Joey They look fine. Janis
RE: [patch] [arm] New option for PIC offset unfixed
This patch address all comments.
Thanks,
Joey
-Original Message-
From: Richard Earnshaw
Sent: Wednesday, November 13, 2013 19:07
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [patch] [arm] New option for PIC offset unfixed
On 13/11/13 10:20, Joey Ye wrote:
+@item -mpic-data-is-text-relative
+@opindex mpic-data-is-text-relative Assume that each data
+segments are relative to text segment at load
time.
+Therefore, prevent PC relative and GOTOFF style relocations to
+reference data.
I think the sense of this sentence is now backwards. I'd also try
to
avoid
GOTOFF in the user part of the manual.
How about
Therefore, prevent addressing data with relocation types that doesn't
apply in such circumstance.
No, that's still backwards. Remember, the option is now pic-data-is-text-
relative, so the option /permits/ addressing data using PC-relative
operations.
R.diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7757e86..5a95399 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2504,6 +2504,11 @@ arm_option_override (void)
arm_pic_register = pic_register;
}
+ if (arm_pic_data_is_text_relative 0 TARGET_VXWORKS_RTP)
+arm_pic_data_is_text_relative = 0;
+ else
+arm_pic_data_is_text_relative = 1;
+
/* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores. */
if (fix_cm3_ldrd == 2)
{
@@ -6020,7 +6025,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode,
rtx reg)
|| (GET_CODE (orig) == SYMBOL_REF
SYMBOL_REF_LOCAL_P (orig)))
NEED_GOT_RELOC
- !TARGET_VXWORKS_RTP)
+ arm_pic_data_is_text_relative)
insn = arm_pic_static_addr (orig, reg);
else
{
@@ -21498,7 +21503,7 @@ arm_assemble_integer (rtx x, unsigned int size, int
aligned_p)
{
/* See legitimize_pic_address for an explanation of the
TARGET_VXWORKS_RTP check. */
- if (TARGET_VXWORKS_RTP
+ if (!arm_pic_data_is_text_relative
|| (GET_CODE (x) == SYMBOL_REF !SYMBOL_REF_LOCAL_P (x)))
fputs ((GOT), asm_out_file);
else
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 9b74038..adac749 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -158,6 +158,10 @@ mlong-calls
Target Report Mask(LONG_CALLS)
Generate call insns as indirect calls, if necessary
+mpic-data-is-text-relative
+Target Report Var(arm_pic_data_is_text_relative) Init(-1)
+Assume data segments are relative to text segment.
+
mpic-register=
Target RejectNegative Joined Var(arm_pic_register_string)
Specify the register to be used for PIC addressing
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 863e518..fbe77e6 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12120,6 +12120,12 @@ before execution begins.
Specify the register to be used for PIC addressing. The default is R10
unless stack-checking is enabled, when R9 is used.
+@item -mpic-data-is-text-relative
+@opindex mpic-data-is-text-relative
+Assume that each data segments are relative to text segment at load time.
+Therefore, it permits addressing data using PC-relative operations.
+This option is on by default for targets other than VxWorks RTP.
+
@item -mpoke-function-name
@opindex mpoke-function-name
Write the name of each function into the text section, directly
RE: [patch] [arm] New option for PIC offset unfixed
-Original Message-
From: Richard Earnshaw
Sent: Tuesday, November 12, 2013 18:49
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [patch] [arm] New option for PIC offset unfixed
The name of the option and the documentation highlights that the
option's concept is confusing.
I think what you really need to do is to reverse the sense of the option
name and have
-mpic-data-is-text-relative
with the inverse (-mno-pic-data-is-text-relative) being the active value
that triggers for vxworks. That is, PIC data being text-relative is the
default for all targets except vxworks.
R.
Oh, and at run time, we should be talking about segments, not sections!
Richard, Thanks for quick response.
Updated patch renamed the option, variables and macro. Also use segments in
document. OK?
2013-11-13 Joey Ye joey...@arm.com
* config/arm/arm.c (arm_option_override): Error if
-mpic-data-is-text-relative without -fpic, and set for
VxWorks RTP.
(legitimize_pic_address): Use arm_pic_data_is_text_relative
(arm_assemble_integer): Likewise.
* config/arm/arm.h
(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE): New macro.
* config/arm/arm.opt (mpic-data-is-text-relative): New option.
* doc/invoke.texi (-mpic-data-is-text-relative): Doc for new option.diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7757e86..fdd5684 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2504,6 +2504,13 @@ arm_option_override (void)
arm_pic_register = pic_register;
}
+ if (TARGET_VXWORKS_RTP)
+arm_pic_data_is_text_relative = 0;
+
+ if (arm_pic_data_is_text_relative != TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
+ !flag_pic)
+error (-mpic-data-is-text-relative must be used with -fpic);
+
/* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores. */
if (fix_cm3_ldrd == 2)
{
@@ -6020,7 +6027,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode,
rtx reg)
|| (GET_CODE (orig) == SYMBOL_REF
SYMBOL_REF_LOCAL_P (orig)))
NEED_GOT_RELOC
- !TARGET_VXWORKS_RTP)
+ arm_pic_data_is_text_relative)
insn = arm_pic_static_addr (orig, reg);
else
{
@@ -21498,7 +21505,7 @@ arm_assemble_integer (rtx x, unsigned int size, int
aligned_p)
{
/* See legitimize_pic_address for an explanation of the
TARGET_VXWORKS_RTP check. */
- if (TARGET_VXWORKS_RTP
+ if (!arm_pic_data_is_text_relative
|| (GET_CODE (x) == SYMBOL_REF !SYMBOL_REF_LOCAL_P (x)))
fputs ((GOT), asm_out_file);
else
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 1781b75..dbd841e 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -568,6 +568,10 @@ extern int prefer_neon_for_64bits;
#define NEED_PLT_RELOC 0
#endif
+#ifndef TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
+#define TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE 1
+#endif
+
/* Nonzero if we need to refer to the GOT with a PC-relative
offset. In other words, generate
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 9b74038..fa0839a 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -158,6 +158,10 @@ mlong-calls
Target Report Mask(LONG_CALLS)
Generate call insns as indirect calls, if necessary
+mpic-data-is-text-relative
+Target Report Var(arm_pic_data_is_text_relative)
Init(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE)
+Assume data segments are relative to text segment.
+
mpic-register=
Target RejectNegative Joined Var(arm_pic_register_string)
Specify the register to be used for PIC addressing
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 863e518..298fcc3 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12120,6 +12120,12 @@ before execution begins.
Specify the register to be used for PIC addressing. The default is R10
unless stack-checking is enabled, when R9 is used.
+@item -mpic-data-is-text-relative
+@opindex mpic-data-is-text-relative
+Assume that each data segments are relative to text segment at load time.
+Therefore, prevent PC relative and GOTOFF style relocations to reference
+data. This is on by default for targets other than VxWorks RTP.
+
@item -mpoke-function-name
@opindex mpoke-function-name
Write the name of each function into the text section, directly
RE: [patch] [arm] ARM Cortex-M3/M4 tuning
Janis, can you please take a look at test case changes. Thanks, Joey -Original Message- From: Ramana Radhakrishnan Sent: Friday, November 08, 2013 17:11 To: Joey Ye Cc: gcc-patches@gcc.gnu.org; jani...@codesourcery.com Subject: Re: [patch] [arm] ARM Cortex-M3/M4 tuning ChangeLog: 2013-11-01 Julian Brown jul...@codesourcery.com Joey Ye joey...@arm.com * config/arm/arm.c (arm_cortex_m_branch_cost): New. (arm_v7m_tune): New. (arm_*_tune): Add comments for Sched adj cost. List all names here please rather than a regexp. * config/arm/arm-cores.def (cortex-m4, cortex-m3): Use arm_v7m_tune tuning. The ARM parts are ok but I'd like a testsuite maintainer to look at the testsuite changes before committing. regards Ramana testsuite: 2013-11-01 Joey Ye joey...@arm.com * gcc.dg/tree-ssa/forwprop-28.c: Disable for cortex_m. * gcc.dg/tree-ssa/vrp47.c: Likewise. * gcc.dg/tree-ssa/vrp87.c: Likewise. * gcc.dg/tree-ssa/ssa-dom-thread-4.c: Ignore for cortex_m. * gcc.dg/tree-ssa/ssa-vrp-thread-1.c: Likewise. v7m_tune_allin1-131016.patch Description: Binary data
[patch] [arm] New option for PIC offset unfixed
For RTOS who need to relocate executable, PC relative and GOTOFF cannot be used as the offset between any sections won't be fixed. Only GOT can be used, just as VxWorks RTP does. This patch introduces a new option enable user to choose between fixed offset or not. Enabled for VxWorks RTP to keep its behavior unchanged. Tested with arm-none-eabi make and VxWorks RTP small case OK to trunk? ChangeLog: 2013-11-12 Joey Ye joey...@arm.com * config/arm/arm.c (arm_option_override): Error if -mpic-offset-unfixed without -fpic, and set for VxWorks RTP. (legitimize_pic_address): Use arm_pic_offset_unfixed. (arm_assemble_integer): Likewise. * config/arm/arm.h (TARGET_DEFAULT_PIC_OFFSET_UNFIXED): New macro. * config/arm/arm.opt (mpic-offset-unfixed): New option. * doc/invoke.texi (-mpic-offset-unfixed): Doc for new option. pic_offset_fixed-1112.patch Description: Binary data
Re: [C] Fix PR57258: unused variable warning is emitted for volatile variables
- warning_at (DECL_SOURCE_LOCATION (p),
- OPT_Wunused_but_set_variable,
- variable %qD set but not used, p);
+ {
+if (!TREE_THIS_VOLATILE (p))
+ warning_at (DECL_SOURCE_LOCATION (p),
+OPT_Wunused_but_set_variable,
+variable %qD set but not used, p);
+ }
I'd prefer
else if (DECL_CONTEXT (p) == current_function_decl
!TREE_THIS_VOLATILE (p))
which concises the logic and avoids indent change
Thanks,
Joey
On Thu, Nov 7, 2013 at 3:37 PM, Bin.Cheng amker.ch...@gmail.com wrote:
On Thu, Nov 7, 2013 at 11:53 AM, Mingjie Xing mingjie.x...@gmail.com wrote:
2013/11/6 Richard Biener richard.guent...@gmail.com:
You miss a testcase.
Also why should the warning be omitted for unused automatic
volatile variables? They cannot be used in any way.
Richard.
Thanks. I've updated the patch with a test case.
c/ChangeLog
PR 57258
Should be:
PR c/57258
Thanks,
bin
* c-decl.c (pop_scope): Don't emit unused variable warnings for
accessed volatile variables.
testsuite/ChangeLog
* gcc.dg/Wunused-var-4.c: New test.
Mingjie
--
Best Regards.
RE: [patch] [arm] ARM Cortex-M3/M4 tuning
Ping -Original Message- From: Joey Ye [mailto:joey...@arm.com] Sent: Friday, November 01, 2013 1:00 To: gcc-patches@gcc.gnu.org Subject: [patch] [arm] ARM Cortex-M3/M4 tuning Based on Julian's http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01006.html and * Merged with latest mainline and applied test * Only apply to Cortex-M3 and M4 * Set LOGICAL_OP_NON_SHORT_CIRCUIT to false Test: - Coremark on M3/M4 gained 5% - GCC make check with qemu Cortex-M3 resulting following expected regressions, which will be fixed by following-up condition compare patches * gcc.target/arm/thumb2-cond-cmp-1.c: Expected fail for now. * gcc.target/arm/thumb2-cond-cmp-2.c: Likewise. * gcc.target/arm/thumb2-cond-cmp-3.c: Likewise. * gcc.target/arm/thumb2-cond-cmp-4.c: Likewise. ChangeLog: 2013-11-01 Julian Brown jul...@codesourcery.com Joey Ye joey...@arm.com * config/arm/arm.c (arm_cortex_m_branch_cost): New. (arm_v7m_tune): New. (arm_*_tune): Add comments for Sched adj cost. * config/arm/arm-cores.def (cortex-m4, cortex-m3): Use arm_v7m_tune tuning. testsuite: 2013-11-01 Joey Ye joey...@arm.com * gcc.dg/tree-ssa/forwprop-28.c: Disable for cortex_m. * gcc.dg/tree-ssa/vrp47.c: Likewise. * gcc.dg/tree-ssa/vrp87.c: Likewise. * gcc.dg/tree-ssa/ssa-dom-thread-4.c: Ignore for cortex_m. * gcc.dg/tree-ssa/ssa-vrp-thread-1.c: Likewise. v7m_tune_allin1-131016.patch Description: Binary data
[PATCH] [libiberty] MAX_PATH problems with mingw gcc
Ping ChangeLog 2013-10-27 Vladimir Simonov vladimir.simo...@acronis.com (include) filename.h (FILENAME_NORMALIZE): New macro. (filename_normalize): New declare. (libiberty) filename_cmp.c (memmove_left): New function. (filename_normalize): Likewise. getpwd.c (getpwd): Use FILENAME_NORMALIZE. (libcpp) directives.c (find_file_in_dir): Use FILENAME_NORMALIZE. Patch is at http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02257.html Thanks, Joey
Re: [PATCH 1/n] Add conditional compare support
+ TARGET_ARM || TARGET_THUMB2
TARGET_32BIT
+static const char *const ite = it\t%d4;
+static const int cmp_idx[9] = {0, 0, 1, 0, 1};
s/9/5/
On Wed, Oct 30, 2013 at 5:32 PM, Zhenqiang Chen zhenqiang.c...@arm.com wrote:
-Original Message-
From: Richard Henderson [mailto:r...@redhat.com]
Sent: Monday, October 28, 2013 11:07 PM
To: Zhenqiang Chen; Richard Earnshaw; 'Richard Biener'
Cc: GCC Patches
Subject: Re: [PATCH 1/n] Add conditional compare support
On 10/28/2013 01:32 AM, Zhenqiang Chen wrote:
Patch is updated according to your comments. Main changes are:
* Add two hooks: legitimize_cmp_combination and
legitimize_ccmp_combination
* Improve document.
No, these are not the hooks I proposed.
You should *not* have a ccmp_optab, because the middle-end has
absolutely no idea what mode TARGET should be in.
The hook needs to return an rtx expression appropriate for feeding to
cbranch et al. E.g. for arm,
(ne (reg:CC_Z CC_REGNUM) (const_int 0))
after having emitted the instruction that sets CC_REGNUM.
We need to push this to the backend because for ia64 the expression needs
to be of te form
(ne (reg:BI new_pseudo) (const_int 0))
Thanks for the clarification.
Patch is updated:
* ccmp_optab is removed.
* Add two hooks gen_ccmp_with_cmp_cmp and gen_ccmp_with_ccmp_cmp for backends
to generate the conditional compare instructions.
Thanks!
-Zhenqiang
[patch] [arm] ARM Cortex-M3/M4 tuning
Based on Julian's http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01006.html and * Merged with latest mainline and applied test * Only apply to Cortex-M3 and M4 * Set LOGICAL_OP_NON_SHORT_CIRCUIT to false Test: - Coremark on M3/M4 gained 5% - GCC make check with qemu Cortex-M3 resulting following expected regressions, which will be fixed by following-up condition compare patches * gcc.target/arm/thumb2-cond-cmp-1.c: Expected fail for now. * gcc.target/arm/thumb2-cond-cmp-2.c: Likewise. * gcc.target/arm/thumb2-cond-cmp-3.c: Likewise. * gcc.target/arm/thumb2-cond-cmp-4.c: Likewise. ChangeLog: 2013-11-01 Julian Brown jul...@codesourcery.com Joey Ye joey...@arm.com * config/arm/arm.c (arm_cortex_m_branch_cost): New. (arm_v7m_tune): New. (arm_*_tune): Add comments for Sched adj cost. * config/arm/arm-cores.def (cortex-m4, cortex-m3): Use arm_v7m_tune tuning. testsuite: 2013-11-01 Joey Ye joey...@arm.com * gcc.dg/tree-ssa/forwprop-28.c: Disable for cortex_m. * gcc.dg/tree-ssa/vrp47.c: Likewise. * gcc.dg/tree-ssa/vrp87.c: Likewise. * gcc.dg/tree-ssa/ssa-dom-thread-4.c: Ignore for cortex_m. * gcc.dg/tree-ssa/ssa-vrp-thread-1.c: Likewise. v7m_tune_allin1-131016.patch Description: Binary data
Re: FW: MAX_PATH problems with mingw gcc
Vladimir, I found no more issue on patch itself. But ChangeLogs are missing. Please refer to format in /include/ChangeLog, and generate yours in your next email for /include /libcpp /libiberty Maintainers of libiberty are in TO list. - Joey
Re: ARM: VFPv3-D16 vs. VFPv3-D32
Which Cortex-R you are targeting that supports both D16 and D32? Thanks, Joey On Thu, Oct 17, 2013 at 3:13 PM, Sebastian Huber sebastian.hu...@embedded-brains.de wrote: Hello, it seems that it is not possible to deduce from GCC built-in defines whether we compile for the VFPv3-D16 or VFPv3-D32 floating point unit. touch empty.c arm-eabi-gcc -march=armv7-r -mfpu=vfpv3-d16 -mfloat-abi=hard -E -P -v -dD empty.c vfpv3-d16.txt arm-eabi-gcc -march=armv7-r -mfpu=vfpv3 -mfloat-abi=hard -E -P -v -dD empty.c vfpv3-d32.txt diff vfpv3-d16.txt vfpv3-d32.txt Is it possible to add a built-in define for this? Or as an alternative is it possible to use a GCC configuration target specific multi-lib define that indicates it? I want to use such a compiler provided define to determine how the context switch support in an operating system should look like. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: ARM: VFPv3-D16 vs. VFPv3-D32
There is no macro to indicate VFP variances. Probably you can check CPU variance instead. As I know Cortex-R only support D16. Joey On Thu, Oct 17, 2013 at 3:47 PM, Sebastian Huber sebastian.hu...@embedded-brains.de wrote: On 2013-10-17 09:28, Joey Ye wrote: Which Cortex-R you are targeting that supports both D16 and D32? I have a Cortex-R variant which supports D16 only and now I want to add a multi-lib to our GCC target configuration and use a compiler built-in to adjust the context switch code for this multi-lib. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: FW: MAX_PATH problems with mingw gcc
On Thu, Oct 17, 2013 at 4:18 PM, Vladimir Simonov vladimir.simo...@acronis.com wrote: Thank you for pointing this problem. So, on file systems with symlinks support playing with filenames as strings is impossible. This means that filename_normalize name is too pretentious - it will do nothing for most of gcc consumers. From other side - it is useful for OS-es with small MAX_PATH. Do you think it should be renamed as filename_shortcut/filename_simplify or something like it? So readers won't expect too much from it even without looking at its body. Is it allowed to write # ifdef HAVE_DOS_BASED_FILE_SYSTEM extern void filename_normalize (char *f); #else #define filename_normalize (char *f) #endif directly in include/filenames.h? This way we'll avoid unnecessary empty call on platforms with symlinks. #define a lower case function name isn't a good practice. So please resume your original change that define FILENAME_XXX here like: #ifdef HAVE_DOS_BASED_FILE_SYSTEM #define FILENAME_XXX(f) filename_xxx(f) #else #define FILENAME_XXX(f) #endif And more common question. I can imagine that different filenames produced after cross build on different OS-es for the same target may confuse some upper-level tools, like code analyzers, code coverage, etc... Does it make sense to push such solution to gcc mainsteam? May be it is better to keep the patch for cross toolchains builders... IMO it is not a concern. One reason libiberty is there, is because people know different OS-es have different filename system. A tool should either use libiberty to process it, or smarter enough to handle difference by themselves. Another good thing about the idea of filename_normalize is that it make possible to do real filename_cmp. Current filename_cmp fails to compare c:/a/b/.. and c:/a. You patch is at least a start to solve it. I do encourage you to upstream this patch, though I'm not the maintainer of libiberty to approve it. Thanks, Joey
Re: FW: MAX_PATH problems with mingw gcc
Thanks for contribution. See review comments at following.
On Wed, Oct 16, 2013 at 6:45 PM, Vladimir Simonov
vladimir.simo...@acronis.com wrote:
Hi,
Resending filename-normalize patch to correct list gcc-patches@gcc.gnu.org.
All context, please, see below.
+extern void filename_normalize (char *f);
+#define FILENAME_NORMALIZE(f) filename_normalize(f)
The macro FILENAME_NORMALIZE doesn't look necessary. I would prefer
use filename_normalize directly, just as filename_cmp is used in GCC
rather than FILENAME_CMP
+ FILENAME_NORMALIZE (path);
Likewise. Change to lower case file name.
+#ifndef HAVE_DOS_BASED_FILE_SYSTEM
+void
+filename_normalize (char *fn)
+#else
+static void
+filename_normalize_unix (char *fn)
+#endif
Redefining function names is easy to be confusing. At least I went up
and down several times to understand difference of two functions. I
feel it much clear to define a single filename_normalize, and then use
HAVE_DOS_BASED_FILE_SYSTEM to inline additional stuff for DOS into
this function. Like
filename_normalize (char *fn)
{
...
#ifdef HAVE_DOS_BASED_FILE_SYSTEM
// Additional work that your current DOS version does, basically
moving fn if needed.
#endif
... // Rest of what you have now
}
+ next = p + 1;
+ if (*next == '\0')
+break;
+ if (!IS_DIR_SEPARATOR( *p))
+ continue;
Fix wrong indent at continue.
Also it normalize filename like:
c:\abc\ into c:/abc\
The ending \ isn't normalized. Need to refine logic here to handle this case.
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+void
+filename_normalize (char *fn)
+{
+ if (IS_DIR_SEPARATOR (fn[0]) || ! IS_ABSOLUTE_PATH (fn))
+/* Absolute path in Unix style or relative path */
+filename_normalize_unix (fn);
+ else if (fn[1] != '\0')
+filename_normalize_unix (fn + 2);
+}
+#endif
+
Inline into unified version of filename_normalize
+filename_normalize (char *fn)
+{
+ char *p;
+ int rest;
It will be more robust to check if fn is NULL at function entry. By
doing so, you can remove the if (pwd) condition in getpwd.c
if (!pwd)
+{
pwd = getcwd (XNEWVEC (char, MAXPATHLEN + 1), MAXPATHLEN + 1
#ifdef VMS
, 0
#endif
);
+if (pwd)
+ FILENAME_NORMALIZE (pwd);
+}
All code inside {} should be two more spaces indent here. And remove if (pwd).
Thanks,
Joey
Re: FW: MAX_PATH problems with mingw gcc
On Wed, Oct 16, 2013 at 6:45 PM, Vladimir Simonov
vladimir.simo...@acronis.com wrote:
There are many pro and contras, i.e. it adds additional, probably unnecessary
work on Linux
time but makes filenames shorter, it affects libiberty which is shared
between gcc/binutils/gdb,
but it may be useful in other packages, etc.,
etc.
There is an issue on file system with symbolic link, like Linux
ext2/3. It could vitally change behavior of GCC. The issue is
described as following.
Given that on Linux you have following directory structure:
base/
level1-a/
level2-a/
level2-b (- level1-a/level2-a)
level2-b is a symbolic link created by:
ln -s level1-a/level2-a level1-b
Now run ls base/level2-b/.. you probably would expect it equal to
ls base, as the logic in your patch implies. However, the actual
behavior is equal to ls base/level1-a instead, because
base/level2-b/.. == base/level1-a/level2-a/.. == base/level1-a
Such a logic cannot be deduced simple from the name string, so your
patch can do nothing about it.
For your patch IMHO the way out could be just define a real function
for DOS FS, and leave a blank function body otherwise. Like:
filename_normalize (char *fn)
{
#if DOS
...
#endif
}
Thanks,
Joey
RE: [PATCH, libgcc] Disable JCR section when java is not enabled
-Original Message- From: Jakub Jelinek [mailto:ja...@redhat.com] Sent: Thursday, October 10, 2013 16:48 To: Joey Ye Cc: p...@bothner.com; a...@redhat.com; Tom Tromey; H.J. Lu; gcc-patches; 'Ian Lance Taylor' Subject: Re: [PATCH, libgcc] Disable JCR section when java is not enabled On Thu, Oct 10, 2013 at 04:22:52PM +0800, Joey Ye wrote: Dear Java maintainers, are you OK with this patch? Given the state of gcj that it is now only rarely used and most people just use OpenJDK instead, wouldn't it be a good idea to just require that gcj code is linked using gcj driver or, if linked in any other driver, just using a special non- default option (-flink-jcr or similar), that would be automatically set by gcj driver, move this JCR stuff out of the normal crt* files and put it into crtjava*.o instead, and only link in if -flink-jcr is passed or gcj driver used? Or treat -lgcj as that magic switch? Or, alternatively, at least for selected targets, live with the extra 8 bytes in .jcr section for every binary/shared library, but move the _Jv_RegisterClasses call into libgcj_nonshared.a and libgcj.a and make libgcj.so be a linker script containing both libgcj_nonshared.a and libgcj.so.*. 8 bytes of RAM is precious for embedded system. As Tom pointed out that a complete solution need significantly more work, I'd prefer to enable this simple fix. OK for trunk? Thanks, Joey
RE: [PATCH, libgcc] Disable JCR section when java is not enabled
Dear Java maintainers, are you OK with this patch?
- Joey
-Original Message-
From: Ian Lance Taylor [mailto:i...@google.com]
Sent: Thursday, September 12, 2013 3:28
To: Joey Ye
Cc: gcc-patches; H.J. Lu; p...@bothner.com; a...@redhat.com; Tom Tromey
Subject: Re: [PATCH, libgcc] Disable JCR section when java is not enabled
On Tue, Sep 10, 2013 at 2:01 AM, Joey Ye joey...@arm.com wrote:
Updated to http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01097.html
Build passes on arm-none-eabi and bootstrap passes on x86.
OK to trunk?
ChangeLog
* libgcc/Makefile.in: Include JAVA_IS_ENABLED in CFLAGS.
* libgcc/configure.ac (java_is_enabled): New variable.
* libgcc/configure: Regenerated.
* libgcc/crtstuff.c: Check JAVA_IS_ENABLED.
The ChangeLog entries should be in libgcc/ChangeLog, and they should not
have the libgcc/ prefix on the file names. Compare to the other entries
in
that file.
This patch is OK for libgcc.
However, before committing it, I would like it to be approved by a Java
maintainer. I've CC'ed the Java maintainers on this message.
Thanks.
Ian
Index: Makefile.in
==
=
--- Makefile.in (revision 194467)
+++ Makefile.in (working copy)
@@ -281,7 +281,8 @@
-finhibit-size-directive -fno-inline -fno-exceptions \
-fno-zero-initialized-in-bss -fno-toplevel-reorder
-fno-tree-vectorize \
-fno-stack-protector \
- $(INHIBIT_LIBC_CFLAGS)
+ $(INHIBIT_LIBC_CFLAGS) \
+ -DJAVA_IS_ENABLED=@java_is_enabled@
# Extra flags to use when compiling crt{begin,end}.o.
CRTSTUFF_T_CFLAGS =
Index: configure.ac
==
=
--- configure.ac(revision 194467)
+++ configure.ac(working copy)
@@ -204,6 +204,17 @@
esac],
[enable_sjlj_exceptions=auto])
+# Disable jcr section if we are not building java case
+,${enable_languages}, in
+ *,java,*)
+java_is_enabled=1
+;;
+ *)
+java_is_enabled=0
+;;
+esac
+AC_SUBST(java_is_enabled)
+
AC_CACHE_CHECK([whether to use setjmp/longjmp exceptions],
[libgcc_cv_lib_sjlj_exceptions], [AC_LANG_CONFTEST(
Index: crtstuff.c
==
=
--- crtstuff.c (revision 194467)
+++ crtstuff.c (working copy)
@@ -145,6 +145,10 @@
# define USE_TM_CLONE_REGISTRY 1
#endif
+#if !JAVA_IS_ENABLED
+#undef JCR_SECTION_NAME
+#endif
+
/* We do not want to add the weak attribute to the declarations of
these
routines in unwind-dw2-fde.h because that will cause the definition
of
these symbols to be weak as well.
Index: configure
==
=
--- configure (revision 194467)
+++ configure (working copy)
@@ -566,6 +566,7 @@
set_use_emutls
set_have_cc_tls
vis_hide
+java_is_enabled
fixed_point
enable_decimal_float
decimal_float
@@ -4191,6 +4192,17 @@
fi
+# Disable jcr section if we are not building java case
+,${enable_languages}, in
+ *,java,*)
+java_is_enabled=1
+;;
+ *)
+java_is_enabled=0
+;;
+esac
+
+
{ $as_echo $as_me:${as_lineno-$LINENO}: checking whether to use
setjmp/longjmp exceptions 5 $as_echo_n checking whether to use
setjmp/longjmp exceptions... 6; } if test
${libgcc_cv_lib_sjlj_exceptions+set} = set; then :
[PATCH, libgcc] Disable JCR section when java is not enabled
Updated to http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01097.html
Build passes on arm-none-eabi and bootstrap passes on x86.
OK to trunk?
ChangeLog
* libgcc/Makefile.in: Include JAVA_IS_ENABLED in CFLAGS.
* libgcc/configure.ac (java_is_enabled): New variable.
* libgcc/configure: Regenerated.
* libgcc/crtstuff.c: Check JAVA_IS_ENABLED.
Index: Makefile.in
===
--- Makefile.in (revision 194467)
+++ Makefile.in (working copy)
@@ -281,7 +281,8 @@
-finhibit-size-directive -fno-inline -fno-exceptions \
-fno-zero-initialized-in-bss -fno-toplevel-reorder -fno-tree-vectorize \
-fno-stack-protector \
- $(INHIBIT_LIBC_CFLAGS)
+ $(INHIBIT_LIBC_CFLAGS) \
+ -DJAVA_IS_ENABLED=@java_is_enabled@
# Extra flags to use when compiling crt{begin,end}.o.
CRTSTUFF_T_CFLAGS =
Index: configure.ac
===
--- configure.ac(revision 194467)
+++ configure.ac(working copy)
@@ -204,6 +204,17 @@
esac],
[enable_sjlj_exceptions=auto])
+# Disable jcr section if we are not building java
+case ,${enable_languages}, in
+ *,java,*)
+java_is_enabled=1
+;;
+ *)
+java_is_enabled=0
+;;
+esac
+AC_SUBST(java_is_enabled)
+
AC_CACHE_CHECK([whether to use setjmp/longjmp exceptions],
[libgcc_cv_lib_sjlj_exceptions],
[AC_LANG_CONFTEST(
Index: crtstuff.c
===
--- crtstuff.c (revision 194467)
+++ crtstuff.c (working copy)
@@ -145,6 +145,10 @@
# define USE_TM_CLONE_REGISTRY 1
#endif
+#if !JAVA_IS_ENABLED
+#undef JCR_SECTION_NAME
+#endif
+
/* We do not want to add the weak attribute to the declarations of these
routines in unwind-dw2-fde.h because that will cause the definition of
these symbols to be weak as well.
Index: configure
===
--- configure (revision 194467)
+++ configure (working copy)
@@ -566,6 +566,7 @@
set_use_emutls
set_have_cc_tls
vis_hide
+java_is_enabled
fixed_point
enable_decimal_float
decimal_float
@@ -4191,6 +4192,17 @@
fi
+# Disable jcr section if we are not building java
+case ,${enable_languages}, in
+ *,java,*)
+java_is_enabled=1
+;;
+ *)
+java_is_enabled=0
+;;
+esac
+
+
{ $as_echo $as_me:${as_lineno-$LINENO}: checking whether to use
setjmp/longjmp exceptions 5
$as_echo_n checking whether to use setjmp/longjmp exceptions... 6; }
if test ${libgcc_cv_lib_sjlj_exceptions+set} = set; then :
RE: [arm-embedded] Request to backport thumb1 far jump patch to embedded 4.8 branch
OK for embedded 4.8 branch - Joey -Original Message- From: Terry Guo Sent: Tuesday, August 06, 2013 14:17 To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: [arm-embedded] Request to backport thumb1 far jump patch to embedded 4.8 branch Hello Joey, The thumb1 far jump patch is about an optimization to avoid unnecessary lr save instruction. It is now in trunk. Is it OK to back port it to embedded 4.8 branch? BR, Terry gcc/ChangeLog.arm 2013-08-05 Terry Guo terry@arm.com Backport from mainline r197956 2013-04-15 Joey Ye joey...@arm.com * config/arm/arm.c (thumb1_final_prescan_insn): Assert lr save for real far jump. (thumb_far_jump_used_p): Count instruction size and set far_jump_used. gcc/testsuite/ChangeLog.arm 2013-08-05 Terry Guo terry@arm.com Backport from mainline r197956 2013-04-15 Joey Ye joey...@arm.com * gcc.target/arm/thumb1-far-jump-1.c: New test. * gcc.target/arm/thumb1-far-jump-2.c: New test.
RE: [arm-embedded] Request to back port Cortex-R7 option support patch
OK to embedded 4.8 branch. -Original Message- From: Terry Guo Sent: Tuesday, August 06, 2013 11:59 To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: [arm-embedded] Request to back port Cortex-R7 option support patch Hi Joey, Attached patch is a backport to support cortex-r7 in gcc command line. Tested and it works. Is it OK to commit? BR, Terry 2013-08-05 Terry Guo terry@arm.com Backport from mainline r197153 2013-03-27 Terry Guo terry@arm.com * config/arm/arm-cores.def: Added core cortex-r7. * config/arm/arm-tune.md: Regenerated. * config/arm/arm-tables.opt: Regenerated. * doc/invoke.texi: Added entry for core cortex-r7.
RE: [arm-embedded] Patch to define multilibs for arm embedded-4_8-branch
OK - Joey -Original Message- From: Terry Guo Sent: Wednesday, July 24, 2013 16:15 To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: [arm-embedded] Patch to define multilibs for arm embedded-4_8- branch Hi Joey, This patch is to define multilibs for recently created embedded-4_8-branch. Is it OK to commit? BR, Terry 2013-07-24 Terry Guo terry@arm.com * configure.ac (with_multilib_list): Export its value. * Makefile.in (with_multilib_list): Import it from configure files. * configure: Regenerated. * config/arm/t-mlibs: New files to define multilibs. * config.gcc: Use above multilib fragment.
[arm/embedded-4_7-branch] Merge with gcc-4_7-branch 199638
Committed as 199680
RE: [RFA/ARM/4.7] Fix PR54974: Thumb literal pools don't handle PC rounding
-Original Message- From: Ramana Radhakrishnan Sent: Friday, May 10, 2013 18:13 To: Matthew Gretton-Dann Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw; d...@canonical.com; Patch Tracking; Richard Biener; Joey Ye Subject: Re: [RFA/ARM/4.7] Fix PR54974: Thumb literal pools don't handle PC rounding OK for 4.7? Ok - I did say ok if no fallout in my original review :) Tested with Qemu Cortex-M3. No regression. Committed to 4.7. - Joey
RE: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump
-Original Message-
From: Ramana Radhakrishnan
Sent: Thursday, April 11, 2013 4:40 PM
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH][ARM][thumb1] Reduce lr save for leaf function with
non-far jump
On 12/20/12 09:53, Joey Ye wrote:
Current GCC thumb1 has an annoying problem that always assuming far
branch.
So it forces to save lr, even when unnecessarily. The most extreme
case complained by partner is:
// compiled with -mthumb -mcpu=cortex-m0 -Os.
void foo() { for (;;); }
=
foo:
push{lr} // Crazy!!!
.L2:
b .L2
The reason is that thumb1 far jump is only resolved in the very late
pass shorten_branch. Prologue/epilogue pass doesn't actually know a
branch is far or not from its attribute. It has to conservatively
save/restore lr whenever there is a branch.
This patch tries to fix it with a simple heuristic, i.e., using
function size to decide if a far jump will likely be used. Function
size information is meaningful in prologue/epilogue pass. The
heuristic uses following check to decide if lr should be saved for far
jump:
function_size * 3 = 2048 // yes: save lr for possible far jump. No:
don't save lr for far jump
The scheme has an issue: if some corner case does break above
condition, there is no chance to fix-up but to ICE. But the heuristic
condition is very conservative. It is base on the worse normal
condition that each instruction is associated with a 4 byte literal (
(2+4)/2=3,
blooming size by 3 times ).
I can't think of a real case to trigger the ICE. So I think it should
work.
Other approaches than the heuristic scheme are too expensive to
implement for this small size/performance issue. I did explored some
but none of them persuaded myself.
Tests passed:
* build libgcc, libstdc++, newlib, libm
* make check-gcc with cpu=cortex-m0
* Small and extreme test cases
ChangeLog:
2012-12-20 Joey Ye joey...@arm.com
* config/arm/arm.c(thumb1_final_prescan_insn):
Assert lr save for real far jump.
(thumb_far_jump_used_p): Count instruction size and set
far_jump_used.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index
327ef22..ad79451 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -21790,6 +21857,11 @@ thumb1_final_prescan_insn (rtx insn)
else if (conds != CONDS_NOCOND)
cfun-machine-thumb1_cc_insn = NULL_RTX;
}
+
+/* Check if unexpected far jump is used. */
+if (cfun-machine-lr_save_eliminated
+ get_attr_far_jump (insn) == FAR_JUMP_YES)
+ internal_error(Unexpected thumb1 far jump);
}
int
@@ -21815,6 +21887,8 @@ static int
thumb_far_jump_used_p (void)
{
rtx insn;
+ bool far_jump = false;
+ unsigned int func_size = 0;
/* This test is only important for leaf functions. */
/* assert (!leaf_function_p ()); */ @@ -21870,6 +21944,26 @@
thumb_far_jump_used_p (void)
get_attr_far_jump (insn) == FAR_JUMP_YES
)
{
+ far_jump = true;
+ }
+ func_size += get_attr_length (insn);
+}
+
+ /* Attribute far_jump will always be true for thumb1 before
shorten_branch
+ pass. So checking far_jump attribute before shorten_branch isn't
much
+ useful.
+
+ Following heuristic tries to estimate more accurately if a far
+ jump
may
+ finally be used. The heuristic is very conservative as there is
+ no
chance
+ to roll-back the decision of not to use far jump.
+
+ Thumb1 long branch offset is -2048 to 2046. The worst case is
+ each
2-byte
+ insn is associated with a 4 byte constant pool. Using function
size
+ 2048/3 as the threshold is conservative enough. */ if
+ (far_jump)
+{
+ if ((func_size * 3) = 2048)
+{
/* Record the fact that we have decided that
the function does use far jumps. */
cfun-machine-far_jump_used = 1;
Check for 80 character line length in the comments above - I can never
tell if
it is my mail client or yours.
Further shorten the lines.
Otherwise ok if no regressions..
Make check targeting Cortex-M0/M3 with qemu. No regression.
Committed as 197956
- Joey
RE: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump
Ping ^ 2
-Original Message-
From: Joey Ye
Sent: Saturday, January 05, 2013 3:41 PM
To: Ramana Radhakrishnan
Cc: Joey Ye; gcc-patches@gcc.gnu.org
Subject: RE: [PATCH][ARM][thumb1] Reduce lr save for leaf function with
non-far jump
Ping
-Original Message-
From: Joey Ye
Sent: Thursday, December 20, 2012 17:53
To: gcc-patches@gcc.gnu.org; Ramana Radhakrishnan
Cc: Joey Ye
Subject: [PATCH][ARM][thumb1] Reduce lr save for leaf function with
non- far jump
Current GCC thumb1 has an annoying problem that always assuming far
branch.
So it forces to save lr, even when unnecessarily. The most extreme
case complained by partner is:
// compiled with -mthumb -mcpu=cortex-m0 -Os.
void foo() { for (;;); }
=
foo:
push{lr} // Crazy!!!
.L2:
b .L2
The reason is that thumb1 far jump is only resolved in the very late
pass shorten_branch. Prologue/epilogue pass doesn't actually know a
branch is far or not from its attribute. It has to conservatively
save/restore lr whenever there is a branch.
This patch tries to fix it with a simple heuristic, i.e., using
function size to decide if a far jump will likely be used. Function
size information is meaningful in prologue/epilogue pass. The
heuristic uses following check to decide if lr should be saved for far
jump:
function_size * 3 = 2048 // yes: save lr for possible far jump. No:
don't
save lr for far jump
The scheme has an issue: if some corner case does break above
condition, there is no chance to fix-up but to ICE. But the heuristic
condition is very conservative. It is base on the worse normal
condition that each instruction is associated with a 4 byte literal (
(2+4)/2=3, blooming size by 3 times ).
I can't think of a real case to trigger the ICE. So I think it should
work.
Other approaches than the heuristic scheme are too expensive to
implement for this small size/performance issue. I did explored some
but none of them persuaded myself.
Tests passed:
* build libgcc, libstdc++, newlib, libm
* make check-gcc with cpu=cortex-m0
* Small and extreme test cases
ChangeLog:
2012-12-20 Joey Ye joey...@arm.com
* config/arm/arm.c(thumb1_final_prescan_insn):
Assert lr save for real far jump.
(thumb_far_jump_used_p): Count instruction size and set
far_jump_used.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index
327ef22..ad79451 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -21790,6 +21857,11 @@ thumb1_final_prescan_insn (rtx insn)
else if (conds != CONDS_NOCOND)
cfun-machine-thumb1_cc_insn = NULL_RTX;
}
+
+/* Check if unexpected far jump is used. */
+if (cfun-machine-lr_save_eliminated
+ get_attr_far_jump (insn) == FAR_JUMP_YES)
+ internal_error(Unexpected thumb1 far jump);
}
int
@@ -21815,6 +21887,8 @@ static int
thumb_far_jump_used_p (void)
{
rtx insn;
+ bool far_jump = false;
+ unsigned int func_size = 0;
/* This test is only important for leaf functions. */
/* assert (!leaf_function_p ()); */ @@ -21870,6 +21944,26 @@
thumb_far_jump_used_p (void)
get_attr_far_jump (insn) == FAR_JUMP_YES
)
{
+ far_jump = true;
+ }
+ func_size += get_attr_length (insn);
+}
+
+ /* Attribute far_jump will always be true for thumb1 before
shorten_branch
+ pass. So checking far_jump attribute before shorten_branch isn't
much
+ useful.
+
+ Following heuristic tries to estimate more accruately if a far
jump
may
+ finally be used. The heuristic is very conservative as there is
+ no
chance
+ to roll-back the decision of not to use far jump.
+
+ Thumb1 long branch offset is -2048 to 2046. The worst case is
+ each
2-byte
+ insn is assiociated with a 4 byte constant pool. Using function
size
+ 2048/3 as the threshold is conservative enough. */ if
+ (far_jump)
+{
+ if ((func_size * 3) = 2048)
+{
/* Record the fact that we have decided that
the function does use far jumps. */
cfun-machine-far_jump_used = 1;
[arm/embedded-4_7-branch] Merge with gcc-4_7-branch r196534
Committed as r196535.
RE: [PATCH] Fix PR50293 - LTO plugin with space in path
-Original Message-
From: Joseph Myers [mailto:jos...@codesourcery.com]
Sent: Monday, March 04, 2013 00:49
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path
On Mon, 18 Feb 2013, Joey Ye wrote:
+static char * convert_white_space (char *);
No space after *.
Fixed
- linker_plugin_file_spec = find_a_file (exec_prefixes,
+ char * temp_spec = find_a_file (exec_prefixes,
LTOPLUGINSONAME, R_OK,
false);
The indentation of the following lines looks odd after this patch;
unless
that's just an effect of TABs plus quoting, make sure they are
reindented
to line up with the new position of the opening '('.
They was unchanged. But since the line with '(' changed, it need changing
too.
Fixed
+/* Insert back slash before spaces in orig (usually a file path), to
Capitalize variable names when referring to the value of the variable,
so
ORIG; likewise elsewhere in this comment. Single work backslash.
+ the filename should be treated as a single argument rather than
being
file name should be two words, according to the GNU Coding Standards.
+ This function converts and only converts all occurrance of ' '
occurrence
+ Return: orig if no conversion needed. orig if conversion needed
but no
+ sufficient memory for a new string. Otherwise a newly allocated
string
Returning wrong results on insufficient memory doesn't make sense.
Anyway, xmalloc always exits the program if there is insufficient memory,
so you don't need any code to allow for that case.
Fixed. Though it is conflicting with secure coding practice.
+static char * convert_white_space (char *orig)
Newline, not space, between return type and function name, so that the
function name comes at the start of the line.
Fixed.
+ if (orig == NULL) return orig;
The comment didn't mention NULL as a valid argument, and it doesn't
appear
NULL can actually be passed to this function. So don't include code to
handle that case.
Fixed.
+ for (len=0; orig[len]; len++)
Spaces around =.
Fixed.
+if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;
No space before ++, but put the body of the if on a separate line.
Fixed.
+ char * new_spec = (char *)xmalloc (len + number_of_space + 1);
No space after *. Space in the cast after (char *).
Fixed.
+ int j,k;
Space after ,.
Fixed.
+ if (new_spec == NULL) return orig;
As discussed above, not needed.
Removed.
+ for (j=0, k=0; j=len; j++, k++)
Spaces around = and =.
Fixed.
+ if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';
Put the if both on a separate line.
Fixed.
+ else return orig;
Put the else body on a separate line.
Fixed.
--
Joseph S. Myers
jos...@codesourcery.com
Index: gcc/gcc.c
===
--- gcc/gcc.c (revision 195189)
+++ gcc/gcc.c (working copy)
@@ -265,6 +265,7 @@
static const char *compare_debug_auxbase_opt_spec_function (int, const char
**);
static const char *pass_through_libs_spec_func (int, const char **);
static const char *replace_extension_spec_func (int, const char **);
+static char *convert_white_space (char *);
/* The Specs Language
@@ -6595,6 +6596,7 @@
X_OK, false);
if (lto_wrapper_file)
{
+ lto_wrapper_file = convert_white_space (lto_wrapper_file);
lto_wrapper_spec = lto_wrapper_file;
obstack_init (collect_obstack);
obstack_grow (collect_obstack, COLLECT_LTO_WRAPPER=,
@@ -7005,12 +7007,13 @@
+ strlen (fuse_linker_plugin), 0))
#endif
{
- linker_plugin_file_spec = find_a_file (exec_prefixes,
-LTOPLUGINSONAME, R_OK,
-false);
- if (!linker_plugin_file_spec)
+ char *temp_spec = find_a_file (exec_prefixes,
+LTOPLUGINSONAME, R_OK,
+false);
+ if (!temp_spec)
fatal_error (-fuse-linker-plugin, but %s not found,
LTOPLUGINSONAME);
+ linker_plugin_file_spec = convert_white_space (temp_spec);
}
#endif
lto_gcc_spec = argv[0];
@@ -8506,3 +8509,51 @@
free (name);
return result;
}
+
+/* Insert backslash before spaces in ORIG (usually a file path), to
+ avoid being broken by spec parser.
+
+ This function is needed as do_spec_1 treats white space (' ' and '\t')
+ as the end of an argument. But in case of -plugin /usr/gcc
install/xxx.so,
+ the file name should be treated as a single argument rather than being
+ broken into multiple. Solution
RE: [PATCH] Fix PR50293 - LTO plugin with space in path
-Original Message- From: Georg-Johann Lay [mailto:g...@gcc.gnu.org] Sent: Monday, March 04, 2013 02:42 To: Joey Ye Cc: 'Joseph Myers'; gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix PR50293 - LTO plugin with space in path Joey Ye schrieb: Ping Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path Does this patch also work with MS-Windows as host, i.e. with \ as path separator? This patch itself doesn't handle '\'. As it should in comments: a\\ b - a b But I doubt \ is a problem, as Mingw or Cygwin changes it to / instead. - Joey
RE: [PATCH] Fix PR50293 - LTO plugin with space in path
Ping
-Original Message-
From: Joey Ye [mailto:joey...@arm.com]
Sent: Monday, February 18, 2013 11:32
To: 'Joseph Myers'
Cc: gcc-patches@gcc.gnu.org
Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path
Joseph, Thanks for your valuable comments. See my reply and new patch
below.
-Original Message-
From: Joseph Myers [mailto:jos...@codesourcery.com]
Sent: Monday, February 18, 2013 06:16
To: Joey Ye
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix PR50293 - LTO plugin with space in path
On Sun, 17 Feb 2013, Joey Ye wrote:
+static char * convert_white_space(char * orig);
Please fix formatting in many places in this patch to follow the GNU
Coding Standards. No space after '*', but space before '('; there
seem
to
be various other formatting problems as well.
My bad. All fixed.
+/* Insert back slash before spaces in a string, to avoid path
+ that has space in it broken into multiple arguments. */
That doesn't seem to be a proper specification of the interface to
this
function. What are the semantics of ORIG? A string that is a
filename,
or something else? What are the exact semantics of the return value
for
quoting - is it correct for the function to convert a (backslash,
space)
pair to (backslash, backslash, space) or not? Is anything special in
the
return value other than backslash and space, and how are any special
characters in the return value to be interpreted?
As it seems like this function frees the argument (why?) this also
needs
to be specified in the comment as part of the semantics of the
function.
This function might need a string longer than original one to
accommodate
additional back slashes. So it has to xmalloc a new string. The original
string should be freed in such a case. However, it is tedious to caller
to figure out that conversion does happens and free the orig. The
solution
is for this function to free it when conversion happens.
By doing so it is required that orig must be allocated and can be freed,
as the newly added comments described explicitly.
It would be a good idea for you to give a more detailed explanation in
the
next version of the patch submission of how the path, before the patch,
got processed so that the spaces were wrongly interpreted. That might
help make clearer whether the interface to this new function is
actually
correct, since the subsequent operations on the return value should
act
as
an inverse to the operation carried out by this function.
--
Joseph S. Myers
jos...@codesourcery.com
Index: gcc/gcc.c
===
--- gcc/gcc.c (revision 195189)
+++ gcc/gcc.c (working copy)
@@ -265,6 +265,7 @@
static const char *compare_debug_auxbase_opt_spec_function (int, const
char **);
static const char *pass_through_libs_spec_func (int, const char **);
static const char *replace_extension_spec_func (int, const char **);
+static char * convert_white_space (char *);
/* The Specs Language
@@ -6595,6 +6596,7 @@
X_OK, false);
if (lto_wrapper_file)
{
+ lto_wrapper_file = convert_white_space (lto_wrapper_file);
lto_wrapper_spec = lto_wrapper_file;
obstack_init (collect_obstack);
obstack_grow (collect_obstack, COLLECT_LTO_WRAPPER=,
@@ -7005,12 +7007,13 @@
+ strlen (fuse_linker_plugin), 0))
#endif
{
- linker_plugin_file_spec = find_a_file (exec_prefixes,
+ char * temp_spec = find_a_file (exec_prefixes,
LTOPLUGINSONAME, R_OK,
false);
- if (!linker_plugin_file_spec)
+ if (!temp_spec)
fatal_error (-fuse-linker-plugin, but %s not found,
LTOPLUGINSONAME);
+ linker_plugin_file_spec = convert_white_space (temp_spec);
}
#endif
lto_gcc_spec = argv[0];
@@ -8506,3 +8509,52 @@
free (name);
return result;
}
+
+/* Insert back slash before spaces in orig (usually a file path), to
+ avoid being broken by spec parser.
+
+ This function is needed as do_spec_1 treats white space (' ' and
'\t')
+ as the end of an argument. But in case of -plugin /usr/gcc
install/xxx.so,
+ the filename should be treated as a single argument rather than
being
+ broken into multiple. Solution is to insert '\\' before the space in
a
+ filename.
+
+ This function converts and only converts all occurrance of ' '
+ to '\\' + ' ' and '\t' to '\\' + '\t'. For example:
+ a b - a\\ b
+ a b - a\\ \\ b
+ a\tb - a\\\tb
+ a\\ b - a b
+
+ orig: input null-terminating string that was allocated by xalloc.
The
+ memory it points to might be freed in this function. Behavior
undefined
+ if orig
