From patchwork Wed Oct 14 12:23:29 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 54912 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f197.google.com (mail-wi0-f197.google.com [209.85.212.197]) by patches.linaro.org (Postfix) with ESMTPS id 88CFE23012 for ; Wed, 14 Oct 2015 12:23:54 +0000 (UTC) Received: by wicid10 with SMTP id id10sf25421511wic.2 for ; Wed, 14 Oct 2015 05:23:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:sender :delivered-to:message-id:date:from:user-agent:mime-version:to:cc :subject:content-type:x-original-sender :x-original-authentication-results; bh=/qcjrAAAUJ2MQBTYqQqRTUQ24arON3b/1EYeFXFkf4w=; b=jQvUTgQQrACjtpp5hAV+iuXMH+/zv4XPJ3XCOaAzMWzam9si+BXrs/RK/g4cCAGq5Y 5zGDgX1T8qEOsuV+55PIiZAs6UQ1/LzP4V8osj+rS3UmP374WcoyA0jj4m1V7OVQy7UM sBmO9zGuwjq/8IjGFyt/2M/D9VbNouSGI7dkCeghSCBb/QK2sHoSaYWsHmxVvWfy11bR cqh7j/UQEp9b0LK1hnNgsTCHjEUppK4snyrR2ixwkbMNzD1g/5Y4c41VTxA7K3SFu0gw STV1auUwRg09p8fxoH5lDGI5JZDT7pM8+fyovC4EZreFAG/jBafJJPpVatrh2+PBFBE+ RSfA== X-Gm-Message-State: ALoCoQm/ouhtEDtyr7mTw0kaPUAxazz0yy49SRHjsyPv8Zdm7OP7OsdhOhEaPMcpWgc7FE1RgeUC X-Received: by 10.180.105.98 with SMTP id gl2mr5483235wib.0.1444825433845; Wed, 14 Oct 2015 05:23:53 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.25.24.170 with SMTP id 42ls45978lfy.3.gmail; Wed, 14 Oct 2015 05:23:53 -0700 (PDT) X-Received: by 10.112.163.193 with SMTP id yk1mr1432107lbb.1.1444825433682; Wed, 14 Oct 2015 05:23:53 -0700 (PDT) Received: from mail-lb0-x22e.google.com (mail-lb0-x22e.google.com. [2a00:1450:4010:c04::22e]) by mx.google.com with ESMTPS id x5si5397462lbb.5.2015.10.14.05.23.53 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Oct 2015 05:23:53 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c04::22e as permitted sender) client-ip=2a00:1450:4010:c04::22e; Received: by lbbck17 with SMTP id ck17so45030156lbb.1 for ; Wed, 14 Oct 2015 05:23:53 -0700 (PDT) X-Received: by 10.112.168.228 with SMTP id zz4mr1457137lbb.73.1444825433498; Wed, 14 Oct 2015 05:23:53 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.59.35 with SMTP id w3csp2700028lbq; Wed, 14 Oct 2015 05:23:52 -0700 (PDT) X-Received: by 10.68.204.37 with SMTP id kv5mr3487073pbc.64.1444825432187; Wed, 14 Oct 2015 05:23:52 -0700 (PDT) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id di2si12903411pbd.158.2015.10.14.05.23.51 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Oct 2015 05:23:52 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-410129-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 109801 invoked by alias); 14 Oct 2015 12:23:39 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list 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 109792 invoked by uid 89); 14 Oct 2015 12:23:38 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Oct 2015 12:23:36 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-9-a_9lvJshT0iuMrBZgKV1qA-1; Wed, 14 Oct 2015 13:23:30 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 14 Oct 2015 13:23:30 +0100 Message-ID: <561E4941.6030204@arm.com> Date: Wed, 14 Oct 2015 13:23:29 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: GCC Patches CC: Ramana Radhakrishnan , Richard Earnshaw Subject: [PATCH][ARM] PR target/67929 Tighten vfp3_const_double_for_bits checks X-MC-Unique: a_9lvJshT0iuMrBZgKV1qA-1 X-IsSubscribed: yes X-Original-Sender: kyrylo.tkachov@arm.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c04::22e as permitted sender) smtp.mailfrom=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@gcc.gnu.org X-Google-Group-Id: 836684582541 Hi all, This patch fixes the referenced PR by rewriting the vfp3_const_double_for_bits function in arm.c The function is supposed to accept positive CONST_DOUBLE rtxes whose value is an exact power of 2 and whose log2 is between 1 and 32. That is values like 2.0, 4.0, 8.9, 16.0 etc... The current implementation seems to have been written under the assumption that exact_real_truncate returns false if the input value is not an exact integer, whereas in fact exact_real_truncate returns false if the truncation operation was not exact, which are different things. This would lead the function to accept any CONST_DOUBLE that can truncate to a power of 2, such as 4.9, 16.2 etc. In any case, I've rewritten this function and used the real_isinteger predicate to check if the real value is an exact integer. The testcase demonstrates the kind of wrong code that this patch addresses. This bug appears on GCC 5 and 4.9 as well, but due to the recent introduction of CONST_DOUBLE_REAL_VALUE this patch doesn't apply on those branches. I will soon post the backportable variant. Bootstrapped and tested on arm-none-linux-gnueabihf. Ok for trunk? Thanks, Kyrill 2015-10-12 Kyrylo Tkachov PR target/67929 * config/arm/arm.c (vfp3_const_double_for_bits): Rewrite. * config/arm/constraints.md (Dp): Update callsite. * config/arm/predicates.md (const_double_vcvt_power_of_two): Likewise. 2015-10-12 Kyrylo Tkachov PR target/67929 * gcc.target/arm/pr67929_1.c: New test. diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 0bf1164..29dd489 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -27734,25 +27734,37 @@ vfp3_const_double_for_fract_bits (rtx operand) return 0; } +/* If X is a CONST_DOUBLE with a value that is a power of 2 whose + log2 is in [1, 32], return that log2. Otherwise return -1. + This is used in the patterns for vcvt.s32.f32 floating-point to + fixed-point conversions. */ + int -vfp3_const_double_for_bits (rtx operand) +vfp3_const_double_for_bits (rtx x) { - const REAL_VALUE_TYPE *r0; + const REAL_VALUE_TYPE *r; - if (!CONST_DOUBLE_P (operand)) - return 0; + if (!CONST_DOUBLE_P (x)) + return -1; - r0 = CONST_DOUBLE_REAL_VALUE (operand); - if (exact_real_truncate (DFmode, r0)) - { - HOST_WIDE_INT value = real_to_integer (r0); - value = value & 0xffffffff; - if ((value != 0) && ( (value & (value - 1)) == 0)) - return int_log2 (value); - } + r = CONST_DOUBLE_REAL_VALUE (x); - return 0; + if (REAL_VALUE_NEGATIVE (*r) + || REAL_VALUE_ISNAN (*r) + || REAL_VALUE_ISINF (*r) + || !real_isinteger (r, SFmode)) + return -1; + + HOST_WIDE_INT hwint = exact_log2 (real_to_integer (r)); + +/* The exact_log2 above will have returned -1 if this is + not an exact log2. */ + if (!IN_RANGE (hwint, 1, 32)) + return -1; + + return hwint; } + /* Emit a memory barrier around an atomic sequence according to MODEL. */ diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md index e24858f..901cfe5 100644 --- a/gcc/config/arm/constraints.md +++ b/gcc/config/arm/constraints.md @@ -339,7 +339,8 @@ "@internal In ARM/ Thumb2 a const_double which can be used with a vcvt.s32.f32 with bits operation" (and (match_code "const_double") - (match_test "TARGET_32BIT && TARGET_VFP && vfp3_const_double_for_bits (op)"))) + (match_test "TARGET_32BIT && TARGET_VFP + && vfp3_const_double_for_bits (op) > 0"))) (define_register_constraint "Ts" "(arm_restrict_it) ? LO_REGS : GENERAL_REGS" "For arm_restrict_it the core registers @code{r0}-@code{r7}. GENERAL_REGS otherwise.") diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md index 08cc899..48e4ba8 100644 --- a/gcc/config/arm/predicates.md +++ b/gcc/config/arm/predicates.md @@ -668,7 +668,7 @@ (define_predicate "const_double_vcvt_power_of_two" (and (match_code "const_double") (match_test "TARGET_32BIT && TARGET_VFP - && vfp3_const_double_for_bits (op)"))) + && vfp3_const_double_for_bits (op) > 0"))) (define_predicate "neon_struct_operand" (and (match_code "mem") diff --git a/gcc/testsuite/gcc.target/arm/pr67929_1.c b/gcc/testsuite/gcc.target/arm/pr67929_1.c new file mode 100644 index 0000000..14943b6 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr67929_1.c @@ -0,0 +1,21 @@ +/* { dg-do run } */ +/* { dg-require-effective-target arm_vfp3_ok } */ +/* { dg-options "-O2 -fno-inline" } */ +/* { dg-add-options arm_vfp3 } */ +/* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ + +int +foo (float a) +{ + return a * 4.9f; +} + + +int +main (void) +{ + if (foo (10.0f) != 49) + __builtin_abort (); + + return 0; +} \ No newline at end of file