From patchwork Thu Jul 6 12:36:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Preudhomme X-Patchwork-Id: 785155 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 3x3HLC17swz9s5L for ; Thu, 6 Jul 2017 22:36:46 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="uCAnjNAJ"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=akD0Y36Lkp7O+DWVb QzsQWzU629hcjQwL4QuJU8tOiImkKQzU3Jmg0F9Id/D29q9042OLfXLdLq0SuQAT q9ZNPZJRM48PUcPkMWz/Id7Zlxl3nuK9nW2QobImYOhlbufkZcnDBm2Lb7d0CXQt xVQAR01dsMtFsFMCjSf4Wa9Xew= 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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=jI5TYashYp2pkfEYr+jj92t o+Nw=; b=uCAnjNAJblHr2CVvm6NnwUombLAMZBCKbd2I+05eQ/ktXpoo9MQ9Bt5 4K5oXg5lsZMLbeOTFcE2DwTfRvY89RNgx/BvFWzptP+O5JWvStZbd3Z505zL/+DE wkCsOTefZro1QjiFMoyIr9mxUbJNIbEWOgDSE/eZYz80tjFpV5vA= Received: (qmail 54068 invoked by alias); 6 Jul 2017 12:36:38 -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 54018 invoked by uid 89); 6 Jul 2017 12:36:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=baking X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Jul 2017 12:36:34 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4C0E02B; Thu, 6 Jul 2017 05:36:33 -0700 (PDT) Received: from [10.2.206.52] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8521A3F557; Thu, 6 Jul 2017 05:36:32 -0700 (PDT) Subject: Re: [PATCH, GCC/ARM] Remove ARMv8-M code for D17-D31 To: "Richard Earnshaw (lists)" , Kyrill Tkachov , Ramana Radhakrishnan , "gcc-patches@gcc.gnu.org" References: From: Thomas Preudhomme Message-ID: <5da9957f-06bc-11dd-a63a-6bf341b84486@foss.arm.com> Date: Thu, 6 Jul 2017 13:36:31 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes Hi Richard, On 28/06/17 16:56, Richard Earnshaw (lists) wrote: >> > > This is silently baking in dangerous assumptions about GCC's internal > numbering of the registers. That's not a good idea from a long-term > portability perspective. > > At the very least you need to assert that all the interesting registers > are numbered in the range 0..63; but ideally the code should just handle > pretty much any assignment of internal register numbers. There is already such an assert in my patch. :-) > > Did you consider using sbitmaps rather than doing all the multi-word > stuff by steam? I did now, most of it is trivial but interaction with compute_not_to_clear_mask is now more verbose because it returns a bitfield and one assert got quite ugly and expensive. Please find an updated patch in attachment and judge by yourself. Best regards, Thomas diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 259597d8890ee84c5bd92b12b6f9f6521c8dcd2e..93e152b1f38d3675e4ada1de7a34c2c209d8db1f 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -3620,6 +3620,11 @@ arm_option_override (void) if (use_cmse && !arm_arch_cmse) error ("target CPU does not support ARMv8-M Security Extensions"); + /* We don't clear D16-D31 VFP registers for cmse_nonsecure_call functions + and ARMv8-M Baseline and Mainline do not allow such configuration. */ + if (use_cmse && LAST_VFP_REGNUM > LAST_LO_VFP_REGNUM) + error ("ARMv8-M Security Extensions incompatible with selected FPU"); + /* Disable scheduling fusion by default if it's not armv7 processor or doesn't prefer ldrd/strd. */ if (flag_schedule_fusion == 2 @@ -24996,42 +25001,41 @@ thumb1_expand_prologue (void) void cmse_nonsecure_entry_clear_before_return (void) { - uint64_t to_clear_mask[2]; + sbitmap to_clear_bitmap; uint32_t padding_bits_to_clear = 0; uint32_t * padding_bits_to_clear_ptr = &padding_bits_to_clear; int regno, maxregno = IP_REGNUM; tree result_type; rtx result_rtl; - to_clear_mask[0] = (1ULL << (NUM_ARG_REGS)) - 1; - to_clear_mask[0] |= (1ULL << IP_REGNUM); + to_clear_bitmap = sbitmap_alloc (maxregno + 1); + bitmap_clear (to_clear_bitmap); + bitmap_set_range (to_clear_bitmap, R0_REGNUM, NUM_ARG_REGS); + bitmap_set_bit (to_clear_bitmap, IP_REGNUM); /* If we are not dealing with -mfloat-abi=soft we will need to clear VFP registers. We also check that TARGET_HARD_FLOAT and !TARGET_THUMB1 hold to make sure the instructions used to clear them are present. */ if (TARGET_HARD_FLOAT && !TARGET_THUMB1) { - uint64_t float_mask = (1ULL << (D7_VFP_REGNUM + 1)) - 1; + int float_bits = D7_VFP_REGNUM - FIRST_VFP_REGNUM + 1; maxregno = LAST_VFP_REGNUM; + to_clear_bitmap = sbitmap_resize (to_clear_bitmap, maxregno, 0); - float_mask &= ~((1ULL << FIRST_VFP_REGNUM) - 1); - to_clear_mask[0] |= float_mask; - - float_mask = (1ULL << (maxregno - 63)) - 1; - to_clear_mask[1] = float_mask; + bitmap_set_range (to_clear_bitmap, FIRST_VFP_REGNUM, float_bits); /* Make sure we don't clear the two scratch registers used to clear the relevant FPSCR bits in output_return_instruction. */ emit_use (gen_rtx_REG (SImode, IP_REGNUM)); - to_clear_mask[0] &= ~(1ULL << IP_REGNUM); + bitmap_clear_bit (to_clear_bitmap, IP_REGNUM); emit_use (gen_rtx_REG (SImode, 4)); - to_clear_mask[0] &= ~(1ULL << 4); + bitmap_clear_bit (to_clear_bitmap, 4); } /* If the user has defined registers to be caller saved, these are no longer restored by the function before returning and must thus be cleared for security purposes. */ - for (regno = NUM_ARG_REGS; regno < LAST_VFP_REGNUM; regno++) + for (regno = NUM_ARG_REGS; regno <= maxregno; regno++) { /* We do not touch registers that can be used to pass arguments as per the AAPCS, since these should never be made callee-saved by user @@ -25041,29 +25045,50 @@ cmse_nonsecure_entry_clear_before_return (void) if (IN_RANGE (regno, IP_REGNUM, PC_REGNUM)) continue; if (call_used_regs[regno]) - to_clear_mask[regno / 64] |= (1ULL << (regno % 64)); + bitmap_set_bit (to_clear_bitmap, regno); } /* Make sure we do not clear the registers used to return the result in. */ result_type = TREE_TYPE (DECL_RESULT (current_function_decl)); if (!VOID_TYPE_P (result_type)) { + unsigned count; + uint64_t to_clear_return_mask; result_rtl = arm_function_value (result_type, current_function_decl, 0); /* No need to check that we return in registers, because we don't support returning on stack yet. */ - to_clear_mask[0] - &= ~compute_not_to_clear_mask (result_type, result_rtl, 0, - padding_bits_to_clear_ptr); + gcc_assert (REG_P (result_rtl)); + to_clear_return_mask + = compute_not_to_clear_mask (result_type, result_rtl, 0, + padding_bits_to_clear_ptr); + if (to_clear_return_mask) + { + gcc_assert (maxregno < sizeof (long long) * __CHAR_BIT__); + for (regno = R0_REGNUM; regno <= maxregno; regno++) + { + if (to_clear_return_mask & (1ULL << regno)) + bitmap_clear_bit (to_clear_bitmap, regno); + } + } } if (padding_bits_to_clear != 0) { rtx reg_rtx; + sbitmap to_clear_arg_regs_bitmap, and_bitmap; + /* Padding bits to clear is not 0 so we know we are dealing with returning a composite type, which only uses r0. Let's make sure that r1-r3 is cleared too, we will use r1 as a scratch register. */ - gcc_assert ((to_clear_mask[0] & 0xe) == 0xe); + to_clear_arg_regs_bitmap = sbitmap_alloc (maxregno + 1); + bitmap_clear (to_clear_arg_regs_bitmap); + and_bitmap = sbitmap_alloc (maxregno + 1); + bitmap_clear (and_bitmap); + bitmap_set_range (to_clear_arg_regs_bitmap, R0_REGNUM + 1, + NUM_ARG_REGS - 1); + bitmap_and (and_bitmap, to_clear_arg_regs_bitmap, to_clear_bitmap); + gcc_assert (bitmap_equal_p (and_bitmap, to_clear_arg_regs_bitmap)); reg_rtx = gen_rtx_REG (SImode, R1_REGNUM); @@ -25085,7 +25110,7 @@ cmse_nonsecure_entry_clear_before_return (void) for (regno = R0_REGNUM; regno <= maxregno; regno++) { - if (!(to_clear_mask[regno / 64] & (1ULL << (regno % 64)))) + if (!bitmap_bit_p (to_clear_bitmap, regno)) continue; if (IS_VFP_REGNUM (regno)) @@ -25094,7 +25119,7 @@ cmse_nonsecure_entry_clear_before_return (void) be cleared, use vmov. */ if (TARGET_VFP_DOUBLE && VFP_REGNO_OK_FOR_DOUBLE (regno) - && to_clear_mask[regno / 64] & (1ULL << ((regno % 64) + 1))) + && bitmap_bit_p (to_clear_bitmap, regno + 1)) { emit_move_insn (gen_rtx_REG (DFmode, regno), CONST1_RTX (DFmode));