diff mbox

[graphite] Constrain only on INTEGER_TYPE

Message ID BLU179-W8096E03B4CD31B588CDE68B67C0@phx.gbl
State New
Headers show

Commit Message

Aditya K Aug. 14, 2015, 9:25 a.m. UTC
----------------------------------------
> Date: Fri, 14 Aug 2015 09:31:32 +0200
> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> From: richard.guenther@gmail.com
> To: hiraditya@msn.com
> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com; sebpop@gmail.com
>
> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K <hiraditya@msn.com> wrote:
>>
>>
>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>> From: richard.guenther@gmail.com
>>> To: tobias@grosser.es
>>> CC: hiraditya@msn.com; gcc-patches@gcc.gnu.org; s.pop@samsung.com;
>>> sebpop@gmail.com
>>>
>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser <tobias@grosser.es>
>>> wrote:
>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote:
>>>>>
>>>>> Passes bootstrap, no regressions.
>>>>>
>>>>> With this patch gcc bootstraps with graphite.
>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>>>>> -floop-block"
>>>>
>>>>
>>>> LGTM, but please use a longer sentence to explain what you do.
>>>
>>> As the middle-end generally freely exchanges INTEGER_TYPE
>>> ENUMERAL_TYPE and BOOLEAN_TYPE
>>> you want to use INTEGRAL_TYPE_P here.
>>>
>>
>> Thanks.
>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little bit of
>> debugging I figured out that it is
>> ENUMERAL_TYPE that causes the failure (miscompile) in tree-vect-data-refs.c.
>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with these). But
>> that would be inconsistent with the type-checks at other places in
>> graphite-*.c.
>> Currently, we are only checking for INTEGER_TYPE at other places.
>
> This is weird - the code you replace did allow both ENUMERAL_TYPE and
> BOOLEAN_TYPE.
>
> my suggestion was
>
> /* We can not handle REAL_TYPE. Failed for pr39260. */
> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op))
>
> you also need to adjust the comment btw.

This is exactly what I did and that resulted in miscompile. Then out of curiosity I tried to debug by putting
 /* We can not handle REAL_TYPE. Failed for pr39260. */
 - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
 + || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
 + || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE))

And that passed bootstrap.

Update patch:

Passes bootstrap, no regressions.

With this patch gcc bootstraps with graphite.
make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange -floop-block"

gcc/ChangeLog:

2015-08-12  Aditya Kumar  <hiraditya@msn.com>

        * graphite-scop-detection.c (stmt_simple_for_scop_p):
	Constrain only on INTEGER_TYPE
---
 gcc/graphite-scop-detection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.1.4



>
> Richard.
>
>> -Aditya
>>
>>
>>> RIchard.
>>>
>>>> Tobias

Comments

Richard Biener Aug. 14, 2015, 9:47 a.m. UTC | #1
On Fri, Aug 14, 2015 at 11:25 AM, Aditya K <hiraditya@msn.com> wrote:
>
>
> ----------------------------------------
>> Date: Fri, 14 Aug 2015 09:31:32 +0200
>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>> From: richard.guenther@gmail.com
>> To: hiraditya@msn.com
>> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com; sebpop@gmail.com
>>
>> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K <hiraditya@msn.com> wrote:
>>>
>>>
>>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>> From: richard.guenther@gmail.com
>>>> To: tobias@grosser.es
>>>> CC: hiraditya@msn.com; gcc-patches@gcc.gnu.org; s.pop@samsung.com;
>>>> sebpop@gmail.com
>>>>
>>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser <tobias@grosser.es>
>>>> wrote:
>>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote:
>>>>>>
>>>>>> Passes bootstrap, no regressions.
>>>>>>
>>>>>> With this patch gcc bootstraps with graphite.
>>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>>>>>> -floop-block"
>>>>>
>>>>>
>>>>> LGTM, but please use a longer sentence to explain what you do.
>>>>
>>>> As the middle-end generally freely exchanges INTEGER_TYPE
>>>> ENUMERAL_TYPE and BOOLEAN_TYPE
>>>> you want to use INTEGRAL_TYPE_P here.
>>>>
>>>
>>> Thanks.
>>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little bit of
>>> debugging I figured out that it is
>>> ENUMERAL_TYPE that causes the failure (miscompile) in tree-vect-data-refs.c.
>>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with these). But
>>> that would be inconsistent with the type-checks at other places in
>>> graphite-*.c.
>>> Currently, we are only checking for INTEGER_TYPE at other places.
>>
>> This is weird - the code you replace did allow both ENUMERAL_TYPE and
>> BOOLEAN_TYPE.
>>
>> my suggestion was
>>
>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op))
>>
>> you also need to adjust the comment btw.
>
> This is exactly what I did and that resulted in miscompile. Then out of curiosity I tried to debug by putting
>  /* We can not handle REAL_TYPE. Failed for pr39260. */
>  - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>  + || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
>  + || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE))
>

