public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
@ 2017-04-18 10:38 Bin Cheng
  2017-04-24 21:23 ` Jeff Law
  2017-05-03  7:33 ` Eric Botcazou
  0 siblings, 2 replies; 10+ messages in thread
From: Bin Cheng @ 2017-04-18 10:38 UTC (permalink / raw)
  To: gcc-patches; +Cc: nd

[-- Attachment #1: Type: text/plain, Size: 4368 bytes --]

Hi,
This patch series rewrites parts of IVOPTs.  The change consists of below described parts:
  A) New cost computation model.  Currently, there are big amount code trying to understand
     tree expression and estimate its computation cost.  The model is designed long ago
     for generic tree expressions.  In order to process generic expression (even address
     expression of array/memory references), it has code for too many corner cases.  The
     problem is it's somehow impossible to handle all complicated expressions, even with
     complicated logic in functions like get_computation_cost_at, difference_cost,
     ptr_difference_cost, get_address_cost and so on...  The second problem is it's hard
     to keep cost model consistent among special cases.  As special cases being added
     from time to time, the model is no long unified any more.  There are cases that right
     cost results in bad code, or vice versa, wrong cost results in good code.  Finally,
     it's also difficult to add code for new cases.
     This patch introduces a new cost computation model by using tree affine.  Tree exprs
     are lowered to aff_tree which is simple arithmetic operation usually.  Code handling
     special cases is no longer necessary, which brings us quite simplicity.  It is also
     easier to compute consistent costs among different expressions using tree affine,
     which gives us a unified cost model.
     This change is implemented in [PATCH rewrite-cost-computation-*.txt].
  B) In rewriting both nonlinear iv_use and address iv_use, current code does bad association
     by mixing computation of invariant and induction.  This introduces inconsistency
     between cost computation and code generation because costs of invariant and induction
     are computed separately.  This also prevents loop inv from being hoisted out of loop.
     This change fixes the issue by re-associating invariant and induction parts separately
     for both nonlinear and address iv_use.
     This patch is implemented in two patches:
     [PATCH nonlinear-iv_use-rewrite-*.txt]
     [PATCH address-iv_use-rewrite-*.txt]
  C) Current implementation shares the same register pressure computation with RTL loop
     inv pass.  It has difficulty in handling (especially large) loop nest, and quite
     often generating too many candidates (especially for outer loops).  This change
     introduces new register pressure estimation.  The brief idea is to differentiate
     (hot) innermost loop and outer loop.  for (possibly hot) innermost loop, more registers
     are allowed as long as overall register pressure is within the range of number of
     target available registers.
     This change is implemented in below patches:
     [PATCH record-newly-used-inv_var-*.txt]
     [PATCH skip-non_int-phi-reg-pressure-*.txt]
     [PATCH ivopt-reg_pressure-model-*.txt]
  D) Other small refactors and improvements.  These will be described in each patch's review
     message.
  E) Patches allow better induction variable optimizations for vectorized loops.  These
     patches are blocked at the moment because current IVOPTs implementation can generate
     worse code on targets with limited addressing mode support.
     [PATCH range_info-for-vect_loop-niters-*.txt]
     [PATCH pr69710-*.txt]

As a bonus, issues like PR53090/PR71361 are now fixed with better code generation than what
the two PRs were expecting.

I collected spec2k6 data on my local AArch64 and X86_64 machines.  Overall FP is improved
+1% on both machines; while INT mainly remains neutral.  I think part of improvement comes
from IVOPTs itself, and rest of it comes from opportunities enabled as described by E).  Also It
would be great if other targets can run some benchmarks with this patch series in case of any
performance breakage.

The patch series is bootstrap and test on X86_64 and AArch64, no real regression found,
though some tests do need further adjustment.

As the start, this is the first patch of the series.  It simply handles TRUNCATE between
tieable modes in rtx_cost.  Since we don't need additional instruction for such truncate,
it simply return 0 cost.

Is it OK?

Thanks,
bin

