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

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

Reply via email to