public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* avoid alignment of static variables affecting stack's
@ 2014-10-23  6:50 Jan Beulich
  2014-10-23  6:54 ` Jakub Jelinek
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2014-10-23  6:50 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3572 bytes --]

Function (or more narrow) scope static variables (as well as others not
placed on the stack) should also not have any effect on the stack
alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
construct using an 8-byte aligned sub-file-scope local variable.

According to my checking bad behavior started with 4.6.x (4.5.3 was
still okay), but generated code got quite a bit worse as of 4.9.0.

gcc/
2014-10-23  Jan Beulich  <jbeulich@suse.com>

	* cfgexpand.c (expand_one_var): Exclude static, external, and
	hard register variables when adjusting stack alignment related
	state.

gcc/testsuite/
2014-10-23  Jan Beulich  <jbeulich@suse.com>

	* gcc.c-torture/execute/stkalign.c: New.

--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1233,12 +1233,16 @@ static HOST_WIDE_INT
 expand_one_var (tree var, bool toplevel, bool really_expand)
 {
   unsigned int align = BITS_PER_UNIT;
+  bool stack = true;
   tree origvar = var;
 
   var = SSAVAR (var);
 
   if (TREE_TYPE (var) != error_mark_node && TREE_CODE (var) == VAR_DECL)
     {
+      stack = !TREE_STATIC (var) && !DECL_EXTERNAL (var)
+	      && !DECL_HARD_REGISTER (var);
+
       /* Because we don't know if VAR will be in register or on stack,
 	 we conservatively assume it will be on stack even if VAR is
 	 eventually put into register after RA pass.  For non-automatic
@@ -1267,22 +1271,25 @@ expand_one_var (tree var, bool toplevel,
 	align = POINTER_SIZE;
     }
 
-  if (SUPPORTS_STACK_ALIGNMENT
-      && crtl->stack_alignment_estimated < align)
+  if (stack)
     {
-      /* stack_alignment_estimated shouldn't change after stack
-         realign decision made */
-      gcc_assert (!crtl->stack_realign_processed);
-      crtl->stack_alignment_estimated = align;
+      if (SUPPORTS_STACK_ALIGNMENT
+	  && crtl->stack_alignment_estimated < align)
+	{
+	  /* stack_alignment_estimated shouldn't change after stack
+	     realign decision made */
+	  gcc_assert (!crtl->stack_realign_processed);
+	  crtl->stack_alignment_estimated = align;
+	}
+
+      /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
+	 So here we only make sure stack_alignment_needed >= align.  */
+      if (crtl->stack_alignment_needed < align)
+	crtl->stack_alignment_needed = align;
+      if (crtl->max_used_stack_slot_alignment < align)
+	crtl->max_used_stack_slot_alignment = align;
     }
 
-  /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
-     So here we only make sure stack_alignment_needed >= align.  */
-  if (crtl->stack_alignment_needed < align)
-    crtl->stack_alignment_needed = align;
-  if (crtl->max_used_stack_slot_alignment < align)
-    crtl->max_used_stack_slot_alignment = align;
-
   if (TREE_CODE (origvar) == SSA_NAME)
     {
       gcc_assert (TREE_CODE (var) != VAR_DECL
--- a/gcc/testsuite/gcc.c-torture/execute/stkalign.c
+++ b/gcc/testsuite/gcc.c-torture/execute/stkalign.c
@@ -0,0 +1,26 @@
+/* { dg-options "-fno-inline" } */
+
+#include <assert.h>
+
+#define ALIGNMENT 64
+
+unsigned test(unsigned n, unsigned p)
+{
+  static struct { char __attribute__((__aligned__(ALIGNMENT))) c; } s;
+  unsigned x;
+
+  assert(__alignof__(s) == ALIGNMENT);
+  asm ("" : "=g" (x), "+m" (s) : "0" (&x));
+
+  return n ? test(n - 1, x) : (x ^ p);
+}
+
+int main (int argc, char *argv[] __attribute__((unused)))
+{
+  unsigned int x = test(argc, 0);
+
+  x |= test(argc + 1, 0);
+  x |= test(argc + 2, 0);
+
+  return !(x & (ALIGNMENT - 1));
+}




[-- Attachment #2: gcc-trunk-stack-align-ignore-static.patch --]
[-- Type: text/plain, Size: 3623 bytes --]

avoid alignment of static variables affecting stack's

Function (or more narrow) scope static variables (as well as others not
placed on the stack) should also not have any effect on the stack
alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
construct using an 8-byte aligned sub-file-scope local variable.

According to my checking bad behavior started with 4.6.x (4.5.3 was
still okay), but generated code got quite a bit worse as of 4.9.0.

gcc/
2014-10-23  Jan Beulich  <jbeulich@suse.com>

	* cfgexpand.c (expand_one_var): Exclude static, external, and
	hard register variables when adjusting stack alignment related
	state.

gcc/testsuite/
2014-10-23  Jan Beulich  <jbeulich@suse.com>

	* gcc.c-torture/execute/stkalign.c: New.

--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1233,12 +1233,16 @@ static HOST_WIDE_INT
 expand_one_var (tree var, bool toplevel, bool really_expand)
 {
   unsigned int align = BITS_PER_UNIT;
+  bool stack = true;
   tree origvar = var;
 
   var = SSAVAR (var);
 
   if (TREE_TYPE (var) != error_mark_node && TREE_CODE (var) == VAR_DECL)
     {
+      stack = !TREE_STATIC (var) && !DECL_EXTERNAL (var)
+	      && !DECL_HARD_REGISTER (var);
+
       /* Because we don't know if VAR will be in register or on stack,
 	 we conservatively assume it will be on stack even if VAR is
 	 eventually put into register after RA pass.  For non-automatic
@@ -1267,22 +1271,25 @@ expand_one_var (tree var, bool toplevel,
 	align = POINTER_SIZE;
     }
 
-  if (SUPPORTS_STACK_ALIGNMENT
-      && crtl->stack_alignment_estimated < align)
+  if (stack)
     {
-      /* stack_alignment_estimated shouldn't change after stack
-         realign decision made */
-      gcc_assert (!crtl->stack_realign_processed);
-      crtl->stack_alignment_estimated = align;
+      if (SUPPORTS_STACK_ALIGNMENT
+	  && crtl->stack_alignment_estimated < align)
+	{
+	  /* stack_alignment_estimated shouldn't change after stack
+	     realign decision made */
+	  gcc_assert (!crtl->stack_realign_processed);
+	  crtl->stack_alignment_estimated = align;
+	}
+
+      /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
+	 So here we only make sure stack_alignment_needed >= align.  */
+      if (crtl->stack_alignment_needed < align)
+	crtl->stack_alignment_needed = align;
+      if (crtl->max_used_stack_slot_alignment < align)
+	crtl->max_used_stack_slot_alignment = align;
     }
 
-  /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
-     So here we only make sure stack_alignment_needed >= align.  */
-  if (crtl->stack_alignment_needed < align)
-    crtl->stack_alignment_needed = align;
-  if (crtl->max_used_stack_slot_alignment < align)
-    crtl->max_used_stack_slot_alignment = align;
-
   if (TREE_CODE (origvar) == SSA_NAME)
     {
       gcc_assert (TREE_CODE (var) != VAR_DECL
--- a/gcc/testsuite/gcc.c-torture/execute/stkalign.c
+++ b/gcc/testsuite/gcc.c-torture/execute/stkalign.c
@@ -0,0 +1,26 @@
+/* { dg-options "-fno-inline" } */
+
+#include <assert.h>
+
+#define ALIGNMENT 64
+
+unsigned test(unsigned n, unsigned p)
+{
+  static struct { char __attribute__((__aligned__(ALIGNMENT))) c; } s;
+  unsigned x;
+
+  assert(__alignof__(s) == ALIGNMENT);
+  asm ("" : "=g" (x), "+m" (s) : "0" (&x));
+
+  return n ? test(n - 1, x) : (x ^ p);
+}
+
+int main (int argc, char *argv[] __attribute__((unused)))
+{
+  unsigned int x = test(argc, 0);
+
+  x |= test(argc + 1, 0);
+  x |= test(argc + 2, 0);
+
+  return !(x & (ALIGNMENT - 1));
+}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-23  6:50 avoid alignment of static variables affecting stack's Jan Beulich
@ 2014-10-23  6:54 ` Jakub Jelinek
  2014-10-23  7:11   ` Jan Beulich
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2014-10-23  6:54 UTC (permalink / raw)
  To: Jan Beulich, hjl.tools; +Cc: gcc-patches

On Thu, Oct 23, 2014 at 07:30:27AM +0100, Jan Beulich wrote:
> Function (or more narrow) scope static variables (as well as others not
> placed on the stack) should also not have any effect on the stack
> alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
> construct using an 8-byte aligned sub-file-scope local variable.
> 
> According to my checking bad behavior started with 4.6.x (4.5.3 was
> still okay), but generated code got quite a bit worse as of 4.9.0.

If the static/external var has BLKmode, then perhaps it is safe, but I
wonder about other vars, say vectors etc.  Such vars are most likely
loaded from their memory location, and if for some reason that needs to be
spilled again, stack realignment would not be able to do that.
Or do we inspect the IL and for any pseudos with modes needing larger
alignment we adjust the dynamic stack realignment fields?

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-23  6:54 ` Jakub Jelinek
@ 2014-10-23  7:11   ` Jan Beulich
  2014-10-23 18:14     ` Jeff Law
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2014-10-23  7:11 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, hjl.tools

