On August 14, 2015 4:31:23 PM GMT+02:00, Aditya K <hiradi...@msn.com> wrote:
>
>
>----------------------------------------
>> Date: Fri, 14 Aug 2015 11:47:05 +0200
>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>> From: richard.guent...@gmail.com
>> To: hiradi...@msn.com
>> CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s....@samsung.com;
>seb...@gmail.com
>>
>> On Fri, Aug 14, 2015 at 11:25 AM, Aditya K <hiradi...@msn.com> wrote:
>>>
>>>
>>> ----------------------------------------
>>>> Date: Fri, 14 Aug 2015 09:31:32 +0200
>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>> From: richard.guent...@gmail.com
>>>> To: hiradi...@msn.com
>>>> CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s....@samsung.com;
>seb...@gmail.com
>>>>
>>>> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K <hiradi...@msn.com>
>wrote:
>>>>>
>>>>>
>>>>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>>>> From: richard.guent...@gmail.com
>>>>>> To: tob...@grosser.es
>>>>>> CC: hiradi...@msn.com; gcc-patches@gcc.gnu.org;
>s....@samsung.com;
>>>>>> seb...@gmail.com
>>>>>>
>>>>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser
><tob...@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 <hiradi...@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
>>>
>
