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