From patchwork Fri May 20 21:23:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 624625 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 3rBLY42vG5z9t6N for ; Sat, 21 May 2016 07:24:19 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=aDxsU51X; 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=wBc+0qqeB98RTftUA 2mwl72AIPcLViu7BMR8F5K4wsRu1/1nsDUT679msoAl8AeQhsRJk7uoFjUyEPbd1 fUHG2iQAdNK10pDsZgaq+nQPE3K8Cwdx6ybaXWoToR1aT87h0PgPtV1g7tv6Beb8 AhwYq0t1snPQCftqEKFsKIHMhg= 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=3xGVBm8KLXntFvnNGC0KtAZ QEYo=; b=aDxsU51XdpiWSOvGzy5+Bd+PcgmRars8ztRfRQR6yx8iCq51g8ST9o4 p7bMJ0zGsw2H4PJXXnacFXlSjbetwhzW+WeKW/4WkzUckfnALgQa5z22Y0X+cfFg hokmZhQ80qfcexdtJZvcamwirB50hMbeBv4oDfhjChMnc/fbtqKQ= Received: (qmail 45367 invoked by alias); 20 May 2016 21:24:03 -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 45172 invoked by uid 89); 20 May 2016 21:24:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=burn X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 20 May 2016 21:23:51 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 668348112D; Fri, 20 May 2016 21:23:50 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-172.phx2.redhat.com [10.3.116.172]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4KLNn0p001922; Fri, 20 May 2016 17:23:50 -0400 Subject: [RFA] Minor cleanup to allocate_dynamic_stack_space To: vogt@linux.vnet.ibm.com, gcc-patches@gcc.gnu.org, Andreas Krebbel References: <20160429221242.GA2205@linux.vnet.ibm.com> <20160503141753.GA17351@linux.vnet.ibm.com> <34a785ea-bdd2-023c-4c02-a31dbadf0142@redhat.com> From: Jeff Law Message-ID: <5a4bf204-9919-7930-affe-f854f6ad9171@redhat.com> Date: Fri, 20 May 2016 15:23:49 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <34a785ea-bdd2-023c-4c02-a31dbadf0142@redhat.com> X-IsSubscribed: yes On 05/19/2016 05:11 PM, Jeff Law wrote: [ ... ] >This is a bit of a mess and I think the code > needs some TLC before we start hacking it up further. > > Let's start with clean up of dead code: > > /* We will need to ensure that the address we return is aligned to > REQUIRED_ALIGN. If STACK_DYNAMIC_OFFSET is defined, we don't > always know its final value at this point in the compilation (it > might depend on the size of the outgoing parameter lists, for > example), so we must align the value to be returned in that case. > (Note that STACK_DYNAMIC_OFFSET will have a default nonzero value if > STACK_POINTER_OFFSET or ACCUMULATE_OUTGOING_ARGS are defined). > We must also do an alignment operation on the returned value if > the stack pointer alignment is less strict than REQUIRED_ALIGN. > > If we have to align, we must leave space in SIZE for the hole > that might result from the alignment operation. */ > > must_align = (crtl->preferred_stack_boundary < required_align); > if (must_align) > { > if (required_align > PREFERRED_STACK_BOUNDARY) > extra_align = PREFERRED_STACK_BOUNDARY; > else if (required_align > STACK_BOUNDARY) > extra_align = STACK_BOUNDARY; > else > extra_align = BITS_PER_UNIT; > } > > /* ??? STACK_POINTER_OFFSET is always defined now. */ > #if defined (STACK_DYNAMIC_OFFSET) || defined (STACK_POINTER_OFFSET) > must_align = true; > extra_align = BITS_PER_UNIT; > #endif > > If we look at defaults.h, it always defines STACK_POINTER_OFFSET. So > all the code above I think collapses to: > > must_align = true; > extra_align = BITS_PER_UNIT > > And the only other assignment to must_align assigns it the value "true". > There are two conditionals on must_align that looks like > > if (must_align) > { > CODE; > } > > We should remove the conditional and pull CODE out an indentation level. > And remove all remnants of must_align. > > I don't think that changes your patch in any way. Hopefully it makes > the whole function somewhat easier to grok. > > Thoughts? So here's that cleanup. The diffs are larger than one might expect because of the reindentation that needs to happen. So I've included a -b diff variant which shows how little actually changed here. This should have no impact on any target. Bootstrapped and regression tested on x86_64 linux. Ok for the trunk? Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b8fb96c..257e98b 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2016-05-20 Jeff Law + + * explow.c (allocate_dynamic_stack_space): Simplify + knowing that MUST_ALIGN was always true. + 2016-05-16 Ryan Burn * Makefile.in (GTFILES): Add cilk.h and cilk-common.c. diff --git a/gcc/explow.c b/gcc/explow.c index e0ce201..51897e0 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -1175,7 +1175,6 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, rtx_code_label *final_label; rtx final_target, target; unsigned extra_align = 0; - bool must_align; /* If we're asking for zero bytes, it doesn't matter what we point to since we can't dereference it. But return a reasonable @@ -1245,38 +1244,8 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, if (crtl->preferred_stack_boundary < PREFERRED_STACK_BOUNDARY) crtl->preferred_stack_boundary = PREFERRED_STACK_BOUNDARY; - /* We will need to ensure that the address we return is aligned to - REQUIRED_ALIGN. If STACK_DYNAMIC_OFFSET is defined, we don't - always know its final value at this point in the compilation (it - might depend on the size of the outgoing parameter lists, for - example), so we must align the value to be returned in that case. - (Note that STACK_DYNAMIC_OFFSET will have a default nonzero value if - STACK_POINTER_OFFSET or ACCUMULATE_OUTGOING_ARGS are defined). - We must also do an alignment operation on the returned value if - the stack pointer alignment is less strict than REQUIRED_ALIGN. - - If we have to align, we must leave space in SIZE for the hole - that might result from the alignment operation. */ - - must_align = (crtl->preferred_stack_boundary < required_align); - if (must_align) - { - if (required_align > PREFERRED_STACK_BOUNDARY) - extra_align = PREFERRED_STACK_BOUNDARY; - else if (required_align > STACK_BOUNDARY) - extra_align = STACK_BOUNDARY; - else extra_align = BITS_PER_UNIT; - } - - /* ??? STACK_POINTER_OFFSET is always defined now. */ -#if defined (STACK_DYNAMIC_OFFSET) || defined (STACK_POINTER_OFFSET) - must_align = true; - extra_align = BITS_PER_UNIT; -#endif - if (must_align) - { unsigned extra = (required_align - extra_align) / BITS_PER_UNIT; size = plus_constant (Pmode, size, extra); @@ -1287,7 +1256,6 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, if (extra && size_align > extra_align) size_align = extra_align; - } /* Round the size to a multiple of the required stack alignment. Since the stack if presumed to be rounded before this allocation, @@ -1366,7 +1334,6 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, gen_int_mode (required_align / BITS_PER_UNIT - 1, Pmode), NULL_RTX, 1, OPTAB_LIB_WIDEN); - must_align = true; } func = init_one_libfunc ("__morestack_allocate_stack_space"); @@ -1478,8 +1445,6 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, target = final_target; } - if (must_align) - { /* CEIL_DIV_EXPR needs to worry about the addition overflowing, but we know it can't. So add ourselves and then do TRUNC_DIV_EXPR. */ @@ -1495,7 +1460,6 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, gen_int_mode (required_align / BITS_PER_UNIT, Pmode), NULL_RTX, 1); - } /* Now that we've committed to a return value, mark its alignment. */ mark_reg_pointer (target, required_align); diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b8fb96c..257e98b 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2016-05-20 Jeff Law + + * explow.c (allocate_dynamic_stack_space): Simplify + knowing that MUST_ALIGN was always true. + 2016-05-16 Ryan Burn * Makefile.in (GTFILES): Add cilk.h and cilk-common.c. diff --git a/gcc/explow.c b/gcc/explow.c index e0ce201..51897e0 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -1175,7 +1175,6 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, rtx_code_label *final_label; rtx final_target, target; unsigned extra_align = 0; - bool must_align; /* If we're asking for zero bytes, it doesn't matter what we point to since we can't dereference it. But return a reasonable @@ -1245,49 +1244,18 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, if (crtl->preferred_stack_boundary < PREFERRED_STACK_BOUNDARY) crtl->preferred_stack_boundary = PREFERRED_STACK_BOUNDARY; - /* We will need to ensure that the address we return is aligned to - REQUIRED_ALIGN. If STACK_DYNAMIC_OFFSET is defined, we don't - always know its final value at this point in the compilation (it - might depend on the size of the outgoing parameter lists, for - example), so we must align the value to be returned in that case. - (Note that STACK_DYNAMIC_OFFSET will have a default nonzero value if - STACK_POINTER_OFFSET or ACCUMULATE_OUTGOING_ARGS are defined). - We must also do an alignment operation on the returned value if - the stack pointer alignment is less strict than REQUIRED_ALIGN. - - If we have to align, we must leave space in SIZE for the hole - that might result from the alignment operation. */ - - must_align = (crtl->preferred_stack_boundary < required_align); - if (must_align) - { - if (required_align > PREFERRED_STACK_BOUNDARY) - extra_align = PREFERRED_STACK_BOUNDARY; - else if (required_align > STACK_BOUNDARY) - extra_align = STACK_BOUNDARY; - else - extra_align = BITS_PER_UNIT; - } - - /* ??? STACK_POINTER_OFFSET is always defined now. */ -#if defined (STACK_DYNAMIC_OFFSET) || defined (STACK_POINTER_OFFSET) - must_align = true; extra_align = BITS_PER_UNIT; -#endif - if (must_align) - { - unsigned extra = (required_align - extra_align) / BITS_PER_UNIT; + unsigned extra = (required_align - extra_align) / BITS_PER_UNIT; - size = plus_constant (Pmode, size, extra); - size = force_operand (size, NULL_RTX); + size = plus_constant (Pmode, size, extra); + size = force_operand (size, NULL_RTX); - if (flag_stack_usage_info) - stack_usage_size += extra; + if (flag_stack_usage_info) + stack_usage_size += extra; - if (extra && size_align > extra_align) - size_align = extra_align; - } + if (extra && size_align > extra_align) + size_align = extra_align; /* Round the size to a multiple of the required stack alignment. Since the stack if presumed to be rounded before this allocation, @@ -1366,7 +1334,6 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, gen_int_mode (required_align / BITS_PER_UNIT - 1, Pmode), NULL_RTX, 1, OPTAB_LIB_WIDEN); - must_align = true; } func = init_one_libfunc ("__morestack_allocate_stack_space"); @@ -1478,24 +1445,21 @@ allocate_dynamic_stack_space (rtx size, unsigned size_align, target = final_target; } - if (must_align) - { - /* CEIL_DIV_EXPR needs to worry about the addition overflowing, - but we know it can't. So add ourselves and then do - TRUNC_DIV_EXPR. */ - target = expand_binop (Pmode, add_optab, target, - gen_int_mode (required_align / BITS_PER_UNIT - 1, - Pmode), - NULL_RTX, 1, OPTAB_LIB_WIDEN); - target = expand_divmod (0, TRUNC_DIV_EXPR, Pmode, target, - gen_int_mode (required_align / BITS_PER_UNIT, - Pmode), - NULL_RTX, 1); - target = expand_mult (Pmode, target, - gen_int_mode (required_align / BITS_PER_UNIT, - Pmode), - NULL_RTX, 1); - } + /* CEIL_DIV_EXPR needs to worry about the addition overflowing, + but we know it can't. So add ourselves and then do + TRUNC_DIV_EXPR. */ + target = expand_binop (Pmode, add_optab, target, + gen_int_mode (required_align / BITS_PER_UNIT - 1, + Pmode), + NULL_RTX, 1, OPTAB_LIB_WIDEN); + target = expand_divmod (0, TRUNC_DIV_EXPR, Pmode, target, + gen_int_mode (required_align / BITS_PER_UNIT, + Pmode), + NULL_RTX, 1); + target = expand_mult (Pmode, target, + gen_int_mode (required_align / BITS_PER_UNIT, + Pmode), + NULL_RTX, 1); /* Now that we've committed to a return value, mark its alignment. */ mark_reg_pointer (target, required_align);