Re: [PATCH][ARM] Enable code hoisting with -Os (PR80155)

[Joey Ye]

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

[Joey Ye]

Branch created for Arm GCC releases.

- Joey

Re: [PATCH][OBVIOUS] Close file on return from verify-intermediate

[Joey Ye]

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

[Joey Ye]

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

[Joey Ye]

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

[Joey Ye]

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

[Joey Ye]

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

[Joey Ye]

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

[Joey Ye]

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

[Joey Ye]

> -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

[Joey Ye]

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

[Joey Ye]

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  
* 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: [patch, arm] Minor optimization on thumb2 tail call

[Joey Ye]

Ping

On Wed, Nov 19, 2014 at 10:43 AM, Joey Ye  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

[Joey Ye]

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  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

[Joey Ye]

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  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  
>
> * 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   

[patch, arm] Minor optimization on thumb2 tail call

[Joey Ye]

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, trivial][AArch64] Fix mode iterator for *aarch64_simd_ld1r pattern

[Joey Ye]

Can a new case be rewritten then?

- Joey

On Fri, Nov 14, 2014 at 9:32 AM, Yangfei (Felix)  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) 
>> wrote:
>> > Hi,
>> >
>> >   We find that the VALLDI mode iterator used in *aarch64_simd_ld1r
>> 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  
>> > +   Jiji Jiang  
>> > +
>> > +   * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1r):
>> Use
>> > +   VALL mode iterator instead of VALLDI.
>> > +
>> >  2014-11-11  Andrew Pinski  
>> >
>> > 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_ld1r"
>> > -  [(set (match_operand:VALLDI 0 "register_operand" "=w")
>> > -   (vec_duplicate:VALLDI
>> > +  [(set (match_operand:VALL 0 "register_operand" "=w")
>> > +   (vec_duplicate:VALL
>> >   (match_operand: 1 "aarch64_simd_struct_operand"
>> "Utv")))]
>> >"TARGET_SIMD"
>> >"ld1r\\t{%0.}, %1"

Re: [PATCH, trivial][AArch64] Fix mode iterator for *aarch64_simd_ld1r pattern

[Joey Ye]

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)  wrote:
> Hi,
>
>   We find that the VALLDI mode iterator used in *aarch64_simd_ld1r 
> 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  
> +   Jiji Jiang  
> +
> +   * config/aarch64/aarch64-simd.md (*aarch64_simd_ld1r): Use
> +   VALL mode iterator instead of VALLDI.
> +
>  2014-11-11  Andrew Pinski  
>
> 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_ld1r"
> -  [(set (match_operand:VALLDI 0 "register_operand" "=w")
> -   (vec_duplicate:VALLDI
> +  [(set (match_operand:VALL 0 "register_operand" "=w")
> +   (vec_duplicate:VALL
>   (match_operand: 1 "aarch64_simd_struct_operand" "Utv")))]
>"TARGET_SIMD"
>"ld1r\\t{%0.}, %1"

Re: [PATCH 3/5] IPA ICF pass

[Joey Ye]

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
 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 :
>> 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 f1&&f2 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 = gimp

Re: [PATCH, PR61605, 2/2] Use fuse-caller-save info in pass_cprop_hardreg

[Joey Ye]

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  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 Vries
>>
>> 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
>

[patch][plugin] Fix PR59335 - missing plugin headers again

[Joey Ye]

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: [PATCH] Don't set dir prefix twice (PR middle-end/60484)

[Joey Ye]

OK to 4.8 then?

On Thu, Aug 14, 2014 at 6:36 PM, Richard Biener
 wrote:
> On Thu, Aug 14, 2014 at 7:34 AM, Joey Ye  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
>>  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: [PATCH] Don't set dir prefix twice (PR middle-end/60484)

[Joey Ye]

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
 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: [PATCH, PR61219]: Fix sNaN handling in ARM float to double conversion

[Joey Ye]

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  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  
>
> 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

[Joey Ye]

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  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

[Joey Ye]

> -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  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

[Joey Ye]

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;
+}

RE: [patch] Disable if_conversion2 for Og

[Joey Ye]

> -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

[Joey Ye]

> -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

[Joey Ye]

> -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

[Joey Ye]

> -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

[Joey Ye]

> -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  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

[Joey Ye]

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

[Joey Ye]

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  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

[Joey Ye]

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)

[Joey Ye]

4.8 also has this bug. OK to backport?

On Tue, Mar 11, 2014 at 6:59 PM, Jakub Jelinek  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

[PATCH] [libgcc,arm] Fix PR 60166 - NAN fraction bits

[Joey Ye]

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] [arm] Fix PR60169 - thumb1 far jump

[Joey Ye]

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];
+ }
+  }
+   }
+}

RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8