>>> On 23.10.14 at 08:50, <jakub@redhat.com> wrote:
> On Thu, Oct 23, 2014 at 07:30:27AM +0100, Jan Beulich wrote:
>> Function (or more narrow) scope static variables (as well as others not
>> placed on the stack) should also not have any effect on the stack
>> alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
>> construct using an 8-byte aligned sub-file-scope local variable.
>> 
>> According to my checking bad behavior started with 4.6.x (4.5.3 was
>> still okay), but generated code got quite a bit worse as of 4.9.0.
> 
> If the static/external var has BLKmode, then perhaps it is safe, but I
> wonder about other vars, say vectors etc.  Such vars are most likely
> loaded from their memory location, and if for some reason that needs to be
> spilled again, stack realignment would not be able to do that.
> Or do we inspect the IL and for any pseudos with modes needing larger
> alignment we adjust the dynamic stack realignment fields?

I don't know, but it would seem to me that this ought to happen
anyway: If the pseudo holds the result of some computation
other than a simple load from memory and needs spilling, the same
would apply afaict.

Furthermore, shouldn't there be an existing test case for what you
describe, and hence me not seeing regressions with the patch in
place should be sufficient proof of there not being an issue?

Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-23  7:11   ` Jan Beulich
@ 2014-10-23 18:14     ` Jeff Law
  2014-10-24  9:11       ` Jan Beulich
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff Law @ 2014-10-23 18:14 UTC (permalink / raw)
  To: Jan Beulich, Jakub Jelinek; +Cc: gcc-patches, hjl.tools

On 10/23/14 01:09, Jan Beulich wrote:
>>>> On 23.10.14 at 08:50, <jakub@redhat.com> wrote:
>> On Thu, Oct 23, 2014 at 07:30:27AM +0100, Jan Beulich wrote:
>>> Function (or more narrow) scope static variables (as well as others not
>>> placed on the stack) should also not have any effect on the stack
>>> alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
>>> construct using an 8-byte aligned sub-file-scope local variable.
>>>
>>> According to my checking bad behavior started with 4.6.x (4.5.3 was
>>> still okay), but generated code got quite a bit worse as of 4.9.0.
>>
>> If the static/external var has BLKmode, then perhaps it is safe, but I
>> wonder about other vars, say vectors etc.  Such vars are most likely
>> loaded from their memory location, and if for some reason that needs to be
>> spilled again, stack realignment would not be able to do that.
>> Or do we inspect the IL and for any pseudos with modes needing larger
>> alignment we adjust the dynamic stack realignment fields?
>
> I don't know, but it would seem to me that this ought to happen
> anyway: If the pseudo holds the result of some computation
> other than a simple load from memory and needs spilling, the same
> would apply afaict.
>
> Furthermore, shouldn't there be an existing test case for what you
> describe, and hence me not seeing regressions with the patch in
> place should be sufficient proof of there not being an issue?

For something in static storage, this seems OK.  However, I think a hard 
register variable ought to be left alone -- even if we can't spill it to 
a stack slot today, there's a reasonable chance we might add that 
capability in the future.

The testsuite is not exhaustive, and even if it were today, it may not 
be tomorrow as it can't anticipate what might change.



jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-23 18:14     ` Jeff Law
@ 2014-10-24  9:11       ` Jan Beulich
  2014-10-24  9:12         ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2014-10-24  9:11 UTC (permalink / raw)
  To: Jakub Jelinek, Jeff Law; +Cc: gcc-patches, hjl.tools

>>> On 23.10.14 at 20:13, <law@redhat.com> wrote:
> On 10/23/14 01:09, Jan Beulich wrote:
>>>>> On 23.10.14 at 08:50, <jakub@redhat.com> wrote:
>>> On Thu, Oct 23, 2014 at 07:30:27AM +0100, Jan Beulich wrote:
>>>> Function (or more narrow) scope static variables (as well as others not
>>>> placed on the stack) should also not have any effect on the stack
>>>> alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
>>>> construct using an 8-byte aligned sub-file-scope local variable.
>>>>
>>>> According to my checking bad behavior started with 4.6.x (4.5.3 was
>>>> still okay), but generated code got quite a bit worse as of 4.9.0.
>>>
>>> If the static/external var has BLKmode, then perhaps it is safe, but I
>>> wonder about other vars, say vectors etc.  Such vars are most likely
>>> loaded from their memory location, and if for some reason that needs to be
>>> spilled again, stack realignment would not be able to do that.
>>> Or do we inspect the IL and for any pseudos with modes needing larger
>>> alignment we adjust the dynamic stack realignment fields?
>>
>> I don't know, but it would seem to me that this ought to happen
>> anyway: If the pseudo holds the result of some computation
>> other than a simple load from memory and needs spilling, the same
>> would apply afaict.
> 
> For something in static storage, this seems OK.  However, I think a hard 
> register variable ought to be left alone -- even if we can't spill it to 
> a stack slot today, there's a reasonable chance we might add that 
> capability in the future.

Hmm, but then wouldn't it need to be the code generating the spill
that's responsible for enforcing suitable alignment? I can certainly
re-submit without the hard register special cased (as it would still
fix the original issue I'm seeing), but it feels wrong to do so.

Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-24  9:11       ` Jan Beulich
@ 2014-10-24  9:12         ` Richard Biener
  2014-10-24  9:18           ` Jan Beulich
  2014-10-24  9:19           ` Jakub Jelinek
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Biener @ 2014-10-24  9:12 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Jakub Jelinek, Jeff Law, GCC Patches, H.J. Lu

On Fri, Oct 24, 2014 at 11:01 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 23.10.14 at 20:13, <law@redhat.com> wrote:
>> On 10/23/14 01:09, Jan Beulich wrote:
>>>>>> On 23.10.14 at 08:50, <jakub@redhat.com> wrote:
>>>> On Thu, Oct 23, 2014 at 07:30:27AM +0100, Jan Beulich wrote:
>>>>> Function (or more narrow) scope static variables (as well as others not
>>>>> placed on the stack) should also not have any effect on the stack
>>>>> alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
>>>>> construct using an 8-byte aligned sub-file-scope local variable.
>>>>>
>>>>> According to my checking bad behavior started with 4.6.x (4.5.3 was
>>>>> still okay), but generated code got quite a bit worse as of 4.9.0.
>>>>
>>>> If the static/external var has BLKmode, then perhaps it is safe, but I
>>>> wonder about other vars, say vectors etc.  Such vars are most likely
>>>> loaded from their memory location, and if for some reason that needs to be
>>>> spilled again, stack realignment would not be able to do that.
>>>> Or do we inspect the IL and for any pseudos with modes needing larger
>>>> alignment we adjust the dynamic stack realignment fields?
>>>
>>> I don't know, but it would seem to me that this ought to happen
>>> anyway: If the pseudo holds the result of some computation
>>> other than a simple load from memory and needs spilling, the same
>>> would apply afaict.
>>
>> For something in static storage, this seems OK.  However, I think a hard
>> register variable ought to be left alone -- even if we can't spill it to
>> a stack slot today, there's a reasonable chance we might add that
>> capability in the future.
>
> Hmm, but then wouldn't it need to be the code generating the spill
> that's responsible for enforcing suitable alignment? I can certainly
> re-submit without the hard register special cased (as it would still
> fix the original issue I'm seeing), but it feels wrong to do so.

Yes, ISTR the spilling code is supposed to update the required
stack alignment.  After all the RA decision might affect required
alignment of spills.

Richard.

> Jan
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-24  9:12         ` Richard Biener
@ 2014-10-24  9:18           ` Jan Beulich
  2014-10-24  9:19           ` Jakub Jelinek
  1 sibling, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2014-10-24  9:18 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches, H.J. Lu, Jakub Jelinek, Jeff Law

