From patchwork Thu May 16 06:42:26 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Glisse X-Patchwork-Id: 244228 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 7A7192C00A8 for ; Thu, 16 May 2013 16:42:40 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=lTXNz11HC3wJ1foxO659Y7rx7PliHCkqGLW9ENl1GRQiuUXOy17zj y4V/Njwnwczvc1jttPltHofFaqZe/8CuNbJy524iyblEzeTC26A1R2kXtMqz/MoF c0fIORjaKTNss4yl5Ley8LEPOi7WQl8vaRdQfqdYNcdzURA9cp0FRQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=uDDfRtGu5XzdgB/fZvaia0voN/g=; b=ez8UmXFL/Br4et9Dm4W0 mVw+Inv3Cxz1P0TlyKf5s1TJIaGFMBwaDLWD9BfpdLZdY7otXaw3hQrbvLqN/Y+k Yl05/+2ejzz4XX1r8MKDgPKaAo4SWhsQ/FF8pkxnaTQ5Z785rwqbOZcnU1+D80tz aDm7QW2o/fD41JC1j9138q8= Received: (qmail 18364 invoked by alias); 16 May 2013 06:42:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 18353 invoked by uid 89); 16 May 2013 06:42:30 -0000 X-Spam-SWARE-Status: No, score=-5.2 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.1 Received: from mail3-relais-sop.national.inria.fr (HELO mail3-relais-sop.national.inria.fr) (192.134.164.104) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 16 May 2013 06:42:29 +0000 Received: from stedding.saclay.inria.fr ([193.55.250.194]) by mail3-relais-sop.national.inria.fr with ESMTP/TLS/DHE-RSA-AES128-SHA; 16 May 2013 08:42:26 +0200 Received: from glisse (helo=localhost) by stedding.saclay.inria.fr with local-esmtp (Exim 4.80) (envelope-from ) id 1UcrtS-0000ro-GU for gcc-patches@gcc.gnu.org; Thu, 16 May 2013 08:42:26 +0200 Date: Thu, 16 May 2013 08:42:26 +0200 (CEST) From: Marc Glisse To: gcc-patches@gcc.gnu.org Subject: Break infinite folding loop Message-ID: User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-Virus-Found: No Hello, we can get into a cycle where: (x<0)|1 becomes (x<0)?-1:1 and (y?-1:1) becomes y|1 Contrary to what I posted in the PR, I am disabling the second transformation here. It can be done later (the x86 target partially does it in the vcond expansion), and the VEC_COND_EXPR allows us to perform further operations: (((x<0)|1)*5-1)/2 becomes (untested) (x<0)?-3:2 Also, this is a partial revert of the last patch, which sounds safer. I am leaving the vector tests in the disabled transformations in case we decide to re-enable them later. This isn't the end of the story, even for fold-const.c. I kept the transformation a?-1:0 -> a, but it does not work because: /* If we try to convert OP0 to our type, the call to fold will try to move the conversion inside a COND, which will recurse. In that case, the COND_EXPR is probably the best choice, so leave it alone. */ && type == TREE_TYPE (arg0)) and when a is a comparison, its type is a different type (even looking through main variant and canonical), only useless_type_conversion_p notices they are so similar, and I would still need a conversion, otherwise the front-end complains when I try to assign the result that it has a different type than the variable I want to assign it to (I expected the result of the comparison to be opaque, and thus no complaining, but apparently not). Also, we may want to make fold_binary_op_with_conditional_arg more strict on how much folding is necessary to consider the transformation worth it. For VEC_COND_EXPR where both branches are evaluated anyway, at least if we started from a comparison and not already a VEC_COND_EXPR, we could require that both branches fold. But it seems better to fix the ICE quickly and do the rest later. Passes bootstrap+testsuite on x86_64-linux-gnu. 2013-05-16 Marc Glisse PR middle-end/57286 gcc/ * fold-const.c (fold_ternary_loc) : Disable some transformations to avoid an infinite loop. gcc/testsuite/ * gcc.dg/pr57286.c: New testcase. * gcc.dg/vector-shift-2.c: Don't assume int has size 4. * g++.dg/ext/vector22.C: Comment out transformations not performed anymore. Index: testsuite/gcc.dg/pr57286.c =================================================================== --- testsuite/gcc.dg/pr57286.c (revision 0) +++ testsuite/gcc.dg/pr57286.c (revision 0) @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-O" } */ + +typedef int vec __attribute__ ((vector_size (4*sizeof(int)))); +void f (vec *x){ + *x = (*x < 0) | 1; +} Property changes on: testsuite/gcc.dg/pr57286.c ___________________________________________________________________ Added: svn:keywords + Author Date Id Revision URL Added: svn:eol-style + native Index: testsuite/gcc.dg/vector-shift-2.c =================================================================== --- testsuite/gcc.dg/vector-shift-2.c (revision 198950) +++ testsuite/gcc.dg/vector-shift-2.c (working copy) @@ -1,13 +1,13 @@ /* { dg-do compile } */ /* { dg-options "-O -fdump-tree-ccp1" } */ -typedef unsigned vec __attribute__ ((vector_size (16))); +typedef unsigned vec __attribute__ ((vector_size (4*sizeof(int)))); void f (vec *a) { vec s = { 5, 5, 5, 5 }; *a = *a << s; } /* { dg-final { scan-tree-dump "<< 5" "ccp1" } } */ /* { dg-final { cleanup-tree-dump "ccp1" } } */ Index: testsuite/g++.dg/ext/vector22.C =================================================================== --- testsuite/g++.dg/ext/vector22.C (revision 198950) +++ testsuite/g++.dg/ext/vector22.C (working copy) @@ -1,20 +1,22 @@ /* { dg-do compile } */ /* { dg-options "-O -fdump-tree-gimple" } */ typedef unsigned vec __attribute__((vector_size(4*sizeof(int)))); +/* Disabled after PR57286 void f(vec*a,vec*b){ *a=(*a)?-1:(*b<10); *b=(*b)?(*a<10):0; } +*/ void g(vec*a,vec*b){ *a=(*a)?(*a<*a):-1; - *b=(*b)?-1:(*b<*b); +// *b=(*b)?-1:(*b<*b); } void h(vec*a){ *a=(~*a==5); } /* { dg-final { scan-tree-dump-not "~" "gimple" } } */ /* { dg-final { scan-tree-dump-not "VEC_COND_EXPR" "gimple" } } */ /* { dg-final { cleanup-tree-dump "gimple" } } */ Index: fold-const.c =================================================================== --- fold-const.c (revision 198950) +++ fold-const.c (working copy) @@ -14204,20 +14204,26 @@ fold_ternary_loc (location_t loc, enum t && TREE_CODE (arg0) == NE_EXPR && integer_zerop (TREE_OPERAND (arg0, 1)) && integer_pow2p (arg1) && TREE_CODE (TREE_OPERAND (arg0, 0)) == BIT_AND_EXPR && operand_equal_p (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1), arg1, OEP_ONLY_CONST)) return pedantic_non_lvalue_loc (loc, fold_convert_loc (loc, type, TREE_OPERAND (arg0, 0))); + /* Disable the transformations below for vectors, since + fold_binary_op_with_conditional_arg may undo them immediately, + yielding an infinite loop. */ + if (code == VEC_COND_EXPR) + return NULL_TREE; + /* Convert A ? B : 0 into A && B if A and B are truth values. */ if (integer_zerop (op2) && truth_value_p (TREE_CODE (arg0)) && truth_value_p (TREE_CODE (arg1)) && (code == VEC_COND_EXPR || !VECTOR_TYPE_P (type))) return fold_build2_loc (loc, code == VEC_COND_EXPR ? BIT_AND_EXPR : TRUTH_ANDIF_EXPR, type, fold_convert_loc (loc, type, arg0), arg1); /* Convert A ? B : 1 into !A || B if A and B are truth values. */