well, each op is either != INTEGER or != BOOLEAN ... this is equal to
doing || true

> And that passed bootstrap.
>
> Update patch:
>
> Passes bootstrap, no regressions.
>
> With this patch gcc bootstraps with graphite.
> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange -floop-block"
>
> gcc/ChangeLog:
>
> 2015-08-12  Aditya Kumar  <hiraditya@msn.com>
>
>         * graphite-scop-detection.c (stmt_simple_for_scop_p):
>         Constrain only on INTEGER_TYPE
> ---
>  gcc/graphite-scop-detection.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c
> index fb7247e..0f02a71 100644
> --- a/gcc/graphite-scop-detection.c
> +++ b/gcc/graphite-scop-detection.c
> @@ -409,8 +409,8 @@ stmt_simple_for_scop_p (basic_block scop_entry, loop_p outermost_loop,
>           {
>             tree op = gimple_op (stmt, i);
>             if (!graphite_can_represent_expr (scop_entry, loop, op)
> -               /* We can not handle REAL_TYPE. Failed for pr39260.  */
> -               || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
> +               /* We can only constrain on integer type.  */
> +               || (TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE))
>               {
>                 if (dump_file && (dump_flags & TDF_DETAILS))
>                   {
> --
> 2.1.4
>
>
>
>>
>> Richard.
>>
>>> -Aditya
>>>
>>>
>>>> RIchard.
>>>>
>>>>> Tobias
>
Aditya K Aug. 14, 2015, 2:31 p.m. UTC | #2
----------------------------------------
> Date: Fri, 14 Aug 2015 11:47:05 +0200
> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> From: richard.guenther@gmail.com
> To: hiraditya@msn.com
> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com; sebpop@gmail.com
>
> On Fri, Aug 14, 2015 at 11:25 AM, Aditya K <hiraditya@msn.com> wrote:
>>
>>
>> ----------------------------------------
>>> Date: Fri, 14 Aug 2015 09:31:32 +0200
>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>> From: richard.guenther@gmail.com
>>> To: hiraditya@msn.com
>>> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com; sebpop@gmail.com
>>>
>>> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K <hiraditya@msn.com> wrote:
>>>>
>>>>
>>>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>>> From: richard.guenther@gmail.com
>>>>> To: tobias@grosser.es
>>>>> CC: hiraditya@msn.com; gcc-patches@gcc.gnu.org; s.pop@samsung.com;
>>>>> sebpop@gmail.com
>>>>>
>>>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser <tobias@grosser.es>
>>>>> wrote:
>>>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote:
>>>>>>>
>>>>>>> Passes bootstrap, no regressions.
>>>>>>>
>>>>>>> With this patch gcc bootstraps with graphite.
>>>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>>>>>>> -floop-block"
>>>>>>
>>>>>>
>>>>>> LGTM, but please use a longer sentence to explain what you do.
>>>>>
>>>>> As the middle-end generally freely exchanges INTEGER_TYPE
>>>>> ENUMERAL_TYPE and BOOLEAN_TYPE
>>>>> you want to use INTEGRAL_TYPE_P here.
>>>>>
>>>>
>>>> Thanks.
>>>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little bit of
>>>> debugging I figured out that it is
>>>> ENUMERAL_TYPE that causes the failure (miscompile) in tree-vect-data-refs.c.
>>>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with these). But
>>>> that would be inconsistent with the type-checks at other places in
>>>> graphite-*.c.
>>>> Currently, we are only checking for INTEGER_TYPE at other places.
>>>
>>> This is weird - the code you replace did allow both ENUMERAL_TYPE and
>>> BOOLEAN_TYPE.
>>>
>>> my suggestion was
>>>
>>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>>> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op))
>>>
>>> you also need to adjust the comment btw.
>>
>> This is exactly what I did and that resulted in miscompile. Then out of curiosity I tried to debug by putting
>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>> + || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
>> + || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE))
>>
>
> well, each op is either != INTEGER or != BOOLEAN ... this is equal to
> doing || true