>>> On 24.10.14 at 11:10, <richard.guenther@gmail.com> wrote:
> On Fri, Oct 24, 2014 at 11:01 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 23.10.14 at 20:13, <law@redhat.com> wrote:
>>> On 10/23/14 01:09, Jan Beulich wrote:
>>>>>>> On 23.10.14 at 08:50, <jakub@redhat.com> wrote:
>>>>> On Thu, Oct 23, 2014 at 07:30:27AM +0100, Jan Beulich wrote:
>>>>>> Function (or more narrow) scope static variables (as well as others not
>>>>>> placed on the stack) should also not have any effect on the stack
>>>>>> alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
>>>>>> construct using an 8-byte aligned sub-file-scope local variable.
>>>>>>
>>>>>> According to my checking bad behavior started with 4.6.x (4.5.3 was
>>>>>> still okay), but generated code got quite a bit worse as of 4.9.0.
>>>>>
>>>>> If the static/external var has BLKmode, then perhaps it is safe, but I
>>>>> wonder about other vars, say vectors etc.  Such vars are most likely
>>>>> loaded from their memory location, and if for some reason that needs to be
>>>>> spilled again, stack realignment would not be able to do that.
>>>>> Or do we inspect the IL and for any pseudos with modes needing larger
>>>>> alignment we adjust the dynamic stack realignment fields?
>>>>
>>>> I don't know, but it would seem to me that this ought to happen
>>>> anyway: If the pseudo holds the result of some computation
>>>> other than a simple load from memory and needs spilling, the same
>>>> would apply afaict.
>>>
>>> For something in static storage, this seems OK.  However, I think a hard
>>> register variable ought to be left alone -- even if we can't spill it to
>>> a stack slot today, there's a reasonable chance we might add that
>>> capability in the future.
>>
>> Hmm, but then wouldn't it need to be the code generating the spill
>> that's responsible for enforcing suitable alignment? I can certainly
>> re-submit without the hard register special cased (as it would still
>> fix the original issue I'm seeing), but it feels wrong to do so.
> 
> Yes, ISTR the spilling code is supposed to update the required
> stack alignment.  After all the RA decision might affect required
> alignment of spills.

Thanks for confirming. So is the patch then okay to commit as is?

Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-24  9:12         ` Richard Biener
  2014-10-24  9:18           ` Jan Beulich
@ 2014-10-24  9:19           ` Jakub Jelinek
  2014-10-24  9:54             ` Richard Biener
  1 sibling, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2014-10-24  9:19 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Beulich, Jeff Law, GCC Patches, H.J. Lu

On Fri, Oct 24, 2014 at 11:10:08AM +0200, Richard Biener wrote:
> >> For something in static storage, this seems OK.  However, I think a hard
> >> register variable ought to be left alone -- even if we can't spill it to
> >> a stack slot today, there's a reasonable chance we might add that
> >> capability in the future.
> >
> > Hmm, but then wouldn't it need to be the code generating the spill
> > that's responsible for enforcing suitable alignment? I can certainly
> > re-submit without the hard register special cased (as it would still
> > fix the original issue I'm seeing), but it feels wrong to do so.
> 
> Yes, ISTR the spilling code is supposed to update the required
> stack alignment.  After all the RA decision might affect required
> alignment of spills.

From what I remember, at RA time you already have to know conservatively
that you'll want to do dynamic stack realignment and what the highest needed
alignment will be, so various parts of expansion etc. conservatively compute
what will be needed.  I think that is because you e.g. need to reserve some
registers (vDRAP, etc.) if doing dynamic realignment.
If you conservatively assume you'll need dynamic stack realignment and after
RA you find you really don't need it, there are some optimizations in
prologue threading where it attempts to at least decrease amount of
unnecessary code, but the harm has already been done.

Might be that with LRA perhaps this could be changed and not conservatively
assume more alignment than proven to be needed, but such code isn't there I
think.

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-24  9:19           ` Jakub Jelinek
@ 2014-10-24  9:54             ` Richard Biener
  2014-10-24 10:16               ` Jan Beulich
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2014-10-24  9:54 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jan Beulich, Jeff Law, GCC Patches, H.J. Lu

On Fri, Oct 24, 2014 at 11:18 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Oct 24, 2014 at 11:10:08AM +0200, Richard Biener wrote:
>> >> For something in static storage, this seems OK.  However, I think a hard
>> >> register variable ought to be left alone -- even if we can't spill it to
>> >> a stack slot today, there's a reasonable chance we might add that
>> >> capability in the future.
>> >
>> > Hmm, but then wouldn't it need to be the code generating the spill
>> > that's responsible for enforcing suitable alignment? I can certainly
>> > re-submit without the hard register special cased (as it would still
>> > fix the original issue I'm seeing), but it feels wrong to do so.
>>
>> Yes, ISTR the spilling code is supposed to update the required
>> stack alignment.  After all the RA decision might affect required
>> alignment of spills.
>
> From what I remember, at RA time you already have to know conservatively
> that you'll want to do dynamic stack realignment and what the highest needed
> alignment will be, so various parts of expansion etc. conservatively compute
> what will be needed.  I think that is because you e.g. need to reserve some
> registers (vDRAP, etc.) if doing dynamic realignment.
> If you conservatively assume you'll need dynamic stack realignment and after
> RA you find you really don't need it, there are some optimizations in
> prologue threading where it attempts to at least decrease amount of
> unnecessary code, but the harm has already been done.
>
> Might be that with LRA perhaps this could be changed and not conservatively
> assume more alignment than proven to be needed, but such code isn't there I
> think.

I stand corrected then.

Richard.

>         Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-24  9:54             ` Richard Biener
@ 2014-10-24 10:16               ` Jan Beulich
  2014-10-24 10:42                 ` Richard Biener
  2014-10-24 16:42                 ` Jeff Law
  0 siblings, 2 replies; 12+ messages in thread
From: Jan Beulich @ 2014-10-24 10:16 UTC (permalink / raw)
  To: Richard Biener, Jakub Jelinek; +Cc: GCC Patches, H.J. Lu, Jeff Law

>>> On 24.10.14 at 11:52, <richard.guenther@gmail.com> wrote:
> On Fri, Oct 24, 2014 at 11:18 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Fri, Oct 24, 2014 at 11:10:08AM +0200, Richard Biener wrote:
>>> >> For something in static storage, this seems OK.  However, I think a hard
>>> >> register variable ought to be left alone -- even if we can't spill it to
>>> >> a stack slot today, there's a reasonable chance we might add that
>>> >> capability in the future.
>>> >
>>> > Hmm, but then wouldn't it need to be the code generating the spill
>>> > that's responsible for enforcing suitable alignment? I can certainly
>>> > re-submit without the hard register special cased (as it would still
>>> > fix the original issue I'm seeing), but it feels wrong to do so.
>>>
>>> Yes, ISTR the spilling code is supposed to update the required
>>> stack alignment.  After all the RA decision might affect required
>>> alignment of spills.
>>
>> From what I remember, at RA time you already have to know conservatively
>> that you'll want to do dynamic stack realignment and what the highest needed
>> alignment will be, so various parts of expansion etc. conservatively compute
>> what will be needed.  I think that is because you e.g. need to reserve some
>> registers (vDRAP, etc.) if doing dynamic realignment.
>> If you conservatively assume you'll need dynamic stack realignment and after
>> RA you find you really don't need it, there are some optimizations in
>> prologue threading where it attempts to at least decrease amount of
>> unnecessary code, but the harm has already been done.
>>
>> Might be that with LRA perhaps this could be changed and not conservatively
>> assume more alignment than proven to be needed, but such code isn't there I
>> think.
> 
> I stand corrected then.

So am I to conclude then that I need to take out the hard register
check in order for this to be accepted?

Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-24 10:16               ` Jan Beulich
@ 2014-10-24 10:42                 ` Richard Biener
  2014-10-24 16:42                 ` Jeff Law
  1 sibling, 0 replies; 12+ messages in thread
From: Richard Biener @ 2014-10-24 10:42 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Jakub Jelinek, GCC Patches, H.J. Lu, Jeff Law

On Fri, Oct 24, 2014 at 12:12 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 24.10.14 at 11:52, <richard.guenther@gmail.com> wrote:
>> On Fri, Oct 24, 2014 at 11:18 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Fri, Oct 24, 2014 at 11:10:08AM +0200, Richard Biener wrote:
>>>> >> For something in static storage, this seems OK.  However, I think a hard
>>>> >> register variable ought to be left alone -- even if we can't spill it to
>>>> >> a stack slot today, there's a reasonable chance we might add that
>>>> >> capability in the future.
>>>> >
>>>> > Hmm, but then wouldn't it need to be the code generating the spill
>>>> > that's responsible for enforcing suitable alignment? I can certainly
>>>> > re-submit without the hard register special cased (as it would still
>>>> > fix the original issue I'm seeing), but it feels wrong to do so.
>>>>
>>>> Yes, ISTR the spilling code is supposed to update the required
>>>> stack alignment.  After all the RA decision might affect required
>>>> alignment of spills.
>>>
>>> From what I remember, at RA time you already have to know conservatively
>>> that you'll want to do dynamic stack realignment and what the highest needed
>>> alignment will be, so various parts of expansion etc. conservatively compute
>>> what will be needed.  I think that is because you e.g. need to reserve some
>>> registers (vDRAP, etc.) if doing dynamic realignment.
>>> If you conservatively assume you'll need dynamic stack realignment and after
>>> RA you find you really don't need it, there are some optimizations in
>>> prologue threading where it attempts to at least decrease amount of
>>> unnecessary code, but the harm has already been done.
>>>
>>> Might be that with LRA perhaps this could be changed and not conservatively
>>> assume more alignment than proven to be needed, but such code isn't there I
>>> think.
>>
>> I stand corrected then.
>
> So am I to conclude then that I need to take out the hard register
> check in order for this to be accepted?

Yes.

Thanks,
Richard.

> Jan
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: avoid alignment of static variables affecting stack's
  2014-10-24 10:16               ` Jan Beulich
  2014-10-24 10:42                 ` Richard Biener
@ 2014-10-24 16:42                 ` Jeff Law
  1 sibling, 0 replies; 12+ messages in thread
From: Jeff Law @ 2014-10-24 16:42 UTC (permalink / raw)
  To: Jan Beulich, Richard Biener, Jakub Jelinek; +Cc: GCC Patches, H.J. Lu

On 10/24/14 04:12, Jan Beulich wrote:
>>>> On 24.10.14 at 11:52, <richard.guenther@gmail.com> wrote:
>> On Fri, Oct 24, 2014 at 11:18 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Fri, Oct 24, 2014 at 11:10:08AM +0200, Richard Biener wrote:
>>>>>> For something in static storage, this seems OK.  However, I think a hard
>>>>>> register variable ought to be left alone -- even if we can't spill it to
>>>>>> a stack slot today, there's a reasonable chance we might add that
>>>>>> capability in the future.
>>>>>
>>>>> Hmm, but then wouldn't it need to be the code generating the spill
>>>>> that's responsible for enforcing suitable alignment? I can certainly
>>>>> re-submit without the hard register special cased (as it would still
>>>>> fix the original issue I'm seeing), but it feels wrong to do so.
>>>>
>>>> Yes, ISTR the spilling code is supposed to update the required
>>>> stack alignment.  After all the RA decision might affect required
>>>> alignment of spills.
>>>
>>>  From what I remember, at RA time you already have to know conservatively
>>> that you'll want to do dynamic stack realignment and what the highest needed
>>> alignment will be, so various parts of expansion etc. conservatively compute
>>> what will be needed.  I think that is because you e.g. need to reserve some
>>> registers (vDRAP, etc.) if doing dynamic realignment.
>>> If you conservatively assume you'll need dynamic stack realignment and after
>>> RA you find you really don't need it, there are some optimizations in
>>> prologue threading where it attempts to at least decrease amount of
>>> unnecessary code, but the harm has already been done.
>>>
>>> Might be that with LRA perhaps this could be changed and not conservatively
>>> assume more alignment than proven to be needed, but such code isn't there I
>>> think.
>>
>> I stand corrected then.
>
> So am I to conclude then that I need to take out the hard register
> check in order for this to be accepted?
At least for now, yes.  We can always revisit hard registers if/when 
IRA/LRA can be enhanced to deal with these issues.


jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-10-24 16:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-23  6:50 avoid alignment of static variables affecting stack's Jan Beulich
2014-10-23  6:54 ` Jakub Jelinek
2014-10-23  7:11   ` Jan Beulich
2014-10-23 18:14     ` Jeff Law
2014-10-24  9:11       ` Jan Beulich
2014-10-24  9:12         ` Richard Biener
2014-10-24  9:18           ` Jan Beulich
2014-10-24  9:19           ` Jakub Jelinek
2014-10-24  9:54             ` Richard Biener
2014-10-24 10:16               ` Jan Beulich
2014-10-24 10:42                 ` Richard Biener
2014-10-24 16:42                 ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).