This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[RFA] Minor cleanup to allocate_dynamic_stack_space
- From: Jeff Law <law at redhat dot com>
- To: vogt at linux dot vnet dot ibm dot com, gcc-patches at gcc dot gnu dot org, Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>
- Date: Fri, 20 May 2016 15:23:49 -0600
- Subject: [RFA] Minor cleanup to allocate_dynamic_stack_space
- Authentication-results: sourceware.org; auth=none
- References: <20160429221242 dot GA2205 at linux dot vnet dot ibm dot com> <e128d673-061b-5b9a-90ef-84613093bf90 at redhat dot com> <20160503141753 dot GA17351 at linux dot vnet dot ibm dot com> <34a785ea-bdd2-023c-4c02-a31dbadf0142 at redhat dot com>
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 <law@redhat.com>
+
+ * explow.c (allocate_dynamic_stack_space): Simplify
+ knowing that MUST_ALIGN was always true.
+
2016-05-16 Ryan Burn <contact@rnburn.com>
* 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);
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 <law@redhat.com>
+
+ * explow.c (allocate_dynamic_stack_space): Simplify
+ knowing that MUST_ALIGN was always true.
+
2016-05-16 Ryan Burn <contact@rnburn.com>
* 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);