RE: [PATCH] [graphite] Constrain only on INTEGER_TYPE

[Aditya K <hiraditya at msn dot com>]

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

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