My bad, thanks for correction.
I tried this way (using BOOLEAN_TYPE) and this also failed bootstrap.

@@ -410,7 +410,8 @@ stmt_simple_for_scop_p (basic_block scop_entry, loop_p outermost_loop,
            tree op = gimple_op (stmt, i);
            if (!graphite_can_represent_expr (scop_entry, loop, op)
                /* We can not handle REAL_TYPE. Failed for pr39260.  */
-               || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
+               || ((TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
+               && (TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE)))
              {
                if (dump_file && (dump_flags & TDF_DETAILS))


While, using ENUMERAL_TYPE passed bootstrap.
@@ -410,7 +410,8 @@ stmt_simple_for_scop_p (basic_block scop_entry, loop_p outermost_loop,
            tree op = gimple_op (stmt, i);
            if (!graphite_can_represent_expr (scop_entry, loop, op)
                /* We can not handle REAL_TYPE. Failed for pr39260.  */
-               || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
+               || ((TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
+               && (TREE_CODE (TREE_TYPE (op)) != ENUMERAL_TYPE)))
              {
                if (dump_file && (dump_flags & TDF_DETAILS))

Where,
BOOT_CFLAGS="-g -O2 -fgraphite-identity  -floop-block -floop-interchange"

-Aditya










>
>> And that passed bootstrap.
>>
>> Update patch:
>>
>> Passes bootstrap, no regressions.
>>
>> With this patch gcc bootstraps with graphite.
>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange -floop-block"
>>
>> gcc/ChangeLog:
>>
>> 2015-08-12 Aditya Kumar <hiraditya@msn.com>
>>
>> * graphite-scop-detection.c (stmt_simple_for_scop_p):
>> Constrain only on INTEGER_TYPE
>> ---
>> gcc/graphite-scop-detection.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c
>> index fb7247e..0f02a71 100644
>> --- a/gcc/graphite-scop-detection.c
>> +++ b/gcc/graphite-scop-detection.c
>> @@ -409,8 +409,8 @@ stmt_simple_for_scop_p (basic_block scop_entry, loop_p outermost_loop,
>> {
>> tree op = gimple_op (stmt, i);
>> if (!graphite_can_represent_expr (scop_entry, loop, op)
>> - /* We can not handle REAL_TYPE. Failed for pr39260. */
>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>> + /* We can only constrain on integer type. */
>> + || (TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE))
>> {
>> if (dump_file && (dump_flags & TDF_DETAILS))
>> {
>> --
>> 2.1.4
>>
>>
>>
>>>
>>> Richard.
>>>
>>>> -Aditya
>>>>
>>>>
>>>>> RIchard.
>>>>>
>>>>>> Tobias
>>
Richard Biener Aug. 14, 2015, 2:47 p.m. UTC | #3
On August 14, 2015 4:31:23 PM GMT+02:00, Aditya K <hiraditya@msn.com> wrote:
>
>
>----------------------------------------
>> Date: Fri, 14 Aug 2015 11:47:05 +0200
>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>> From: richard.guenther@gmail.com
>> To: hiraditya@msn.com
>> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com;
>sebpop@gmail.com
>>
>> On Fri, Aug 14, 2015 at 11:25 AM, Aditya K <hiraditya@msn.com> wrote:
>>>
>>>
>>> ----------------------------------------
>>>> Date: Fri, 14 Aug 2015 09:31:32 +0200
>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>> From: richard.guenther@gmail.com
>>>> To: hiraditya@msn.com
>>>> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com;
>sebpop@gmail.com
>>>>
>>>> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K <hiraditya@msn.com>
>wrote:
>>>>>
>>>>>
>>>>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>>>> From: richard.guenther@gmail.com
>>>>>> To: tobias@grosser.es
>>>>>> CC: hiraditya@msn.com; gcc-patches@gcc.gnu.org;
>s.pop@samsung.com;
>>>>>> sebpop@gmail.com
>>>>>>
>>>>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser
><tobias@grosser.es>
>>>>>> wrote:
>>>>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote:
>>>>>>>>
>>>>>>>> Passes bootstrap, no regressions.
>>>>>>>>
>>>>>>>> With this patch gcc bootstraps with graphite.
>>>>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>>>>>>>> -floop-block"
>>>>>>>
>>>>>>>
>>>>>>> LGTM, but please use a longer sentence to explain what you do.
>>>>>>
>>>>>> As the middle-end generally freely exchanges INTEGER_TYPE
>>>>>> ENUMERAL_TYPE and BOOLEAN_TYPE
>>>>>> you want to use INTEGRAL_TYPE_P here.
>>>>>>
>>>>>
>>>>> Thanks.
>>>>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little
>bit of
>>>>> debugging I figured out that it is
>>>>> ENUMERAL_TYPE that causes the failure (miscompile) in
>tree-vect-data-refs.c.
>>>>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with
>these). But
>>>>> that would be inconsistent with the type-checks at other places in
>>>>> graphite-*.c.
>>>>> Currently, we are only checking for INTEGER_TYPE at other places.
>>>>
>>>> This is weird - the code you replace did allow both ENUMERAL_TYPE
>and
>>>> BOOLEAN_TYPE.
>>>>
>>>> my suggestion was
>>>>
>>>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>>>> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op))
>>>>
>>>> you also need to adjust the comment btw.
>>>
>>> This is exactly what I did and that resulted in miscompile. Then out
>of curiosity I tried to debug by putting
>>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>>> + || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
>>> + || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE))
>>>
>>
>> well, each op is either != INTEGER or != BOOLEAN ... this is equal to
>> doing || true
>
>
>My bad, thanks for correction.
>I tried this way (using BOOLEAN_TYPE) and this also failed bootstrap.

As previously said the REAL_TYPE condition also allowed BOOLEAN_TYPE and thus also should have failed to bootstrap then(?)

Richard.

>@@ -410,7 +410,8 @@ stmt_simple_for_scop_p (basic_block scop_entry,
>loop_p outermost_loop,
>            tree op = gimple_op (stmt, i);
>            if (!graphite_can_represent_expr (scop_entry, loop, op)
>                /* We can not handle REAL_TYPE. Failed for pr39260.  */
>-               || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>+               || ((TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
>+               && (TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE)))
>              {
>                if (dump_file && (dump_flags & TDF_DETAILS))
>
>
>While, using ENUMERAL_TYPE passed bootstrap.
>@@ -410,7 +410,8 @@ stmt_simple_for_scop_p (basic_block scop_entry,
>loop_p outermost_loop,
>            tree op = gimple_op (stmt, i);
>            if (!graphite_can_represent_expr (scop_entry, loop, op)
>                /* We can not handle REAL_TYPE. Failed for pr39260.  */
>-               || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>+               || ((TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
>+               && (TREE_CODE (TREE_TYPE (op)) != ENUMERAL_TYPE)))
>              {
>                if (dump_file && (dump_flags & TDF_DETAILS))
>
>Where,
>BOOT_CFLAGS="-g -O2 -fgraphite-identity  -floop-block
>-floop-interchange"
>
>-Aditya
>
>
>
>
>
>
>
>
>
>
>>
>>> And that passed bootstrap.
>>>
>>> Update patch:
>>>
>>> Passes bootstrap, no regressions.
>>>
>>> With this patch gcc bootstraps with graphite.
>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>-floop-block"
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-08-12 Aditya Kumar <hiraditya@msn.com>
>>>
>>> * graphite-scop-detection.c (stmt_simple_for_scop_p):
>>> Constrain only on INTEGER_TYPE
>>> ---
>>> gcc/graphite-scop-detection.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gcc/graphite-scop-detection.c
>b/gcc/graphite-scop-detection.c
>>> index fb7247e..0f02a71 100644
>>> --- a/gcc/graphite-scop-detection.c
>>> +++ b/gcc/graphite-scop-detection.c
>>> @@ -409,8 +409,8 @@ stmt_simple_for_scop_p (basic_block scop_entry,
>loop_p outermost_loop,
>>> {
>>> tree op = gimple_op (stmt, i);
>>> if (!graphite_can_represent_expr (scop_entry, loop, op)
>>> - /* We can not handle REAL_TYPE. Failed for pr39260. */
>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>>> + /* We can only constrain on integer type. */
>>> + || (TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE))
>>> {
>>> if (dump_file && (dump_flags & TDF_DETAILS))
>>> {
>>> --
>>> 2.1.4
>>>
>>>
>>>
>>>>
>>>> Richard.
>>>>
>>>>> -Aditya
>>>>>
>>>>>
>>>>>> RIchard.
>>>>>>
>>>>>>> Tobias
>>>
>
Aditya K Aug. 14, 2015, 3:09 p.m. UTC | #4
----------------------------------------
> Subject: RE: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> From: richard.guenther@gmail.com
> Date: Fri, 14 Aug 2015 16:47:41 +0200
> To: hiraditya@msn.com
> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com; sebpop@gmail.com
>
> On August 14, 2015 4:31:23 PM GMT+02:00, Aditya K <hiraditya@msn.com> wrote:
>>
>>
>>----------------------------------------
>>> Date: Fri, 14 Aug 2015 11:47:05 +0200
>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>> From: richard.guenther@gmail.com
>>> To: hiraditya@msn.com
>>> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com;
>>sebpop@gmail.com
>>>
>>> On Fri, Aug 14, 2015 at 11:25 AM, Aditya K <hiraditya@msn.com> wrote:
>>>>
>>>>
>>>> ----------------------------------------
>>>>> Date: Fri, 14 Aug 2015 09:31:32 +0200
>>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>>> From: richard.guenther@gmail.com
>>>>> To: hiraditya@msn.com
>>>>> CC: tobias@grosser.es; gcc-patches@gcc.gnu.org; s.pop@samsung.com;
>>sebpop@gmail.com
>>>>>
>>>>> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K <hiraditya@msn.com>
>>wrote:
>>>>>>
>>>>>>
>>>>>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>>>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>>>>> From: richard.guenther@gmail.com
>>>>>>> To: tobias@grosser.es
>>>>>>> CC: hiraditya@msn.com; gcc-patches@gcc.gnu.org;
>>s.pop@samsung.com;
>>>>>>> sebpop@gmail.com
>>>>>>>
>>>>>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser
>><tobias@grosser.es>
>>>>>>> wrote:
>>>>>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote:
>>>>>>>>>
>>>>>>>>> Passes bootstrap, no regressions.
>>>>>>>>>
>>>>>>>>> With this patch gcc bootstraps with graphite.
>>>>>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>>>>>>>>> -floop-block"
>>>>>>>>
>>>>>>>>
>>>>>>>> LGTM, but please use a longer sentence to explain what you do.
>>>>>>>
>>>>>>> As the middle-end generally freely exchanges INTEGER_TYPE
>>>>>>> ENUMERAL_TYPE and BOOLEAN_TYPE
>>>>>>> you want to use INTEGRAL_TYPE_P here.
>>>>>>>
>>>>>>
>>>>>> Thanks.
>>>>>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little
>>bit of
>>>>>> debugging I figured out that it is
>>>>>> ENUMERAL_TYPE that causes the failure (miscompile) in
>>tree-vect-data-refs.c.
>>>>>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with
>>these). But
>>>>>> that would be inconsistent with the type-checks at other places in
>>>>>> graphite-*.c.
>>>>>> Currently, we are only checking for INTEGER_TYPE at other places.
>>>>>
>>>>> This is weird - the code you replace did allow both ENUMERAL_TYPE
>>and
>>>>> BOOLEAN_TYPE.
>>>>>
>>>>> my suggestion was
>>>>>
>>>>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>>>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>>>>> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op))
>>>>>
>>>>> you also need to adjust the comment btw.
>>>>
>>>> This is exactly what I did and that resulted in miscompile. Then out
>>of curiosity I tried to debug by putting
>>>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>>>> + || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
>>>> + || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE))
>>>>
>>>
>>> well, each op is either != INTEGER or != BOOLEAN ... this is equal to
>>> doing || true
>>
>>
>>My bad, thanks for correction.
>>I tried this way (using BOOLEAN_TYPE) and this also failed bootstrap.
>
> As previously said the REAL_TYPE condition also allowed BOOLEAN_TYPE and thus also should have failed to bootstrap then(?)
>

Yet it did fail before. I started working on this patch because gcc was failing bootstrap with 
BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-block -floop-interchange"
-Aditya

> Richard.
>
>
diff mbox

Patch

diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c
index fb7247e..0f02a71 100644
--- a/gcc/graphite-scop-detection.c
+++ b/gcc/graphite-scop-detection.c
@@ -409,8 +409,8 @@  stmt_simple_for_scop_p (basic_block scop_entry, loop_p outermost_loop,
 	  {
 	    tree op = gimple_op (stmt, i);
 	    if (!graphite_can_represent_expr (scop_entry, loop, op)
-		/* We can not handle REAL_TYPE. Failed for pr39260.  */
-		|| TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
+		/* We can only constrain on integer type.  */
+		|| (TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE))
 	      {
 		if (dump_file && (dump_flags & TDF_DETAILS))
 		  {