[Joey Ye]

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  
> > > > > >
> > > > > > 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/

RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8

[Joey Ye]

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  
> > > > >
> > > > > 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  
> > > > >
> > > > > * expr.c (expand_assignment): Remove dependency on
> > > > > flag_strict_volatile_bitfields. Always set the memory
> > >

RE: [patch] [arm] Fix PR60169 - thumb1 far jump

[Joey Ye]

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.

[patch] [arm] Fix PR60169 - thumb1 far jump

[Joey Ye]

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][4.8] Backport strict-volatile-bitfields fixes to 4.8

[Joey Ye]

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  
> > > >
> > > > 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  
> > > >
> > > > * 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  
> > > >
> > > > 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.
> > > > (stor

[patch] Shorten Windows path

[Joey Ye]

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

[Joey Ye]

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  
> > >
> > > 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  
> > >
> > > * 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  
> > >
> > > 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/
> > >   

RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8

[Joey Ye]

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  
> >
> > 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  
> >
> > * 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  
> >
> > 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  
> >  Sandra Loosemore  
> >
> > 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.
> > (st

RE: [PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8

[Joey Ye]

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  
> 
> 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  
> 
> * 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  
> 
> 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  
>  Sandra Loosemore  
> 
> 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  
> 
> PR middle-end/59134
> * expmed.c (stor

Re: [GCC, ARM] Backport trunk patch to 4.8 to reclassify ARM preload insn

[Joey Ye]

ChangeLog is messed up with other one.

On Thu, Jan 16, 2014 at 3:33 PM, Terry Guo  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

[PATCH][4.8] Backport strict-volatile-bitfields fixes to 4.8

[Joey Ye]

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  

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  

* 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  

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  
 Sandra Loosemore  

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  
 
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

[patch] [plugin] Fix PR 59335 plugin build

[Joey Ye]

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  

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

[PATCH] [doc] Update plugin doc

[Joey Ye]

Update plugin document after switching to C++, also make it more friendly to
cross-build.

ChangeLog:
2014-01-08  Joey Ye  
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.

Re: [arm-embedded] Backport trunk thumb1 pic fix to embedded-4_8-branch

[Joey Ye]

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  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  
>
> Backport mainline r205391
> 2013-11-26  Terry Guo  
>
> * 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  
>
> Backport mainline r205391
> 2013-11-26  Terry Guo  
>
> * 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

[Joey Ye]

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

[Joey Ye]

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  wrote:
> > On Thu, Nov 14, 2013 at 2:24 AM, Joey Ye  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] ARM Cortex-M3/M4 tuning

[Joey Ye]

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

[Joey Ye]

> -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] New option for PIC offset unfixed

[Joey Ye]

> -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

[Joey Ye]

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

[Joey Ye]

> -Original Message-
> From: Richard Earnshaw
> Sent: Wednesday, November 13, 2013 17:49
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> 
> On 13/11/13 06:18, Joey Ye wrote:
> >> -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  
> >
> > * 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.
> >
> >
> > pic_data_text_rel-1113.patch.txt
> >
> >
> > 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;
> 
> Why is this needed?  Surely, even a VxWorks user should have the right to
> force the compiler to behave differently.  You've set things up through
the
> default, now just accept what the user has asked for.
The reason is that TARGET_VXWORKS_RTP isn't a compile time value to initiate
arm.opt. Instead it is true only when -mrtp is specified in runtime. Also
enable text relative may result in runtime error on vxworks, it is better to
prevent it here.
> 
> 
> > +
> > +  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");
> 
> I'm not sure about this either.  The option should just be ignored when
not
> PIC.
It is ignored if -fpic isn't given. I'm OK to remove the error here, while
Ramana preferred to error it.

> 
> > +
> >/* 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
&

RE: [patch] [arm] ARM Cortex-M3/M4 tuning

[Joey Ye]

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  
> >>  Joey Ye  
> >>
> >>  * 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  
> >>
> >>  * 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: [patch] [arm] New option for PIC offset unfixed

[Joey Ye]

> -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  

* 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

[patch] [arm] New option for PIC offset unfixed

[Joey Ye]

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  

* 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: [patch] [arm] ARM Cortex-M3/M4 tuning

[Joey Ye]

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  
> Joey Ye  
> 
> * 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  
> 
> * 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: [C] Fix PR57258: unused variable warning is emitted for volatile variables

[Joey Ye]

-  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  wrote:
> On Thu, Nov 7, 2013 at 11:53 AM, Mingjie Xing  wrote:
>> 2013/11/6 Richard Biener :
>>> 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.

[PATCH] [libiberty] MAX_PATH problems with mingw gcc

[Joey Ye]

Ping

ChangeLog

  2013-10-27  Vladimir Simonov  

(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

[Joey Ye]

+  "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  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

[Joey Ye]

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  
    Joey Ye  

* 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  

* 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

[Joey Ye]

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: FW: MAX_PATH problems with mingw gcc

[Joey Ye]

On Thu, Oct 17, 2013 at 4:18 PM, Vladimir Simonov
 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

[Joey Ye]

On Wed, Oct 16, 2013 at 6:45 PM, Vladimir Simonov
 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: FW: MAX_PATH problems with mingw gcc

[Joey Ye]

Thanks for contribution. See review comments at following.

On Wed, Oct 16, 2013 at 6:45 PM, Vladimir Simonov
 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: [PATCH, libgcc] Disable JCR section when java is not enabled

[Joey Ye]

> -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

[Joey Ye]

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  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

[Joey Ye]

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

[Joey Ye]

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  
> 
>   Backport from mainline r197956
>   2013-04-15  Joey Ye  
> 
>   * 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  
> 
>   Backport from mainline r197956
>   2013-04-15  Joey Ye  
> 
>   * 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

[Joey Ye]

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  
> 
>   Backport from mainline r197153
>   2013-03-27  Terry Guo  
> 
>   * 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

[Joey Ye]

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  
> 
>   * 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

[Joey Ye]

Committed as 199680

RE: [RFA/ARM/4.7] Fix PR54974: Thumb literal pools don't handle PC rounding

[Joey Ye]

> -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

[Joey Ye]

> -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  
> >
> > * 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

[Joey Ye]

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  
> >
> > * 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

[Joey Ye]

Committed as r196535.

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

> -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

[Joey Ye]

> -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,
+LT

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

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
> +  

[arm/embedded-4_7-branch] Merge with gcc-4_7-branch r196107

[Joey Ye]

Committed as r196116.

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

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-term

[PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

Mainline and 4.7 failed to use LTO when toolchain is installed to a path
with space in it. Resulting in error message like:

/ld.exe: c:/program: error loading plugin
collect2.exe: error: ld returned 1 exit status

Root cause is when GCC driver process specs, it doesn't handle plugin file
name properly. Here is a patch fixing it.

Bootstraped and make check on x86 and aarch32, no regression.

OK to trunk and 4.7?

ChangeLog:

PR lto/50293
* gcc.c (convert_white_space): New function.
(main): Handles white space in function name.

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 * orig);


 /* 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,30 @@
   free (name);
   return result;
 }
+
+/* Insert back slash before spaces in a string, to avoid path
+   that has space in it broken into multiple arguments.  */
+
+static char * convert_white_space(char * orig)
+{
+  int len, number_of_space = 0;
+  char *p = orig;
+  if (orig == NULL) return NULL;
+
+  for (len=0; p[len]; len++)
+if (p[len] == ' ' || p[len] == '\t') number_of_space ++;
+
+  if (number_of_space)
+{
+  char * new_spec = (char *)xmalloc(len + number_of_space + 1);
+  int j,k;
+  for (j=0, k=0; j<=len; j++, k++)
+   {
+ if (p[j] == ' ' || p[j] == '\t') new_spec[k++]='\\';
+ new_spec[k] = p[j];
+   }
+  free(CONST_CAST(char *, orig));
+  return new_spec;
+  }
+  else return orig;
+}

RE: [PATCH] [testsuite] [arm] Test thumb1 far jump

[Joey Ye]

> -Original Message-
> From: Janis Johnson [mailto:janis_john...@mentor.com]
> Sent: Thursday, January 17, 2013 10:41
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] [testsuite] [arm] Test thumb1 far jump
> 
> On 01/16/2013 06:05 PM, Joey Ye wrote:
> > Test cases for previous patch "no lr save for non-far branches in leaf
> > function".
> >
> > * gcc.target/arm/thumb1-far-jump-1.c: New.
> > * gcc.target/arm/thumb1-far-jump-2.c: New.
> >
> > Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
> > ===
> > --- gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c(revision 0)
> > +++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c(revision 0)
> > @@ -0,0 +1,58 @@
> > +/* Check for thumb1 far jump. This is the extreme case that far jump
> > + * will be used with minimum number of instructions. By passing this
> case
> > + * it means the heuristic of saving lr for far jump meets the most
> extreme
> > + * requirement.  */
> > +/* { dg-require-effective-target arm_thumb1_ok } */
> > +/* { dg-options "-Os" } */
> > +/* { dg-skip-if "" { ! { arm_thumb1 } } } */
> 
> The effective target arm_thumb1_ok returns 1 if it's OK to add -mthumb
> to the current multilib flags and together they generate code for
> Thumb-1.  arm_thumb1 says that the current multilib flags generate
> code for Thumb-1.
> 
> If you want to add -mthumb to the test then use:
> 
> /* { dg-require-effective-target arm_thumb1_ok } */
> /* { dg-options "-Os -mthumb" } */
> 
> Otherwise use
> 
> /* { dg-skip-if "" { ! arm_thumb1 } } */
> /* { dg-options "-Os" } */
This is what I intented

> 
> > +/* { dg-final { scan-assembler "push.*lr" } } */
> > Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c
> > ===
> > --- gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c(revision 0)
> > +++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c(revision 0)
> > @@ -0,0 +1,35 @@
> > +/* Check for thumb1 far jump. Shouldn't save lr for small leaf
> functions
> > + * even with a branch in it.  */
> > +/* { dg-require-effective-target arm_thumb1_ok } */
> > +/* { dg-options "-Os" } */
> > +/* { dg-skip-if "" { ! { arm_thumb1 } } } */
> 
> Same here.
> 
> The tests are OK with those changes, but please wait a day or two
> for other comments or a clear OK from an ARM maintainer.
These test cases should be committed together with
http://gcc.gnu.org/ml/gcc-patches/2013-01/msg00221.html or otherwise one of
them will fail. 

> 
> Janis
Updated patch:

Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
===
--- gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c(revision 0)
+++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c(revision 0)
@@ -0,0 +1,58 @@
+/* Check for thumb1 far jump. This is the extreme case that far jump
+ * will be used with minimum number of instructions. By passing this case
+ * it means the heuristic of saving lr for far jump meets the most extreme
+ * requirement.  */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "" { ! { arm_thumb1 } } } */
+
+volatile register r4 asm("r4");
+void f3(int i)
+{
+#define GO(n) \
+  extern volatile int g_##n; \
+  r4=(int)&g_##n;
+
+#define GO8(n) \
+  GO(n##_0) \
+  GO(n##_1) \
+  GO(n##_2) \
+  GO(n##_3) \
+  GO(n##_4) \
+  GO(n##_5) \
+  GO(n##_6) \
+  GO(n##_7)
+
+#define GO64(n) \
+  GO8(n##_0) \
+  GO8(n##_1) \
+  GO8(n##_2) \
+  GO8(n##_3) \
+  GO8(n##_4) \
+  GO8(n##_5) \
+  GO8(n##_6) \
+  GO8(n##_7) \
+
+#define GO498(n) \
+  GO64(n##_0) \
+  GO64(n##_1) \
+  GO64(n##_2) \
+  GO64(n##_3) \
+  GO64(n##_4) \
+  GO64(n##_5) \
+  GO64(n##_6) \
+  GO8(n##_0) \
+  GO8(n##_1) \
+  GO8(n##_2) \
+  GO8(n##_3) \
+  GO8(n##_4) \
+  GO8(n##_5) \
+  GO(n##_0) \
+  GO(n##_1) \
+
+  if (i) {
+GO498(0);
+  }
+}
+
+/* { dg-final { scan-assembler "push.*lr" } } */
Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c
===
--- gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c(revision 0)
+++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c(revision 0)
@@ -0,0 +1,35 @@
+/* Check for thumb1 far jump. Shouldn't save lr for small leaf functions
+ * even with a branch in it.  */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "" { ! { arm_thumb1 } } } */
+
+void f()
+{
+  for (;;);
+}
+
+volatile int g;
+void f2(int i)
+{
+  if (i) g=0;
+}
+
+void f3(int i)
+{
+  if (i) {
+g=0;
+g=1;
+g=2;
+g=3;
+g=4;
+g=5;
+g=6;
+g=7;
+g=8;
+g=9;
+  }
+}
+
+/* { dg-final { scan-assembler-not "push.*lr" } } */
+

[PATCH] [testsuite] [arm] Test thumb1 far jump

[Joey Ye]

Test cases for previous patch "no lr save for non-far branches in leaf
function".

* gcc.target/arm/thumb1-far-jump-1.c: New.
* gcc.target/arm/thumb1-far-jump-2.c: New.

Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
===
--- gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c(revision 0)
+++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c(revision 0)
@@ -0,0 +1,58 @@
+/* Check for thumb1 far jump. This is the extreme case that far jump
+ * will be used with minimum number of instructions. By passing this case
+ * it means the heuristic of saving lr for far jump meets the most extreme
+ * requirement.  */
+/* { dg-require-effective-target arm_thumb1_ok } */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "" { ! { arm_thumb1 } } } */
+
+volatile register r4 asm("r4");
+void f3(int i)
+{
+#define GO(n) \
+  extern volatile int g_##n; \
+  r4=(int)&g_##n;
+
+#define GO8(n) \
+  GO(n##_0) \
+  GO(n##_1) \
+  GO(n##_2) \
+  GO(n##_3) \
+  GO(n##_4) \
+  GO(n##_5) \
+  GO(n##_6) \
+  GO(n##_7)
+
+#define GO64(n) \
+  GO8(n##_0) \
+  GO8(n##_1) \
+  GO8(n##_2) \
+  GO8(n##_3) \
+  GO8(n##_4) \
+  GO8(n##_5) \
+  GO8(n##_6) \
+  GO8(n##_7) \
+
+#define GO498(n) \
+  GO64(n##_0) \
+  GO64(n##_1) \
+  GO64(n##_2) \
+  GO64(n##_3) \
+  GO64(n##_4) \
+  GO64(n##_5) \
+  GO64(n##_6) \
+  GO8(n##_0) \
+  GO8(n##_1) \
+  GO8(n##_2) \
+  GO8(n##_3) \
+  GO8(n##_4) \
+  GO8(n##_5) \
+  GO(n##_0) \
+  GO(n##_1) \
+
+  if (i) {
+GO498(0);
+  }
+}
+
+/* { dg-final { scan-assembler "push.*lr" } } */
Index: gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c
===
--- gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c(revision 0)
+++ gcc/testsuite/gcc.target/arm/thumb1-far-jump-1.c(revision 0)
@@ -0,0 +1,35 @@
+/* Check for thumb1 far jump. Shouldn't save lr for small leaf functions
+ * even with a branch in it.  */
+/* { dg-require-effective-target arm_thumb1_ok } */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "" { ! { arm_thumb1 } } } */
+
+void f()
+{
+  for (;;);
+}
+
+volatile int g;
+void f2(int i)
+{
+  if (i) g=0;
+}
+
+void f3(int i)
+{
+  if (i) {
+g=0;
+g=1;
+g=2;
+g=3;
+g=4;
+g=5;
+g=6;
+g=7;
+g=8;
+g=9;
+  }
+}
+
+/* { dg-final { scan-assembler-not "push.*lr" } } */
+

RE: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump

[Joey Ye]

Ping^2

> -Original Message-
> From: Joey Ye
> Sent: Saturday, January 05, 2013 15:41
> 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  
> >
> > * 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;
> >
> >
> >
> 
> 

RE: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump

[Joey Ye]

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  
> 
>   * 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;
> 
> 
> 

[PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump

[Joey Ye]

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  

* 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;

[PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump

[Joey Ye]

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

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;

RE: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c

[Joey Ye]

> -Original Message-
> From: H.J. Lu [mailto:hjl.to...@gmail.com]
> Sent: Tuesday, December 18, 2012 12:10
> To: Joseph Prostko
> Cc: Joey Ye; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c
> 
> On Mon, Dec 17, 2012 at 5:23 PM, Joseph Prostko 
> wrote:
> > On Mon, Dec 17, 2012 at 2:28 PM, H.J. Lu  wrote:
> >> On Mon, Dec 17, 2012 at 1:50 AM, Joey Ye  wrote:
> >
> >>>   * libgcc/Makefile.in: Include TARGET_USE_JCR_SECTION in CFLAGS.
> >>>   * libgcc/configure.ac (use_jcr_section): New variable.
> >>>   * libgcc/configure: Regenerated.
> >>>   * libgcc/crtstuff.c: Check TARGET_USE_JCR_SECTION.
> >>
> >>
> >> I would use JAVA_IS_ENABLED instead of TARGET_USE_JCR_SECTION.
> >> But it is only my personal preference.
> >
> > I believe Joey did that to be consistent with the
> > TARGET_USE_JCR_SECTION macro used in gcc/defaults.h that can be turned
> > on or off for targets that specify it .  If JAVA_IS_ENABLED is used
> > instead, should TARGET_USE_JCR_SECTION be deprecated?
> >
> 
> TARGET_USE_JCR_SECTION determines whether to use the JCR section
> to register Java classes. We don't need to do anything for Java if Java
> isn't enabled. The change can be as simple as
> 
> #ifndef JAVA_IS_ENABLED
> #undef JCR_SECTION_NAME
> #endif
> 
> in crtstuff.c.  Can you give it a try?
Tried and it simply worked. But undef JCR_SECTION_NAME here is very hacking.

> 
> 
> --
> H.J.

RE: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c

[Joey Ye]

> -Original Message-
> From: H.J. Lu [mailto:hjl.to...@gmail.com]
> Sent: Saturday, December 15, 2012 01:20
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org; Joseph Prostko
> Subject: Re: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c
> Can't you do
> 
> +# Disable jcr section if we're not building java
> +case ,${enable_languages}, in
> +  *java*)
> +use_jcr_section=1
> +;;
> +  *)
> +use_jcr_section=0
> +;;
> +esac
> 
> in libgcc/configure.ac?
> 
> BTW, checking *,java,* is wrong for
> 
> --enable-languages=c,c++,java
Oh yes, it works. I didn't expect top level configure expands
--enable-languages=all to individual languages. Patch simplified.

However, I noticed that patterns like "*,java,*)" are widely used in
configure and they do work. Can you explain more why it is wrong?

  * libgcc/Makefile.in: Include TARGET_USE_JCR_SECTION in CFLAGS.
  * libgcc/configure.ac (use_jcr_section): New variable.
  * libgcc/configure: Regenerated.
  * libgcc/crtstuff.c: Check TARGET_USE_JCR_SECTION.

jcr_diable_non_java-1217.patch
Description: Binary data

RE: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c

[Joey Ye]

> -Original Message-
> From: H.J. Lu [mailto:hjl.to...@gmail.com]
> Sent: Friday, December 14, 2012 11:55
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org; Joseph Prostko
> Subject: Re: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c
> 
> > 2012-12-12  Joey Ye  
> >
> >  * configure.ac (enable-jcr-section): New target_configargs.
> >  * configure: Regenerated.
> >  * libgcc/Makefile.in: Include TARGET_USE_JCR_SECTION in CFLAGS.
> >  * libgcc/configure.ac (use_jcr_section): New variable.
> >  * libgcc/configure: Regenerated.
> >  * libgcc/crtstuff.c: Check TARGET_USE_JCR_SECTION.
> 
> Why do we need a new configure option at toplevel?
> Can't you check --enable-languages=.. in libgcc?
Although --enable-languages=... is passed to sub-configure, it needs a whole
bunch of code to process with consideration of --disable-languages and
targets. These code doesn't exist in libgcc/configure, and I intend not to
duplicate them.

- Joey

RE: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c

[Joey Ye]

> -Original Message-
> From: H.J. Lu [mailto:hjl.to...@gmail.com]
> Sent: Tuesday, November 27, 2012 12:56
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c
> >> >
> >> > OK to trunk?
> >> >
> >> > 2012-09-21  Joey Ye  
> >> >
> >> > * crtstuff.c: Check TARGET_USE_JCR_SECTION.
> >> >
> 
> Since we have --enable-languages=.., can't we disable
> JCR_SECTION section if java isn't enabled?
Updated configure disabling jcr section usage if not configured with java.

2012-12-12  Joey Ye  

 * configure.ac (enable-jcr-section): New target_configargs.
 * configure: Regenerated.
 * libgcc/Makefile.in: Include TARGET_USE_JCR_SECTION in CFLAGS.
 * libgcc/configure.ac (use_jcr_section): New variable.
 * libgcc/configure: Regenerated.
 * libgcc/crtstuff.c: Check TARGET_USE_JCR_SECTION.

jcr_diable_non_java-1212.patch
Description: Binary data

RE: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c

[Joey Ye]

Ping^2

> -Original Message-
> From: Joey Ye
> Sent: Tuesday, November 20, 2012 10:09
> To: gcc-patches@gcc.gnu.org
> Cc: Joey Ye
> Subject: RE: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c
> 
> Ping, as Joseph Prostko is saying that this patch shall solve the same
> problem he's facing.
> 
> > -----Original Message-
> > From: Joey Ye [mailto:joey...@arm.com]
> > Sent: Friday, September 21, 2012 15:42
> > To: gcc-patches@gcc.gnu.org
> > Subject: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c
> >
> > Current crtstuff.c checks if JCR_SECTION_NAME is defined to decide
> > whether
> > do work for JCR. However, defaults.h always defines JCR_SECTION_NAME:
> >
> > #ifndef JCR_SECTION_NAME
> > #define JCR_SECTION_NAME ".jcr"
> > #endif
> >
> > So it is impossible to disable JCR related code in crtbegin.o, which
> > can
> > save some bytes for every applications that doesn't need java.
> >
> > This patch revise the check of JCR_SECTION_NAME to
> > TARGET_USE_JCR_SECTION.
> > By defining latter to zero disable JCR in crtstuff. This change
> doesn't
> > impact logic of any target given following defines in defaults.h:
> >
> > #ifndef TARGET_USE_JCR_SECTION
> > #ifdef JCR_SECTION_NAME
> > #define TARGET_USE_JCR_SECTION 1
> > #else
> > #define TARGET_USE_JCR_SECTION 0
> > #endif
> > #endif
> >
> > Again, this patch doesn't impact libgcc on any target, unless
> > TARGET_USE_JCR_SECTION is explicitly defined to 0 with make
> > CFLAGS_FOR_TARGET=-DTARGET_USE_JCR_SECTION=0. AIX defines
> > TARGET_USE_JCR_SECTION to 0, but it has no crtbegin/end stuff. So also
> > no impact.
> >
> > OK to trunk?
> >
> > 2012-09-21  Joey Ye  
> >
> > * crtstuff.c: Check TARGET_USE_JCR_SECTION.
> >
> > Index: libgcc/crtstuff.c
> > ===
> > --- libgcc/crtstuff.c   (revision 190556)
> > +++ libgcc/crtstuff.c   (working copy)
> > @@ -256,13 +256,13 @@
> >   = { };
> >  #endif /* USE_EH_FRAME_REGISTRY */
> >
> > -#ifdef JCR_SECTION_NAME
> > +#if TARGET_USE_JCR_SECTION && defined (JCR_SECTION_NAME)
> >  /* Stick a label at the beginning of the java class registration info
> > so we can register them properly.  */
> >  STATIC void *__JCR_LIST__[]
> >__attribute__ ((used, section(JCR_SECTION_NAME),
> > aligned(sizeof(void*
> >= { };
> > -#endif /* JCR_SECTION_NAME */
> > +#endif /* TARGET_USE_JCR_SECTION && JCR_SECTION_NAME */
> >
> >  #if USE_TM_CLONE_REGISTRY
> >  STATIC func_ptr __TMC_LIST__[]
> > @@ -438,7 +438,7 @@
> >  #endif
> >
> >  #if defined(USE_EH_FRAME_REGISTRY) \
> > -|| defined(JCR_SECTION_NAME) \
> > +|| defined(TARGET_USE_JCR_SECTION) \
> >  || defined(USE_TM_CLONE_REGISTRY)
> >  /* Stick a call to __register_frame_info into the .init section.  For
> > some
> > reason calls with no arguments work more reliably in .init, so
> > stick the
> > @@ -461,7 +461,7 @@
> >  #endif /* CRT_GET_RFIB_DATA */
> >  #endif /* USE_EH_FRAME_REGISTRY */
> >
> > -#ifdef JCR_SECTION_NAME
> > +#if TARGET_USE_JCR_SECTION
> >if (__JCR_LIST__[0])
> >  {
> >void (*register_classes) (void *) = _Jv_RegisterClasses;
> > @@ -469,7 +469,7 @@
> >if (register_classes)
> > register_classes (__JCR_LIST__);
> >  }
> > -#endif /* JCR_SECTION_NAME */
> > +#endif /* TARGET_USE_JCR_SECTION */
> >
> >  #if USE_TM_CLONE_REGISTRY
> >register_tm_clones ();
> > @@ -483,7 +483,7 @@
> >__attribute__ ((__used__, section(".init_array"),
> > aligned(sizeof(func_ptr
> >= { frame_dummy };
> >  #endif /* !defined(INIT_SECTION_ASM_OP) */
> > -#endif /* USE_EH_FRAME_REGISTRY || JCR_SECTION_NAME ||
> > USE_TM_CLONE_REGISTRY */
> > +#endif /* USE_EH_FRAME_REGISTRY || TARGET_USE_JCR_SECTION ||
> > USE_TM_CLONE_REGISTRY */
> >
> >  #else  /* OBJECT_FORMAT_ELF */
> >
> > @@ -551,7 +551,7 @@
> >  }
> >
> >  #if defined(USE_EH_FRAME_REGISTRY) \
> > -|| defined(JCR_SECTION_NAME) \
> > +|| defined(TARGET_USE_JCR_SECTION) \
> >  || defined(USE_TM_CLONE_REGISTRY)
> >  /* A helper function for __do_global_ctors, which is in crtend.o.
> > Here
> > in crtbegin.o, we can reference a couple of symbols not visible
> >

RE: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c

[Joey Ye]

Ping, as Joseph Prostko is saying that this patch shall solve the same
problem he's facing.

> -Original Message-
> From: Joey Ye [mailto:joey...@arm.com]
> Sent: Friday, September 21, 2012 15:42
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH, libgcc] Make possible to disable JCR in crtstuff.c
> 
> Current crtstuff.c checks if JCR_SECTION_NAME is defined to decide
> whether
> do work for JCR. However, defaults.h always defines JCR_SECTION_NAME:
> 
> #ifndef JCR_SECTION_NAME
> #define JCR_SECTION_NAME ".jcr"
> #endif
> 
> So it is impossible to disable JCR related code in crtbegin.o, which
> can
> save some bytes for every applications that doesn't need java.
> 
> This patch revise the check of JCR_SECTION_NAME to
> TARGET_USE_JCR_SECTION.
> By defining latter to zero disable JCR in crtstuff. This change doesn't
> impact logic of any target given following defines in defaults.h:
> 
> #ifndef TARGET_USE_JCR_SECTION
> #ifdef JCR_SECTION_NAME
> #define TARGET_USE_JCR_SECTION 1
> #else
> #define TARGET_USE_JCR_SECTION 0
> #endif
> #endif
> 
> Again, this patch doesn't impact libgcc on any target, unless
> TARGET_USE_JCR_SECTION is explicitly defined to 0 with make
> CFLAGS_FOR_TARGET=-DTARGET_USE_JCR_SECTION=0. AIX defines
> TARGET_USE_JCR_SECTION to 0, but it has no crtbegin/end stuff. So also
> no impact.
> 
> OK to trunk?
> 
> 2012-09-21  Joey Ye  
> 
>   * crtstuff.c: Check TARGET_USE_JCR_SECTION.
> 
> Index: libgcc/crtstuff.c
> ===
> --- libgcc/crtstuff.c (revision 190556)
> +++ libgcc/crtstuff.c (working copy)
> @@ -256,13 +256,13 @@
>   = { };
>  #endif /* USE_EH_FRAME_REGISTRY */
> 
> -#ifdef JCR_SECTION_NAME
> +#if TARGET_USE_JCR_SECTION && defined (JCR_SECTION_NAME)
>  /* Stick a label at the beginning of the java class registration info
> so we can register them properly.  */
>  STATIC void *__JCR_LIST__[]
>__attribute__ ((used, section(JCR_SECTION_NAME),
> aligned(sizeof(void*
>= { };
> -#endif /* JCR_SECTION_NAME */
> +#endif /* TARGET_USE_JCR_SECTION && JCR_SECTION_NAME */
> 
>  #if USE_TM_CLONE_REGISTRY
>  STATIC func_ptr __TMC_LIST__[]
> @@ -438,7 +438,7 @@
>  #endif
> 
>  #if defined(USE_EH_FRAME_REGISTRY) \
> -|| defined(JCR_SECTION_NAME) \
> +|| defined(TARGET_USE_JCR_SECTION) \
>  || defined(USE_TM_CLONE_REGISTRY)
>  /* Stick a call to __register_frame_info into the .init section.  For
> some
> reason calls with no arguments work more reliably in .init, so
> stick the
> @@ -461,7 +461,7 @@
>  #endif /* CRT_GET_RFIB_DATA */
>  #endif /* USE_EH_FRAME_REGISTRY */
> 
> -#ifdef JCR_SECTION_NAME
> +#if TARGET_USE_JCR_SECTION
>if (__JCR_LIST__[0])
>  {
>void (*register_classes) (void *) = _Jv_RegisterClasses;
> @@ -469,7 +469,7 @@
>if (register_classes)
>   register_classes (__JCR_LIST__);
>  }
> -#endif /* JCR_SECTION_NAME */
> +#endif /* TARGET_USE_JCR_SECTION */
> 
>  #if USE_TM_CLONE_REGISTRY
>register_tm_clones ();
> @@ -483,7 +483,7 @@
>__attribute__ ((__used__, section(".init_array"),
> aligned(sizeof(func_ptr
>= { frame_dummy };
>  #endif /* !defined(INIT_SECTION_ASM_OP) */
> -#endif /* USE_EH_FRAME_REGISTRY || JCR_SECTION_NAME ||
> USE_TM_CLONE_REGISTRY */
> +#endif /* USE_EH_FRAME_REGISTRY || TARGET_USE_JCR_SECTION ||
> USE_TM_CLONE_REGISTRY */
> 
>  #else  /* OBJECT_FORMAT_ELF */
> 
> @@ -551,7 +551,7 @@
>  }
> 
>  #if defined(USE_EH_FRAME_REGISTRY) \
> -|| defined(JCR_SECTION_NAME) \
> +|| defined(TARGET_USE_JCR_SECTION) \
>  || defined(USE_TM_CLONE_REGISTRY)
>  /* A helper function for __do_global_ctors, which is in crtend.o.
> Here
> in crtbegin.o, we can reference a couple of symbols not visible
> there.
> @@ -566,7 +566,7 @@
>  __register_frame_info (__EH_FRAME_BEGIN__, &object);
>  #endif
> 
> -#ifdef JCR_SECTION_NAME
> +#if TARGET_USE_JCR_SECTION
>if (__JCR_LIST__[0])
>  {
>void (*register_classes) (void *) = _Jv_RegisterClasses;
> @@ -580,7 +580,7 @@
>register_tm_clones ();
>  #endif /* USE_TM_CLONE_REGISTRY */
>  }
> -#endif /* USE_EH_FRAME_REGISTRY || JCR_SECTION_NAME ||
> USE_TM_CLONE_REGISTRY */
> +#endif /* USE_EH_FRAME_REGISTRY || TARGET_USE_JCR_SECTION ||
> USE_TM_CLONE_REGISTRY */
> 
>  #else /* ! INIT_SECTION_ASM_OP && ! HAS_INIT_SECTION */
>  #error "What are you doing with crtstuff.c, then?"
> @@ -656,13 +656,13 @@
>   = { 0 };
>  #endif /* EH_FRAME_SECTION_NAME */
&