From patchwork Mon Nov 17 15:38:36 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Lawrence X-Patchwork-Id: 411708 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B725514011D for ; Tue, 18 Nov 2014 02:38:54 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=MBfY0Q5xUGEi4YZGMy+xHbf64uM6KcWVhswOTObFsYLC9X dCMGBgp+5W/jiGaloHoBTxdxfvy7qT/eHbXEEH1QvcuQ8JlRVyzczIhEfP/xHZut 4Ts5CyzXJL+MA0GKVjNVtPSzBerO4K0k7yv68W6YIUBe0Ws0XFJ/U7+pkrWfk= 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 :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=vR0GrRyu1+ex+//ZW6zKutHXP9o=; b=Igbr2rVerd6GRUE0YBG8 8HPNCKUcdiGouuu0VZOPKlIGx8PnRwWmnw+eDqG0OnqLshXm2ZJ8HTRNV1BS7aD4 sTLFJ02g7TG+y8bu4eQm7xYajyq63qUzFcf1dHXdTpwiLnidIqzza3dfPh1fYCOx I9wukbHrF53nI0VcoHP9HjM= Received: (qmail 31050 invoked by alias); 17 Nov 2014 15:38:46 -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 31034 invoked by uid 89); 17 Nov 2014 15:38:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Nov 2014 15:38:40 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Mon, 17 Nov 2014 15:38:37 +0000 Received: from [10.1.209.51] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 17 Nov 2014 15:38:37 +0000 Message-ID: <546A167C.5090902@arm.com> Date: Mon, 17 Nov 2014 15:38:36 +0000 From: Alan Lawrence User-Agent: Thunderbird 2.0.0.24 (X11/20101213) MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" Subject: [PATCH][AArch64] Fix __builtin_aarch64_absdi, must not fold to ABS_EXPR X-MC-Unique: 114111715383715201 X-IsSubscribed: yes ...as the former is defined as returning MIN_VALUE for argument MIN_VALUE, whereas the latter is 'undefined', and gcc can optimize "abs(x)>=0" to "true", which is wrong for __builtin_aarch64_abs. There has been much debate here, although not recently - I think the last was https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00387.html . However without a definite solution, we should at least stop the folding, as otherwise this is a bug. The complication here is that the folding meant we never expanded the call to __builtin_aarch64_absdi, and thus never realized that expanding would cause an ICE: gen_absdi2() takes only two parameters (source and dest operand), and has no parameter corresponding to the scratch operand in the insn_data; but the code in aarch64_simd_expand_args, reads the number of arguments from the insn_data, and so tries to get another operand from the call to __builtin_aarch64_absdi, causing the ICE. Hence, we have to introduce a SIMD_ARG_SCRATCH, used in aarch64_simd_expand_args to skip over the argument. (There is an alternative way of doing this, i.e. to update the for-loop in aarch64_simd_expand_builtin by adding yet another version of 'k'. However, I thought there were enough versions already that I really didn't want to add one more...) To go with this, I've renamed qualifier_internal to qualifier_scratch, as it's not clear that any non-scratch internal operands (whatever such might be) would want the same treatment! Cross-tested check-gcc on aarch64-none-elf. Ok for trunk? gcc/ChangeLog: * config/aarch64/aarch64-builtins.c (enum aarch64_type_qualifiers): Rename qualifier_internal to qualifier_scratch. (aarch64_types_unop_qualifiers, aarch64_init_simd_builtins): Follow renaming. (builtin_simd_arg): New SIMD_ARG_SCRATCH enum value. (aarch64_simd_expand_args): Skip over SIMD_ARG_SCRATCHes. (aarch64_simd_expand_builtin): Handle qualifier_scratch. (aarch64_fold_builtin): Remove folding of abs. diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c index 1af17900785685e4e005710d3bb1743d88a11c16..4cd3d654c0543fad1b88549683222c060c7978d9 100644 --- a/gcc/config/aarch64/aarch64-builtins.c +++ b/gcc/config/aarch64/aarch64-builtins.c @@ -117,9 +117,10 @@ enum aarch64_type_qualifiers qualifier_maybe_immediate = 0x10, /* 1 << 4 */ /* void foo (...). */ qualifier_void = 0x20, /* 1 << 5 */ - /* Some patterns may have internal operands, this qualifier is an - instruction to the initialisation code to skip this operand. */ - qualifier_internal = 0x40, /* 1 << 6 */ + /* Tells the initialisation code to skip this operand when generating + the type of the builtin function (and the expander to skip it when + expanding a call to said builtin function). */ + qualifier_scratch = 0x40, /* 1 << 6 */ /* Some builtins should use the T_*mode* encoded in a simd_builtin_datum rather than using the type of the operand. */ qualifier_map_mode = 0x80, /* 1 << 7 */ @@ -140,11 +141,11 @@ typedef struct enum aarch64_type_qualifiers *qualifiers; } aarch64_simd_builtin_datum; -/* The qualifier_internal allows generation of a unary builtin from - a pattern with a third pseudo-operand such as a match_scratch. */ +/* The qualifier_scratch allows generation of a unary builtin from + a pattern with a third pseudo-operand i.e. match_scratch. */ static enum aarch64_type_qualifiers aarch64_types_unop_qualifiers[SIMD_MAX_BUILTIN_ARGS] - = { qualifier_none, qualifier_none, qualifier_internal }; + = { qualifier_none, qualifier_none, qualifier_scratch }; #define TYPES_UNOP (aarch64_types_unop_qualifiers) static enum aarch64_type_qualifiers aarch64_types_unopu_qualifiers[SIMD_MAX_BUILTIN_ARGS] @@ -791,7 +792,7 @@ aarch64_init_simd_builtins (void) type_signature[arg_num] = 's'; /* Skip an internal operand for vget_{low, high}. */ - if (qualifiers & qualifier_internal) + if (qualifiers & qualifier_scratch) continue; /* Some builtins have different user-facing types @@ -899,6 +900,7 @@ typedef enum { SIMD_ARG_COPY_TO_REG, SIMD_ARG_CONSTANT, + SIMD_ARG_SCRATCH, SIMD_ARG_STOP } builtin_simd_arg; @@ -919,13 +921,13 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval, || !(*insn_data[icode].operand[0].predicate) (target, tmode))) target = gen_reg_rtx (tmode); - for (;;) + for (;; args++) { - builtin_simd_arg thisarg = args[argc]; + builtin_simd_arg thisarg = *args; if (thisarg == SIMD_ARG_STOP) break; - else + else if (thisarg != SIMD_ARG_SCRATCH) { arg[argc] = CALL_EXPR_ARG (exp, argc); op[argc] = expand_normal (arg[argc]); @@ -950,6 +952,7 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval, break; case SIMD_ARG_STOP: + case SIMD_ARG_SCRATCH: gcc_unreachable (); } @@ -1061,6 +1064,8 @@ aarch64_simd_expand_builtin (int fcode, tree exp, rtx target) (arg, insn_data[icode].operand[operands_k].mode)); args[k] = op_const_int_p ? SIMD_ARG_CONSTANT : SIMD_ARG_COPY_TO_REG; } + else if (d->qualifiers[qualifiers_k] & qualifier_scratch) + args[k] = SIMD_ARG_SCRATCH; else args[k] = SIMD_ARG_COPY_TO_REG; @@ -1317,9 +1322,6 @@ aarch64_fold_builtin (tree fndecl, int n_args ATTRIBUTE_UNUSED, tree *args, switch (fcode) { - BUILTIN_VALLDI (UNOP, abs, 2) - return fold_build1 (ABS_EXPR, type, args[0]); - break; BUILTIN_VALLDI (BINOP, cmge, 0) return fold_build2 (GE_EXPR, type, args[0], args[1]); break;