2017-04-11  Bin Cheng  <bin.cheng@arm.com>

	* rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.

[-- Attachment #2: 0001-no_cost-for-tieable-type-truncate-20170220.txt --]
[-- Type: text/plain, Size: 864 bytes --]

From d9b17e5d303d5fb1c75f489753b4578f8c907453 Mon Sep 17 00:00:00 2001
From: Bin Cheng <binche01@e108451-lin.cambridge.arm.com>
Date: Mon, 27 Feb 2017 14:51:56 +0000
Subject: [PATCH 01/33] no_cost-for-tieable-type-truncate-20170220.txt

---
 gcc/rtlanal.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index acb4230..6019c3e 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4146,6 +4146,14 @@ rtx_cost (rtx x, machine_mode mode, enum rtx_code outer_code,
 	return COSTS_N_INSNS (2 + factor);
       break;
 
+    case TRUNCATE:
+      /* If we can tie these modes, make this cheap.  */
+      if (MODES_TIEABLE_P (mode, GET_MODE (SUBREG_REG (x))))
+	{
+	  total = 0;
+	  break;
+	}
+      /* FALLTHRU */
     default:
       if (targetm.rtx_costs (x, mode, outer_code, opno, &total, speed))
 	return total;
-- 
1.9.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-04-18 10:38 [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost Bin Cheng
@ 2017-04-24 21:23 ` Jeff Law
  2017-05-03  7:33 ` Eric Botcazou
  1 sibling, 0 replies; 10+ messages in thread
From: Jeff Law @ 2017-04-24 21:23 UTC (permalink / raw)
  To: Bin Cheng, gcc-patches; +Cc: nd

On 04/18/2017 04:37 AM, Bin Cheng wrote:
> Is it OK?
> 
> Thanks,
> bin
> 
> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
> 
> 	* rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
This is fine.  You might consider adding tests for this kind of change, 
but I also realize they could end up being pretty fragile.  Hmm, maybe 
they would be better as unit tests of rtx_cost?

jeff

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-04-18 10:38 [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost Bin Cheng
  2017-04-24 21:23 ` Jeff Law
@ 2017-05-03  7:33 ` Eric Botcazou
  2017-05-03  9:00   ` Bin.Cheng
  1 sibling, 1 reply; 10+ messages in thread
From: Eric Botcazou @ 2017-05-03  7:33 UTC (permalink / raw)
  To: Bin Cheng; +Cc: gcc-patches, nd

> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
> 
> 	* rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.

This breaks bootstrap with RTL checking:

/home/eric/build/gcc/native/./gcc/xgcc -B/home/eric/build/gcc/native/./gcc/ -
nostdinc -x c /dev/null -S -o /dev/null -fself-
test=/home/eric/svn/gcc/gcc/testsuite/selftests
cc1: internal compiler error: RTL check: expected code 'subreg', have 
'truncate' in rtx_cost, at rtlanal.c:4169
0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int, 
char const*)
        /home/eric/svn/gcc/gcc/rtl.c:829
0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
        /home/eric/svn/gcc/gcc/rtlanal.c:4169
0x8517e6 set_src_cost
        /home/eric/svn/gcc/gcc/rtl.h:2685
0x8517e6 init_expmed_one_conv
        /home/eric/svn/gcc/gcc/expmed.c:142
0x8517e6 init_expmed_one_mode
        /home/eric/svn/gcc/gcc/expmed.c:209
0x853fb2 init_expmed()
        /home/eric/svn/gcc/gcc/expmed.c:270
0xc45974 backend_init_target
        /home/eric/svn/gcc/gcc/toplev.c:1665
0xc45974 initialize_rtl()

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-05-03  7:33 ` Eric Botcazou
@ 2017-05-03  9:00   ` Bin.Cheng
  2017-05-03 10:07     ` Bin.Cheng
  0 siblings, 1 reply; 10+ messages in thread
From: Bin.Cheng @ 2017-05-03  9:00 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches List

On Wed, May 3, 2017 at 7:17 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>
>>       * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>
> This breaks bootstrap with RTL checking:
>
> /home/eric/build/gcc/native/./gcc/xgcc -B/home/eric/build/gcc/native/./gcc/ -
> nostdinc -x c /dev/null -S -o /dev/null -fself-
> test=/home/eric/svn/gcc/gcc/testsuite/selftests
> cc1: internal compiler error: RTL check: expected code 'subreg', have
> 'truncate' in rtx_cost, at rtlanal.c:4169
> 0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int,
> char const*)
>         /home/eric/svn/gcc/gcc/rtl.c:829
> 0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
>         /home/eric/svn/gcc/gcc/rtlanal.c:4169
> 0x8517e6 set_src_cost
>         /home/eric/svn/gcc/gcc/rtl.h:2685
> 0x8517e6 init_expmed_one_conv
>         /home/eric/svn/gcc/gcc/expmed.c:142
> 0x8517e6 init_expmed_one_mode
>         /home/eric/svn/gcc/gcc/expmed.c:209
> 0x853fb2 init_expmed()
>         /home/eric/svn/gcc/gcc/expmed.c:270
> 0xc45974 backend_init_target
>         /home/eric/svn/gcc/gcc/toplev.c:1665
> 0xc45974 initialize_rtl()
>
Sorry for disturbing, I will revert this if can't fix today.

Thanks,
bin
> --
> Eric Botcazou

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-05-03  9:00   ` Bin.Cheng
@ 2017-05-03 10:07     ` Bin.Cheng
  2017-05-03 10:12       ` Kyrill Tkachov
  0 siblings, 1 reply; 10+ messages in thread
From: Bin.Cheng @ 2017-05-03 10:07 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches List

[-- Attachment #1: Type: text/plain, Size: 1672 bytes --]

On Wed, May 3, 2017 at 9:38 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, May 3, 2017 at 7:17 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>>
>>>       * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>
>> This breaks bootstrap with RTL checking:
>>
>> /home/eric/build/gcc/native/./gcc/xgcc -B/home/eric/build/gcc/native/./gcc/ -
>> nostdinc -x c /dev/null -S -o /dev/null -fself-
>> test=/home/eric/svn/gcc/gcc/testsuite/selftests
>> cc1: internal compiler error: RTL check: expected code 'subreg', have
>> 'truncate' in rtx_cost, at rtlanal.c:4169
>> 0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int,
>> char const*)
>>         /home/eric/svn/gcc/gcc/rtl.c:829
>> 0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
>>         /home/eric/svn/gcc/gcc/rtlanal.c:4169
>> 0x8517e6 set_src_cost
>>         /home/eric/svn/gcc/gcc/rtl.h:2685
>> 0x8517e6 init_expmed_one_conv
>>         /home/eric/svn/gcc/gcc/expmed.c:142
>> 0x8517e6 init_expmed_one_mode
>>         /home/eric/svn/gcc/gcc/expmed.c:209
>> 0x853fb2 init_expmed()
>>         /home/eric/svn/gcc/gcc/expmed.c:270
>> 0xc45974 backend_init_target
>>         /home/eric/svn/gcc/gcc/toplev.c:1665
>> 0xc45974 initialize_rtl()
>>
> Sorry for disturbing, I will revert this if can't fix today.
It looks bogus and I couldn't find the motivating case for it, so
revert with attached patch.  Build on x86 and commit as obvious.

Thanks,
bin
2017-05-03  Bin Cheng  <bin.cheng@arm.com>

    Revert
    2017-05-02  Bin Cheng  <bin.cheng@arm.com>
    * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.

[-- Attachment #2: revert-247509.txt --]
[-- Type: text/plain, Size: 550 bytes --]

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index f18245f..321363f 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4164,14 +4164,6 @@ rtx_cost (rtx x, machine_mode mode, enum rtx_code outer_code,
 	return COSTS_N_INSNS (2 + factor);
       break;
 
-    case TRUNCATE:
-      /* If we can tie these modes, make this cheap.  */
-      if (MODES_TIEABLE_P (mode, GET_MODE (SUBREG_REG (x))))
-	{
-	  total = 0;
-	  break;
-	}
-      /* FALLTHRU */
     default:
       if (targetm.rtx_costs (x, mode, outer_code, opno, &total, speed))
 	return total;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-05-03 10:07     ` Bin.Cheng
@ 2017-05-03 10:12       ` Kyrill Tkachov
  2017-05-03 10:12         ` Bin.Cheng
  2017-05-10 14:38         ` Bin.Cheng
  0 siblings, 2 replies; 10+ messages in thread
From: Kyrill Tkachov @ 2017-05-03 10:12 UTC (permalink / raw)
  To: Bin.Cheng, Eric Botcazou; +Cc: gcc-patches List

Hi Bin,

On 03/05/17 11:02, Bin.Cheng wrote:
> On Wed, May 3, 2017 at 9:38 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Wed, May 3, 2017 at 7:17 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>>>
>>>>        * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>> This breaks bootstrap with RTL checking:
>>>
>>> /home/eric/build/gcc/native/./gcc/xgcc -B/home/eric/build/gcc/native/./gcc/ -
>>> nostdinc -x c /dev/null -S -o /dev/null -fself-
>>> test=/home/eric/svn/gcc/gcc/testsuite/selftests
>>> cc1: internal compiler error: RTL check: expected code 'subreg', have
>>> 'truncate' in rtx_cost, at rtlanal.c:4169
>>> 0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int,
>>> char const*)
>>>          /home/eric/svn/gcc/gcc/rtl.c:829
>>> 0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
>>>          /home/eric/svn/gcc/gcc/rtlanal.c:4169
>>> 0x8517e6 set_src_cost
>>>          /home/eric/svn/gcc/gcc/rtl.h:2685
>>> 0x8517e6 init_expmed_one_conv
>>>          /home/eric/svn/gcc/gcc/expmed.c:142
>>> 0x8517e6 init_expmed_one_mode
>>>          /home/eric/svn/gcc/gcc/expmed.c:209
>>> 0x853fb2 init_expmed()
>>>          /home/eric/svn/gcc/gcc/expmed.c:270
>>> 0xc45974 backend_init_target
>>>          /home/eric/svn/gcc/gcc/toplev.c:1665
>>> 0xc45974 initialize_rtl()
>>>
>> Sorry for disturbing, I will revert this if can't fix today.
> It looks bogus and I couldn't find the motivating case for it, so
> revert with attached patch.  Build on x86 and commit as obvious.
>
> Thanks,
> bin
> 2017-05-03  Bin Cheng  <bin.cheng@arm.com>
>
>      Revert
>      2017-05-02  Bin Cheng  <bin.cheng@arm.com>
>      * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.

Looking at the code in the patch...

+    case TRUNCATE:
+      /* If we can tie these modes, make this cheap.  */
+      if (MODES_TIEABLE_P (mode, GET_MODE (SUBREG_REG (x))))

'code' here is GET_CODE (x) and in this case it is TRUNCATE.
SUBREG_REG asserts (in RTL checking mode) that its argument is a SUBREG, so passing it a TRUNCATE rtx would cause
the checking failure Eric reported. I think you meant to use XEXP (x, 0)  instead of SUBREG_REG (x) ?

Thanks,
Kyrill

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-05-03 10:12       ` Kyrill Tkachov
@ 2017-05-03 10:12         ` Bin.Cheng
  2017-05-03 14:38           ` Christophe Lyon
  2017-05-10 14:38         ` Bin.Cheng
  1 sibling, 1 reply; 10+ messages in thread
From: Bin.Cheng @ 2017-05-03 10:12 UTC (permalink / raw)
  To: Kyrill Tkachov; +Cc: Eric Botcazou, gcc-patches List

On Wed, May 3, 2017 at 11:09 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi Bin,
>
>
> On 03/05/17 11:02, Bin.Cheng wrote:
>>
>> On Wed, May 3, 2017 at 9:38 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>
>>> On Wed, May 3, 2017 at 7:17 AM, Eric Botcazou <ebotcazou@adacore.com>
>>> wrote:
>>>>>
>>>>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>>>>
>>>>>        * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>>>
>>>> This breaks bootstrap with RTL checking:
>>>>
>>>> /home/eric/build/gcc/native/./gcc/xgcc
>>>> -B/home/eric/build/gcc/native/./gcc/ -
>>>> nostdinc -x c /dev/null -S -o /dev/null -fself-
>>>> test=/home/eric/svn/gcc/gcc/testsuite/selftests
>>>> cc1: internal compiler error: RTL check: expected code 'subreg', have
>>>> 'truncate' in rtx_cost, at rtlanal.c:4169
>>>> 0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*,
>>>> int,
>>>> char const*)
>>>>          /home/eric/svn/gcc/gcc/rtl.c:829
>>>> 0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
>>>>          /home/eric/svn/gcc/gcc/rtlanal.c:4169
>>>> 0x8517e6 set_src_cost
>>>>          /home/eric/svn/gcc/gcc/rtl.h:2685
>>>> 0x8517e6 init_expmed_one_conv
>>>>          /home/eric/svn/gcc/gcc/expmed.c:142
>>>> 0x8517e6 init_expmed_one_mode
>>>>          /home/eric/svn/gcc/gcc/expmed.c:209
>>>> 0x853fb2 init_expmed()
>>>>          /home/eric/svn/gcc/gcc/expmed.c:270
>>>> 0xc45974 backend_init_target
>>>>          /home/eric/svn/gcc/gcc/toplev.c:1665
>>>> 0xc45974 initialize_rtl()
>>>>
>>> Sorry for disturbing, I will revert this if can't fix today.
>>
>> It looks bogus and I couldn't find the motivating case for it, so
>> revert with attached patch.  Build on x86 and commit as obvious.
>>
>> Thanks,
>> bin
>> 2017-05-03  Bin Cheng  <bin.cheng@arm.com>
>>
>>      Revert
>>      2017-05-02  Bin Cheng  <bin.cheng@arm.com>
>>      * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>
>
> Looking at the code in the patch...
>
> +    case TRUNCATE:
> +      /* If we can tie these modes, make this cheap.  */
> +      if (MODES_TIEABLE_P (mode, GET_MODE (SUBREG_REG (x))))
>
> 'code' here is GET_CODE (x) and in this case it is TRUNCATE.
> SUBREG_REG asserts (in RTL checking mode) that its argument is a SUBREG, so
> passing it a TRUNCATE rtx would cause
> the checking failure Eric reported. I think you meant to use XEXP (x, 0)
> instead of SUBREG_REG (x) ?
Yes, I guess so.  Reverted since I couldn't find the original test.

Thanks,
bin
>
> Thanks,
> Kyrill
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-05-03 10:12         ` Bin.Cheng
@ 2017-05-03 14:38           ` Christophe Lyon
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2017-05-03 14:38 UTC (permalink / raw)
  To: Bin.Cheng; +Cc: Kyrill Tkachov, Eric Botcazou, gcc-patches List

Hi Bin,

On 3 May 2017 at 12:12, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, May 3, 2017 at 11:09 AM, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> Hi Bin,
>>
>>
>> On 03/05/17 11:02, Bin.Cheng wrote:
>>>
>>> On Wed, May 3, 2017 at 9:38 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>>
>>>> On Wed, May 3, 2017 at 7:17 AM, Eric Botcazou <ebotcazou@adacore.com>
>>>> wrote:
>>>>>>
>>>>>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>>>>>
>>>>>>        * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>>>>
>>>>> This breaks bootstrap with RTL checking:
>>>>>
>>>>> /home/eric/build/gcc/native/./gcc/xgcc
>>>>> -B/home/eric/build/gcc/native/./gcc/ -
>>>>> nostdinc -x c /dev/null -S -o /dev/null -fself-
>>>>> test=/home/eric/svn/gcc/gcc/testsuite/selftests
>>>>> cc1: internal compiler error: RTL check: expected code 'subreg', have
>>>>> 'truncate' in rtx_cost, at rtlanal.c:4169
>>>>> 0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*,
>>>>> int,
>>>>> char const*)
>>>>>          /home/eric/svn/gcc/gcc/rtl.c:829
>>>>> 0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
>>>>>          /home/eric/svn/gcc/gcc/rtlanal.c:4169
>>>>> 0x8517e6 set_src_cost
>>>>>          /home/eric/svn/gcc/gcc/rtl.h:2685
>>>>> 0x8517e6 init_expmed_one_conv
>>>>>          /home/eric/svn/gcc/gcc/expmed.c:142
>>>>> 0x8517e6 init_expmed_one_mode
>>>>>          /home/eric/svn/gcc/gcc/expmed.c:209
>>>>> 0x853fb2 init_expmed()
>>>>>          /home/eric/svn/gcc/gcc/expmed.c:270
>>>>> 0xc45974 backend_init_target
>>>>>          /home/eric/svn/gcc/gcc/toplev.c:1665
>>>>> 0xc45974 initialize_rtl()
>>>>>
>>>> Sorry for disturbing, I will revert this if can't fix today.
>>>
>>> It looks bogus and I couldn't find the motivating case for it, so
>>> revert with attached patch.  Build on x86 and commit as obvious.
>>>
>>> Thanks,
>>> bin
>>> 2017-05-03  Bin Cheng  <bin.cheng@arm.com>
>>>
>>>      Revert
>>>      2017-05-02  Bin Cheng  <bin.cheng@arm.com>
>>>      * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>
>>
>> Looking at the code in the patch...
>>
>> +    case TRUNCATE:
>> +      /* If we can tie these modes, make this cheap.  */
>> +      if (MODES_TIEABLE_P (mode, GET_MODE (SUBREG_REG (x))))
>>
>> 'code' here is GET_CODE (x) and in this case it is TRUNCATE.
>> SUBREG_REG asserts (in RTL checking mode) that its argument is a SUBREG, so
>> passing it a TRUNCATE rtx would cause
>> the checking failure Eric reported. I think you meant to use XEXP (x, 0)
>> instead of SUBREG_REG (x) ?
> Yes, I guess so.  Reverted since I couldn't find the original test.
>

If it helps, your patch also introduced regressions on some arm targets.
See: http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/247509/report-build-info.html

    gcc.c-torture/execute/pr53645-2.c   -O2  (test for excess errors)
    gcc.c-torture/execute/pr53645-2.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  (test for excess errors)
    gcc.c-torture/execute/pr53645-2.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  (test for excess errors)
    gcc.c-torture/execute/pr53645-2.c   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
    gcc.c-torture/execute/pr53645-2.c   -O3 -g  (test for excess errors)

Thanks,

Christophe

> Thanks,
> bin
>>
>> Thanks,
>> Kyrill
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-05-03 10:12       ` Kyrill Tkachov
  2017-05-03 10:12         ` Bin.Cheng
@ 2017-05-10 14:38         ` Bin.Cheng
  2017-05-11 15:02           ` Christophe Lyon
  1 sibling, 1 reply; 10+ messages in thread
From: Bin.Cheng @ 2017-05-10 14:38 UTC (permalink / raw)
  To: Kyrill Tkachov; +Cc: Eric Botcazou, gcc-patches List

[-- Attachment #1: Type: text/plain, Size: 2587 bytes --]

On Wed, May 3, 2017 at 11:09 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi Bin,
>
>
> On 03/05/17 11:02, Bin.Cheng wrote:
>>
>> On Wed, May 3, 2017 at 9:38 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>
>>> On Wed, May 3, 2017 at 7:17 AM, Eric Botcazou <ebotcazou@adacore.com>
>>> wrote:
>>>>>
>>>>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>>>>
>>>>>        * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>>>
>>>> This breaks bootstrap with RTL checking:
>>>>
>>>> /home/eric/build/gcc/native/./gcc/xgcc
>>>> -B/home/eric/build/gcc/native/./gcc/ -
>>>> nostdinc -x c /dev/null -S -o /dev/null -fself-
>>>> test=/home/eric/svn/gcc/gcc/testsuite/selftests
>>>> cc1: internal compiler error: RTL check: expected code 'subreg', have
>>>> 'truncate' in rtx_cost, at rtlanal.c:4169
>>>> 0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*,
>>>> int,
>>>> char const*)
>>>>          /home/eric/svn/gcc/gcc/rtl.c:829
>>>> 0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
>>>>          /home/eric/svn/gcc/gcc/rtlanal.c:4169
>>>> 0x8517e6 set_src_cost
>>>>          /home/eric/svn/gcc/gcc/rtl.h:2685
>>>> 0x8517e6 init_expmed_one_conv
>>>>          /home/eric/svn/gcc/gcc/expmed.c:142
>>>> 0x8517e6 init_expmed_one_mode
>>>>          /home/eric/svn/gcc/gcc/expmed.c:209
>>>> 0x853fb2 init_expmed()
>>>>          /home/eric/svn/gcc/gcc/expmed.c:270
>>>> 0xc45974 backend_init_target
>>>>          /home/eric/svn/gcc/gcc/toplev.c:1665
>>>> 0xc45974 initialize_rtl()
>>>>
>>> Sorry for disturbing, I will revert this if can't fix today.
>>
>> It looks bogus and I couldn't find the motivating case for it, so
>> revert with attached patch.  Build on x86 and commit as obvious.
>>
>> Thanks,
>> bin
>> 2017-05-03  Bin Cheng  <bin.cheng@arm.com>
>>
>>      Revert
>>      2017-05-02  Bin Cheng  <bin.cheng@arm.com>
>>      * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>
>
> Looking at the code in the patch...
>
> +    case TRUNCATE:
> +      /* If we can tie these modes, make this cheap.  */
> +      if (MODES_TIEABLE_P (mode, GET_MODE (SUBREG_REG (x))))
>
> 'code' here is GET_CODE (x) and in this case it is TRUNCATE.
> SUBREG_REG asserts (in RTL checking mode) that its argument is a SUBREG, so
> passing it a TRUNCATE rtx would cause
> the checking failure Eric reported. I think you meant to use XEXP (x, 0)
> instead of SUBREG_REG (x) ?
Turned out this is still necessary for a test case pr49781-1.c.   I
updated patch as suggested.  This is kind of an obvious update.

Thanks,
bin
>
> Thanks,
> Kyrill
>

[-- Attachment #2: tieable-truncate.txt --]
[-- Type: text/plain, Size: 489 bytes --]

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 321363f..d9f57c3 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4164,6 +4164,13 @@ rtx_cost (rtx x, machine_mode mode, enum rtx_code outer_code,
 	return COSTS_N_INSNS (2 + factor);
       break;
 
+    case TRUNCATE:
+      if (MODES_TIEABLE_P (mode, GET_MODE (XEXP (x, 0))))
+	{
+	  total = 0;
+	  break;
+	}
+      /* FALLTHRU */
     default:
       if (targetm.rtx_costs (x, mode, outer_code, opno, &total, speed))
 	return total;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost
  2017-05-10 14:38         ` Bin.Cheng
@ 2017-05-11 15:02           ` Christophe Lyon
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2017-05-11 15:02 UTC (permalink / raw)
  To: Bin.Cheng; +Cc: Kyrill Tkachov, Eric Botcazou, gcc-patches List

Hi Bin,


On 10 May 2017 at 16:31, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, May 3, 2017 at 11:09 AM, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> Hi Bin,
>>
>>
>> On 03/05/17 11:02, Bin.Cheng wrote:
>>>
>>> On Wed, May 3, 2017 at 9:38 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>>
>>>> On Wed, May 3, 2017 at 7:17 AM, Eric Botcazou <ebotcazou@adacore.com>
>>>> wrote:
>>>>>>
>>>>>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>>>>>
>>>>>>        * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>>>>
>>>>> This breaks bootstrap with RTL checking:
>>>>>
>>>>> /home/eric/build/gcc/native/./gcc/xgcc
>>>>> -B/home/eric/build/gcc/native/./gcc/ -
>>>>> nostdinc -x c /dev/null -S -o /dev/null -fself-
>>>>> test=/home/eric/svn/gcc/gcc/testsuite/selftests
>>>>> cc1: internal compiler error: RTL check: expected code 'subreg', have
>>>>> 'truncate' in rtx_cost, at rtlanal.c:4169
>>>>> 0xbae338 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*,
>>>>> int,
>>>>> char const*)
>>>>>          /home/eric/svn/gcc/gcc/rtl.c:829
>>>>> 0xbbc9b4 rtx_cost(rtx_def*, machine_mode, rtx_code, int, bool)
>>>>>          /home/eric/svn/gcc/gcc/rtlanal.c:4169
>>>>> 0x8517e6 set_src_cost
>>>>>          /home/eric/svn/gcc/gcc/rtl.h:2685
>>>>> 0x8517e6 init_expmed_one_conv
>>>>>          /home/eric/svn/gcc/gcc/expmed.c:142
>>>>> 0x8517e6 init_expmed_one_mode
>>>>>          /home/eric/svn/gcc/gcc/expmed.c:209
>>>>> 0x853fb2 init_expmed()
>>>>>          /home/eric/svn/gcc/gcc/expmed.c:270
>>>>> 0xc45974 backend_init_target
>>>>>          /home/eric/svn/gcc/gcc/toplev.c:1665
>>>>> 0xc45974 initialize_rtl()
>>>>>
>>>> Sorry for disturbing, I will revert this if can't fix today.
>>>
>>> It looks bogus and I couldn't find the motivating case for it, so
>>> revert with attached patch.  Build on x86 and commit as obvious.
>>>
>>> Thanks,
>>> bin
>>> 2017-05-03  Bin Cheng  <bin.cheng@arm.com>
>>>
>>>      Revert
>>>      2017-05-02  Bin Cheng  <bin.cheng@arm.com>
>>>      * rtlanal.c (rtx_cost): Handle TRUNCATE between tieable modes.
>>
>>
>> Looking at the code in the patch...
>>
>> +    case TRUNCATE:
>> +      /* If we can tie these modes, make this cheap.  */
>> +      if (MODES_TIEABLE_P (mode, GET_MODE (SUBREG_REG (x))))
>>
>> 'code' here is GET_CODE (x) and in this case it is TRUNCATE.
>> SUBREG_REG asserts (in RTL checking mode) that its argument is a SUBREG, so
>> passing it a TRUNCATE rtx would cause
>> the checking failure Eric reported. I think you meant to use XEXP (x, 0)
>> instead of SUBREG_REG (x) ?
> Turned out this is still necessary for a test case pr49781-1.c.   I
> updated patch as suggested.  This is kind of an obvious update.
>

With your commit r247881, I still see a few regressions in some arm*
configurations:
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/247881/report-build-info.html

Christophe

> Thanks,
> bin
>>
>> Thanks,
>> Kyrill
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-11 15:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 10:38 [PATCH GCC8][01/33]Handle TRUNCATE between tieable modes in rtx_cost Bin Cheng
2017-04-24 21:23 ` Jeff Law
2017-05-03  7:33 ` Eric Botcazou
2017-05-03  9:00   ` Bin.Cheng
2017-05-03 10:07     ` Bin.Cheng
2017-05-03 10:12       ` Kyrill Tkachov
2017-05-03 10:12         ` Bin.Cheng
2017-05-03 14:38           ` Christophe Lyon
2017-05-10 14:38         ` Bin.Cheng
2017-05-11 15:02           ` Christophe Lyon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).