Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-06-02 Thread Richard Earnshaw 
On 01/06/15 13:07, Jakub Jelinek wrote:
 On Thu, May 07, 2015 at 12:16:32PM +0100, Alan Lawrence wrote:
 So for my two cents, or perhaps three:
 
 Any progress on this PR?
 A P1 bug that affects several packages stalled for a month isn't a very good
 thing... (not to mention broken profiledbootstrap on ARM due to the same
 issue).
 I've checked and llvm on ARM ignores the alignment on the scalar
 arguments...
 
   Jakub
 

We're working on some updates to the ABI documents.  If we're going to
break ABI compatibility, even in some corner cases, it would make sense
to only do this once.

We need to think about more than just LLVM and GCC, so it's not as
simple as just copying what LLVM does.

Note that there's almost certainly a similar problem for AArch64, though
it is probably less common for it to manifest itself -- probably only
when structs contain 128-bit aligned objects.

R.

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-06-02 Thread Alan Lawrence 

Richard Earnshaw wrote:

On 01/06/15 13:07, Jakub Jelinek wrote:

On Thu, May 07, 2015 at 12:16:32PM +0100, Alan Lawrence wrote:

So for my two cents, or perhaps three:

Any progress on this PR?
A P1 bug that affects several packages stalled for a month isn't a very good
thing... (not to mention broken profiledbootstrap on ARM due to the same
issue).
I've checked and llvm on ARM ignores the alignment on the scalar
arguments...

Jakub



We're working on some updates to the ABI documents.  If we're going to
break ABI compatibility, even in some corner cases, it would make sense
to only do this once.


One question is whether to treat structs differently from scalars in the ABI 
specification. Structs raise lots of corner cases! I notice the following 
oddity, and wonder if anyone can shed any light on this:


typedef __attribute__((aligned(8))) struct { int x; int y; } foo;
typedef struct __attribute__((aligned(8))) { int x; int y; } bar;
typedef struct { int x; int y; } __attribute__((aligned(8))) baz;
typedef struct { int x; int y; } qux __attribute__((aligned(8)));

create typedefs (foo, bar, baz, qux) all with alignment 8, as expected. However, 
the TYPE_MAIN_VARIANT (an anonymous struct type) has alignment 4 for foo and qux 
(so the attribute has been applied only to the typedef), but alignment 8 for bar 
and baz (i.e. the attribute has been applied to the underlying struct).


--Alan

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-06-01 Thread Jakub Jelinek 
On Thu, May 07, 2015 at 12:16:32PM +0100, Alan Lawrence wrote:
 So for my two cents, or perhaps three:

Any progress on this PR?
A P1 bug that affects several packages stalled for a month isn't a very good
thing... (not to mention broken profiledbootstrap on ARM due to the same
issue).
I've checked and llvm on ARM ignores the alignment on the scalar
arguments...

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-07 Thread Jakub Jelinek 
On Thu, May 07, 2015 at 12:16:32PM +0100, Alan Lawrence wrote:
 So for my two cents, or perhaps three:
 
 (1) It'd be great to have something in the documentation for
 TARGET_FUNCTION_ARG that explains what the contract for the type information
 provided is. Even/especially if some of this is considered useless i.e.
 provided on a best effort basis by compiler analysis. (What happens in other
 cases where there are subtyping-like relationships? E.g. genuine C++
 subtyping, or other attributes like const, volatile? I don't imagine that
 any backend's ABI depends on those attributes, but do we want to rule that
 out, e.g. for attributes that don't exist yet?)

Sure, documentation improvements are always welcome.
Yes, const/volatile/__restrict, or various attributes like deprecated and
various others shouldn't be taken into account when deciding on argument (or
return value) passing.

 (2) If all backends were expected to save the function prototype in
 TARGET_INIT_CUMULATIVE_ARGS and use that, it'd be a shame to duplicate that.
 However, it seems this is because other backends don't depend on TYPE_ALIGN.
 It doesn't seem unreasonable for argument passing rules to depend on
 alignment, however.

Most of them don't need that.  The type they see in the callee is compatible
with the type they see in the caller (except for when the user is supposed
to ensure that, like KR calls or va_arg), otherwise supposedly the FEs
should loudly complain.
But, stuff like alignment or various other attributes on scalar types
are considered useless for function calls by the FEs, and so the backends
should treat them IMNSHO too.  Thus argument passing rules depending on
alignment of scalars is unreasonable.

BTW, I've checked llvm and it ignores the alignment attribute on scalar
types for argument passing, so the patch I've posted, in addition to
allowing GCC to be compatible with itself, would make it compatible with
clang too in that regard.

 (3) C++11 rules out using __align_as on function arguments, including via a
 typedef. GCC allows __attribute__ aligned on arguments *only via a typedef*.
 Is that how we want it?

Trying to change a GCC extension accepted for many years is not a good idea.

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-07 Thread Alan Lawrence 

Richard Biener wrote:

On May 5, 2015 4:33:58 PM GMT+02:00, Richard Earnshaw 
richard.earns...@foss.arm.com wrote:

On 05/05/15 15:33, Richard Earnshaw wrote:

On 05/05/15 15:29, Jakub Jelinek wrote:

On Tue, May 05, 2015 at 02:20:43PM +0100, Richard Earnshaw wrote:

On 05/05/15 14:06, Jakub Jelinek wrote:

For the middle-end, the TYPE_ALIGN info on expression types is

considered

useless, you can get there anything.  There is no conversion rule

to what

you get for myalignedint + int, or (myalignedint) int, or (int)
myalignedint, etc.

create a consistent view of the values passed into the back-end? 

It

seems to me that at present the back-end has to be psychic as to

what is

really happening.

No, the backend just shouldn't consider TYPE_ALIGN on the scalars,

and it

seems only arm ever looks at that.


Nothing in the specification for TARGET_FUNCTION_ARG (or any of the
related functions) makes any mention of this...

While this requirement isn't documented, it is clearly common sense

or at

least something any kind of testing would reveal immediately.

Then clearly no such tests exist in the testsuite :-(


Or, more precisely, no such tests existed in 2008, when the code went
in.


That just means that the code was the first that make this matter.

BTW, what do other compilers do (I suppose llvm supports the gcc extensions for 
alignment)?  Can llvm and gcc interoperate properly here? Do the cited 
testcases work with llvm?

Richard.


So for my two cents, or perhaps three:

(1) It'd be great to have something in the documentation for TARGET_FUNCTION_ARG 
that explains what the contract for the type information provided is. 
Even/especially if some of this is considered useless i.e. provided on a best 
effort basis by compiler analysis. (What happens in other cases where there are 
subtyping-like relationships? E.g. genuine C++ subtyping, or other attributes 
like const, volatile? I don't imagine that any backend's ABI depends on those 
attributes, but do we want to rule that out, e.g. for attributes that don't 
exist yet?)


(2) If all backends were expected to save the function prototype in 
TARGET_INIT_CUMULATIVE_ARGS and use that, it'd be a shame to duplicate that. 
However, it seems this is because other backends don't depend on TYPE_ALIGN. It 
doesn't seem unreasonable for argument passing rules to depend on alignment, 
however.


(3) C++11 rules out using __align_as on function arguments, including via a 
typedef. GCC allows __attribute__ aligned on arguments *only via a typedef*. Is 
that how we want it?


--Alan

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-06 Thread Richard Earnshaw 
On 05/05/15 19:07, Richard Biener wrote:
 On May 5, 2015 4:33:58 PM GMT+02:00, Richard Earnshaw 
 richard.earns...@foss.arm.com wrote:
 On 05/05/15 15:33, Richard Earnshaw wrote:
 On 05/05/15 15:29, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:20:43PM +0100, Richard Earnshaw wrote:
 On 05/05/15 14:06, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:02:19PM +0100, Richard Earnshaw wrote:
 In a literal sense, yes.  However, even KR  stdarg have
 standard
 promotion and conversion rules (size  int = int, floats
 promoted to
 double, etc).  What are those rules for GCC's overaligned types
 (ie
 where in the docs does it say what happens and how a back-end
 should
 interpret them)?  Shouldn't the mid-end be doing that work so as
 to

 For the middle-end, the TYPE_ALIGN info on expression types is
 considered
 useless, you can get there anything.  There is no conversion rule
 to what
 you get for myalignedint + int, or (myalignedint) int, or (int)
 myalignedint, etc.

 create a consistent view of the values passed into the back-end? 
 It
 seems to me that at present the back-end has to be psychic as to
 what is
 really happening.

 No, the backend just shouldn't consider TYPE_ALIGN on the scalars,
 and it
 seems only arm ever looks at that.


 Nothing in the specification for TARGET_FUNCTION_ARG (or any of the
 related functions) makes any mention of this...

 While this requirement isn't documented, it is clearly common sense
 or at
 least something any kind of testing would reveal immediately.

 Then clearly no such tests exist in the testsuite :-(


 Or, more precisely, no such tests existed in 2008, when the code went
 in.
 
 That just means that the code was the first that make this matter.
 

Exactly.  The comment was in response to the suggestion the code wasn't
tested.

 BTW, what do other compilers do (I suppose llvm supports the gcc extensions 
 for alignment)?  Can llvm and gcc interoperate properly here? Do the cited 
 testcases work with llvm?
 

We're looking into it.  I'm talking to our LLVM team and we'll make sure
we have a consistent set of rules.

R.

 Richard.
 

 R.

 R.

 And it is nothing broken recently (except that with the SRA changes
 it hits
 much more often), looking e.g. at GCC 3.2, I'm seeing that
 expand_call is on
 that testcase also called with pretty random TYPE_ALIGN on the
 argument
 types; we didn't have GIMPLE then, so it is nothing that GIMPLE
 brought in.

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 13:37, Richard Biener wrote:
 On May 5, 2015 1:01:59 PM GMT+02:00, Richard Earnshaw 
 richard.earns...@foss.arm.com wrote:
 On 05/05/15 11:54, Richard Earnshaw wrote:
 On 05/05/15 08:32, Jakub Jelinek wrote:
 On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for non-aggregates
 would
 likely not break anything that hasn't been broken already and would
 unbreak
 the majority of cases.

 Attached (untested so far).  It indeed changes code generated for
 over-aligned va_arg, but as I believe you can't properly pass those
 in the
 ... caller, this should just fix it so that va_arg handling matches
 the
 caller (and likewise for callees for named argument passing).

 The following testcase shows that eipa_sra changes alignment even
 for the
 aggregates.  Change aligned (8) to aligned (4) to see another
 possibility.

 Actually I misread it, for the aggregates esra actually doesn't
 change
 anything, which is the reason why the testcase doesn't fail.
 The problem with the scalars is that esra first changed it to the
 over-aligned MEM_REFs and then later on eipa_sra used the types of
 the
 MEM_REFs created by esra.

 2015-05-05  Jakub Jelinek  ja...@redhat.com

PR target/65956
* config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type
 itself.

* gcc.c-torture/execute/pr65956.c: New test.

 --- gcc/config/arm/arm.c.jj2015-05-04 21:51:42.0 +0200
 +++ gcc/config/arm/arm.c   2015-05-05 09:20:52.481693337 +0200
 @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -|| (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
 +return true;

 I don't think this is right (though I suspect the existing code has
 the
 same problem).  We should only look at mode if there is no type
 information.  The problem is that GCC has a nasty habit of assigning
 real machine modes to things that are really BLKmode and we've run
 into
 several cases where this has royally screwed things up.  So for
 consistency in the ARM back-end we are careful to only use mode when
 type is NULL (= it's a libcall).

 +  if (type == NULL_TREE)
 +return false;
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
  }
  



 It ought to be possible to re-order this, though, to

  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 - || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (type != NULL_TREE)
 +{
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
 +}
 +  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY);
  }


 Either way, this would need careful cross-testing against an existing
 compiler.


 It looks as though either patch would cause ABI incompatibility for

 typedef int alignedint __attribute__((aligned((8;

 int  __attribute__((weak)) foo (int a, alignedint b)
 {return b;}

 void bar (alignedint x)
 {
  foo (1, x);
 }

 Where currently gcc uses r2 as the argument register for b in foo.
 
 And for foo (1,2) or an int typed 2nd arg?
 
 Richard.
 
 R.
 
 


As is

int foo2 (int a, int b);

void bar2 (alignedint x)
{
  foo2 (1, x);
}

...

R

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 13:46, Richard Earnshaw wrote:
 On 05/05/15 13:37, Richard Biener wrote:
 On May 5, 2015 1:01:59 PM GMT+02:00, Richard Earnshaw 
 richard.earns...@foss.arm.com wrote:
 On 05/05/15 11:54, Richard Earnshaw wrote:
 On 05/05/15 08:32, Jakub Jelinek wrote:
 On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for non-aggregates
 would
 likely not break anything that hasn't been broken already and would
 unbreak
 the majority of cases.

 Attached (untested so far).  It indeed changes code generated for
 over-aligned va_arg, but as I believe you can't properly pass those
 in the
 ... caller, this should just fix it so that va_arg handling matches
 the
 caller (and likewise for callees for named argument passing).

 The following testcase shows that eipa_sra changes alignment even
 for the
 aggregates.  Change aligned (8) to aligned (4) to see another
 possibility.

 Actually I misread it, for the aggregates esra actually doesn't
 change
 anything, which is the reason why the testcase doesn't fail.
 The problem with the scalars is that esra first changed it to the
 over-aligned MEM_REFs and then later on eipa_sra used the types of
 the
 MEM_REFs created by esra.

 2015-05-05  Jakub Jelinek  ja...@redhat.com

   PR target/65956
   * config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
   types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type
 itself.

   * gcc.c-torture/execute/pr65956.c: New test.

 --- gcc/config/arm/arm.c.jj   2015-05-04 21:51:42.0 +0200
 +++ gcc/config/arm/arm.c  2015-05-05 09:20:52.481693337 +0200
 @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -   || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
 +return true;

 I don't think this is right (though I suspect the existing code has
 the
 same problem).  We should only look at mode if there is no type
 information.  The problem is that GCC has a nasty habit of assigning
 real machine modes to things that are really BLKmode and we've run
 into
 several cases where this has royally screwed things up.  So for
 consistency in the ARM back-end we are careful to only use mode when
 type is NULL (= it's a libcall).

 +  if (type == NULL_TREE)
 +return false;
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
  }
  



 It ought to be possible to re-order this, though, to

  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -|| (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (type != NULL_TREE)
 +{
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
 +}
 +  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY);
  }


 Either way, this would need careful cross-testing against an existing
 compiler.


 It looks as though either patch would cause ABI incompatibility for

 typedef int alignedint __attribute__((aligned((8;

 int  __attribute__((weak)) foo (int a, alignedint b)
 {return b;}

 void bar (alignedint x)
 {
  foo (1, x);
 }

 Where currently gcc uses r2 as the argument register for b in foo.

 And for foo (1,2) or an int typed 2nd arg?

 Richard.

 R.


 
 
 As is
 
 int foo2 (int a, int b);
 
 void bar2 (alignedint x)
 {
   foo2 (1, x);
 }
 

The real question here is why is TYPE the type of the value, rather than
the type of the formal as expressed by the prototype (or implicit
prototype in the case of variadics or KR)?  Surely this is the mid-end
passing the wrong information to the back-end.

R.

 ...
 
 R

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Jakub Jelinek 
On Tue, May 05, 2015 at 01:49:55PM +0100, Richard Earnshaw wrote:
 The real question here is why is TYPE the type of the value, rather than
 the type of the formal as expressed by the prototype (or implicit
 prototype in the case of variadics or KR)?  Surely this is the mid-end
 passing the wrong information to the back-end.

There is nothing else for unnamed arguments (KR, stdarg).
For named arguments, the backend has the option to save the fntype in
CUMULATIVE_ARGS and look it up when it needs that.  But, that will
still mean KR and stdarg will be just broken on arm.

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Jakub Jelinek 
On Tue, May 05, 2015 at 02:02:19PM +0100, Richard Earnshaw wrote:
 In a literal sense, yes.  However, even KR  stdarg have standard
 promotion and conversion rules (size  int = int, floats promoted to
 double, etc).  What are those rules for GCC's overaligned types (ie
 where in the docs does it say what happens and how a back-end should
 interpret them)?  Shouldn't the mid-end be doing that work so as to

For the middle-end, the TYPE_ALIGN info on expression types is considered
useless, you can get there anything.  There is no conversion rule to what
you get for myalignedint + int, or (myalignedint) int, or (int)
myalignedint, etc.

 create a consistent view of the values passed into the back-end?  It
 seems to me that at present the back-end has to be psychic as to what is
 really happening.

No, the backend just shouldn't consider TYPE_ALIGN on the scalars, and it
seems only arm ever looks at that.

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Jakub Jelinek 
On Tue, May 05, 2015 at 12:01:59PM +0100, Richard Earnshaw wrote:
  Either way, this would need careful cross-testing against an existing
  compiler.
  
 
 It looks as though either patch would cause ABI incompatibility for
 
 typedef int alignedint __attribute__((aligned((8;
 
 int  __attribute__((weak)) foo (int a, alignedint b)
 {return b;}
 
 void bar (alignedint x)
 {
   foo (1, x);
 }
 
 Where currently gcc uses r2 as the argument register for b in foo.

That is true, but as you can't call it with almost anything else:

void bar2 (alignedint x)
{
  foo (1, (alignedint) 1);
}

void bar3 (alignedint x)
{
  foo (1, x + (alignedint) 1);
}

alignedint q;

void bar4 (alignedint x)
{
  foo (1, q);
}

eextern int baz (int, int);

void bar5 (alignedint x)
{
  baz (1, x);
}

is all broken, I'd seriously doubt anybody is actually using it
successfully.
Having an ABI feature that most of the time doesn't work at
all, and occassionally happens to work, is just unsupportable.

You could add a -Wpsabi warning in the callee, warning if any of the
non-aggregate arguments has
TYPE_ALIGN (type) != TYPE_ALIGN (TYPE_MAIN_VARIANT (type))
to let users know that this really didn't work in = 5.1.
And supposedly we need the tree-sra.c patch then so that eipa_sra
doesn't create the over-aligned parameters, because -Wpsabi then would warn
about those.

In any case, this is a P1 issue that needs resolving RSN, not just on the
trunk, but also on the branch, and the tree-sra.c patch I've posted isn't
anywhere close to resolving it without fixing the backend.

I'm in the middle of bootstrapping/regtesting this on armv7hl (both
profiledbootstrap to see if it fixes it and normal one).

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 13:54, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 01:49:55PM +0100, Richard Earnshaw wrote:
 The real question here is why is TYPE the type of the value, rather than
 the type of the formal as expressed by the prototype (or implicit
 prototype in the case of variadics or KR)?  Surely this is the mid-end
 passing the wrong information to the back-end.
 
 There is nothing else for unnamed arguments (KR, stdarg).
 For named arguments, the backend has the option to save the fntype in
 CUMULATIVE_ARGS and look it up when it needs that.  But, that will
 still mean KR and stdarg will be just broken on arm.
 
   Jakub
 

In a literal sense, yes.  However, even KR  stdarg have standard
promotion and conversion rules (size  int = int, floats promoted to
double, etc).  What are those rules for GCC's overaligned types (ie
where in the docs does it say what happens and how a back-end should
interpret them)?  Shouldn't the mid-end be doing that work so as to
create a consistent view of the values passed into the back-end?  It
seems to me that at present the back-end has to be psychic as to what is
really happening.

R.

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 14:06, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:02:19PM +0100, Richard Earnshaw wrote:
 In a literal sense, yes.  However, even KR  stdarg have standard
 promotion and conversion rules (size  int = int, floats promoted to
 double, etc).  What are those rules for GCC's overaligned types (ie
 where in the docs does it say what happens and how a back-end should
 interpret them)?  Shouldn't the mid-end be doing that work so as to
 
 For the middle-end, the TYPE_ALIGN info on expression types is considered
 useless, you can get there anything.  There is no conversion rule to what
 you get for myalignedint + int, or (myalignedint) int, or (int)
 myalignedint, etc.
 
 create a consistent view of the values passed into the back-end?  It
 seems to me that at present the back-end has to be psychic as to what is
 really happening.
 
 No, the backend just shouldn't consider TYPE_ALIGN on the scalars, and it
 seems only arm ever looks at that.
 

Nothing in the specification for TARGET_FUNCTION_ARG (or any of the
related functions) makes any mention of this...

R.

   Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Biener 
On May 5, 2015 1:01:59 PM GMT+02:00, Richard Earnshaw 
richard.earns...@foss.arm.com wrote:
On 05/05/15 11:54, Richard Earnshaw wrote:
 On 05/05/15 08:32, Jakub Jelinek wrote:
 On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for non-aggregates
would
 likely not break anything that hasn't been broken already and would
unbreak
 the majority of cases.

 Attached (untested so far).  It indeed changes code generated for
 over-aligned va_arg, but as I believe you can't properly pass those
in the
 ... caller, this should just fix it so that va_arg handling matches
the
 caller (and likewise for callees for named argument passing).

 The following testcase shows that eipa_sra changes alignment even
for the
 aggregates.  Change aligned (8) to aligned (4) to see another
possibility.

 Actually I misread it, for the aggregates esra actually doesn't
change
 anything, which is the reason why the testcase doesn't fail.
 The problem with the scalars is that esra first changed it to the
 over-aligned MEM_REFs and then later on eipa_sra used the types of
the
 MEM_REFs created by esra.

 2015-05-05  Jakub Jelinek  ja...@redhat.com

 PR target/65956
 * config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
 types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type
itself.

 * gcc.c-torture/execute/pr65956.c: New test.

 --- gcc/config/arm/arm.c.jj 2015-05-04 21:51:42.0 +0200
 +++ gcc/config/arm/arm.c2015-05-05 09:20:52.481693337 +0200
 @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 - || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
 +return true;
 
 I don't think this is right (though I suspect the existing code has
the
 same problem).  We should only look at mode if there is no type
 information.  The problem is that GCC has a nasty habit of assigning
 real machine modes to things that are really BLKmode and we've run
into
 several cases where this has royally screwed things up.  So for
 consistency in the ARM back-end we are careful to only use mode when
 type is NULL (= it's a libcall).
 
 +  if (type == NULL_TREE)
 +return false;
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
  }
  
 
 
 
 It ought to be possible to re-order this, though, to
 
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -  || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (type != NULL_TREE)
 +{
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
 +}
 +  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY);
  }
 
 
 Either way, this would need careful cross-testing against an existing
 compiler.
 

It looks as though either patch would cause ABI incompatibility for

typedef int alignedint __attribute__((aligned((8;

int  __attribute__((weak)) foo (int a, alignedint b)
{return b;}

void bar (alignedint x)
{
  foo (1, x);
}

Where currently gcc uses r2 as the argument register for b in foo.

And for foo (1,2) or an int typed 2nd arg?

Richard.

R.

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 13:37, Richard Biener wrote:
 On May 5, 2015 1:01:59 PM GMT+02:00, Richard Earnshaw 
 richard.earns...@foss.arm.com wrote:
 On 05/05/15 11:54, Richard Earnshaw wrote:
 On 05/05/15 08:32, Jakub Jelinek wrote:
 On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for non-aggregates
 would
 likely not break anything that hasn't been broken already and would
 unbreak
 the majority of cases.

 Attached (untested so far).  It indeed changes code generated for
 over-aligned va_arg, but as I believe you can't properly pass those
 in the
 ... caller, this should just fix it so that va_arg handling matches
 the
 caller (and likewise for callees for named argument passing).

 The following testcase shows that eipa_sra changes alignment even
 for the
 aggregates.  Change aligned (8) to aligned (4) to see another
 possibility.

 Actually I misread it, for the aggregates esra actually doesn't
 change
 anything, which is the reason why the testcase doesn't fail.
 The problem with the scalars is that esra first changed it to the
 over-aligned MEM_REFs and then later on eipa_sra used the types of
 the
 MEM_REFs created by esra.

 2015-05-05  Jakub Jelinek  ja...@redhat.com

PR target/65956
* config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type
 itself.

* gcc.c-torture/execute/pr65956.c: New test.

 --- gcc/config/arm/arm.c.jj2015-05-04 21:51:42.0 +0200
 +++ gcc/config/arm/arm.c   2015-05-05 09:20:52.481693337 +0200
 @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -|| (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
 +return true;

 I don't think this is right (though I suspect the existing code has
 the
 same problem).  We should only look at mode if there is no type
 information.  The problem is that GCC has a nasty habit of assigning
 real machine modes to things that are really BLKmode and we've run
 into
 several cases where this has royally screwed things up.  So for
 consistency in the ARM back-end we are careful to only use mode when
 type is NULL (= it's a libcall).

 +  if (type == NULL_TREE)
 +return false;
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
  }
  



 It ought to be possible to re-order this, though, to

  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 - || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (type != NULL_TREE)
 +{
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
 +}
 +  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY);
  }


 Either way, this would need careful cross-testing against an existing
 compiler.


 It looks as though either patch would cause ABI incompatibility for

 typedef int alignedint __attribute__((aligned((8;

 int  __attribute__((weak)) foo (int a, alignedint b)
 {return b;}

 void bar (alignedint x)
 {
  foo (1, x);
 }

 Where currently gcc uses r2 as the argument register for b in foo.
 
 And for foo (1,2) or an int typed 2nd arg?
 

Yep, looks like that's horribly broken too ;-(

R.

 Richard.
 
 R.

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Jakub Jelinek 
On Tue, May 05, 2015 at 09:32:28AM +0200, Jakub Jelinek wrote:
 2015-05-05  Jakub Jelinek  ja...@redhat.com
 
   PR target/65956
   * config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
   types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type itself.
 
   * gcc.c-torture/execute/pr65956.c: New test.

Note, the patch fixed the profiledbootstrap that has been broken in 5+.

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 15:29, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:20:43PM +0100, Richard Earnshaw wrote:
 On 05/05/15 14:06, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:02:19PM +0100, Richard Earnshaw wrote:
 In a literal sense, yes.  However, even KR  stdarg have standard
 promotion and conversion rules (size  int = int, floats promoted to
 double, etc).  What are those rules for GCC's overaligned types (ie
 where in the docs does it say what happens and how a back-end should
 interpret them)?  Shouldn't the mid-end be doing that work so as to

 For the middle-end, the TYPE_ALIGN info on expression types is considered
 useless, you can get there anything.  There is no conversion rule to what
 you get for myalignedint + int, or (myalignedint) int, or (int)
 myalignedint, etc.

 create a consistent view of the values passed into the back-end?  It
 seems to me that at present the back-end has to be psychic as to what is
 really happening.

 No, the backend just shouldn't consider TYPE_ALIGN on the scalars, and it
 seems only arm ever looks at that.


 Nothing in the specification for TARGET_FUNCTION_ARG (or any of the
 related functions) makes any mention of this...
 
 While this requirement isn't documented, it is clearly common sense or at
 least something any kind of testing would reveal immediately.

Then clearly no such tests exist in the testsuite :-(

R.

 And it is nothing broken recently (except that with the SRA changes it hits
 much more often), looking e.g. at GCC 3.2, I'm seeing that expand_call is on
 that testcase also called with pretty random TYPE_ALIGN on the argument
 types; we didn't have GIMPLE then, so it is nothing that GIMPLE brought in.
 
   Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 15:06, Richard Biener wrote:
 On May 5, 2015 2:49:55 PM GMT+02:00, Richard Earnshaw 
 richard.earns...@foss.arm.com wrote:
 On 05/05/15 13:46, Richard Earnshaw wrote:
 On 05/05/15 13:37, Richard Biener wrote:
 On May 5, 2015 1:01:59 PM GMT+02:00, Richard Earnshaw
 richard.earns...@foss.arm.com wrote:
 On 05/05/15 11:54, Richard Earnshaw wrote:
 On 05/05/15 08:32, Jakub Jelinek wrote:
 On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for
 non-aggregates
 would
 likely not break anything that hasn't been broken already and
 would
 unbreak
 the majority of cases.

 Attached (untested so far).  It indeed changes code generated for
 over-aligned va_arg, but as I believe you can't properly pass
 those
 in the
 ... caller, this should just fix it so that va_arg handling
 matches
 the
 caller (and likewise for callees for named argument passing).

 The following testcase shows that eipa_sra changes alignment
 even
 for the
 aggregates.  Change aligned (8) to aligned (4) to see another
 possibility.

 Actually I misread it, for the aggregates esra actually doesn't
 change
 anything, which is the reason why the testcase doesn't fail.
 The problem with the scalars is that esra first changed it to the
 over-aligned MEM_REFs and then later on eipa_sra used the types
 of
 the
 MEM_REFs created by esra.

 2015-05-05  Jakub Jelinek  ja...@redhat.com

 PR target/65956
 * config/arm/arm.c (arm_needs_doubleword_align): For
 non-aggregate
 types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type
 itself.

 * gcc.c-torture/execute/pr65956.c: New test.

 --- gcc/config/arm/arm.c.jj 2015-05-04 21:51:42.0 +0200
 +++ gcc/config/arm/arm.c2015-05-05 09:20:52.481693337 +0200
 @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 - || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
 +return true;

 I don't think this is right (though I suspect the existing code
 has
 the
 same problem).  We should only look at mode if there is no type
 information.  The problem is that GCC has a nasty habit of
 assigning
 real machine modes to things that are really BLKmode and we've run
 into
 several cases where this has royally screwed things up.  So for
 consistency in the ARM back-end we are careful to only use mode
 when
 type is NULL (= it's a libcall).

 +  if (type == NULL_TREE)
 +return false;
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
  }
  



 It ought to be possible to re-order this, though, to

  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -  || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (type != NULL_TREE)
 +{
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) 
 PARM_BOUNDARY;
 +}
 +  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY);
  }


 Either way, this would need careful cross-testing against an
 existing
 compiler.


 It looks as though either patch would cause ABI incompatibility for

 typedef int alignedint __attribute__((aligned((8;

 int  __attribute__((weak)) foo (int a, alignedint b)
 {return b;}

 void bar (alignedint x)
 {
  foo (1, x);
 }

 Where currently gcc uses r2 as the argument register for b in foo.

 And for foo (1,2) or an int typed 2nd arg?

 Richard.

 R.




 As is

 int foo2 (int a, int b);

 void bar2 (alignedint x)
 {
   foo2 (1, x);
 }


 The real question here is why is TYPE the type of the value, rather
 than
 the type of the formal as expressed by the prototype (or implicit
 prototype in the case of variadics or KR)?  Surely this is the mid-end
 passing the wrong information to the back-end.
 
 No - the issue is you are looking at the type of the value at all, instead of 
 at the type of the formal.
 

And I would argue that passing the type of the acutal is stupid in all
cases except when the type of the formal does not exist.  It's just
asking for problems like these.

R.

 Richard.
 
 R.

 ...

 R

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 15:33, Richard Earnshaw wrote:
 On 05/05/15 15:29, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:20:43PM +0100, Richard Earnshaw wrote:
 On 05/05/15 14:06, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:02:19PM +0100, Richard Earnshaw wrote:
 In a literal sense, yes.  However, even KR  stdarg have standard
 promotion and conversion rules (size  int = int, floats promoted to
 double, etc).  What are those rules for GCC's overaligned types (ie
 where in the docs does it say what happens and how a back-end should
 interpret them)?  Shouldn't the mid-end be doing that work so as to

 For the middle-end, the TYPE_ALIGN info on expression types is considered
 useless, you can get there anything.  There is no conversion rule to what
 you get for myalignedint + int, or (myalignedint) int, or (int)
 myalignedint, etc.

 create a consistent view of the values passed into the back-end?  It
 seems to me that at present the back-end has to be psychic as to what is
 really happening.

 No, the backend just shouldn't consider TYPE_ALIGN on the scalars, and it
 seems only arm ever looks at that.


 Nothing in the specification for TARGET_FUNCTION_ARG (or any of the
 related functions) makes any mention of this...

 While this requirement isn't documented, it is clearly common sense or at
 least something any kind of testing would reveal immediately.
 
 Then clearly no such tests exist in the testsuite :-(
 

Or, more precisely, no such tests existed in 2008, when the code went in.

R.

 R.
 
 And it is nothing broken recently (except that with the SRA changes it hits
 much more often), looking e.g. at GCC 3.2, I'm seeing that expand_call is on
 that testcase also called with pretty random TYPE_ALIGN on the argument
 types; we didn't have GIMPLE then, so it is nothing that GIMPLE brought in.

  Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Jakub Jelinek 
On Tue, May 05, 2015 at 02:20:43PM +0100, Richard Earnshaw wrote:
 On 05/05/15 14:06, Jakub Jelinek wrote:
  On Tue, May 05, 2015 at 02:02:19PM +0100, Richard Earnshaw wrote:
  In a literal sense, yes.  However, even KR  stdarg have standard
  promotion and conversion rules (size  int = int, floats promoted to
  double, etc).  What are those rules for GCC's overaligned types (ie
  where in the docs does it say what happens and how a back-end should
  interpret them)?  Shouldn't the mid-end be doing that work so as to
  
  For the middle-end, the TYPE_ALIGN info on expression types is considered
  useless, you can get there anything.  There is no conversion rule to what
  you get for myalignedint + int, or (myalignedint) int, or (int)
  myalignedint, etc.
  
  create a consistent view of the values passed into the back-end?  It
  seems to me that at present the back-end has to be psychic as to what is
  really happening.
  
  No, the backend just shouldn't consider TYPE_ALIGN on the scalars, and it
  seems only arm ever looks at that.
  
 
 Nothing in the specification for TARGET_FUNCTION_ARG (or any of the
 related functions) makes any mention of this...

While this requirement isn't documented, it is clearly common sense or at
least something any kind of testing would reveal immediately.
And it is nothing broken recently (except that with the SRA changes it hits
much more often), looking e.g. at GCC 3.2, I'm seeing that expand_call is on
that testcase also called with pretty random TYPE_ALIGN on the argument
types; we didn't have GIMPLE then, so it is nothing that GIMPLE brought in.

Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Biener 
On May 5, 2015 4:33:58 PM GMT+02:00, Richard Earnshaw 
richard.earns...@foss.arm.com wrote:
On 05/05/15 15:33, Richard Earnshaw wrote:
 On 05/05/15 15:29, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:20:43PM +0100, Richard Earnshaw wrote:
 On 05/05/15 14:06, Jakub Jelinek wrote:
 On Tue, May 05, 2015 at 02:02:19PM +0100, Richard Earnshaw wrote:
 In a literal sense, yes.  However, even KR  stdarg have
standard
 promotion and conversion rules (size  int = int, floats
promoted to
 double, etc).  What are those rules for GCC's overaligned types
(ie
 where in the docs does it say what happens and how a back-end
should
 interpret them)?  Shouldn't the mid-end be doing that work so as
to

 For the middle-end, the TYPE_ALIGN info on expression types is
considered
 useless, you can get there anything.  There is no conversion rule
to what
 you get for myalignedint + int, or (myalignedint) int, or (int)
 myalignedint, etc.

 create a consistent view of the values passed into the back-end? 
It
 seems to me that at present the back-end has to be psychic as to
what is
 really happening.

 No, the backend just shouldn't consider TYPE_ALIGN on the scalars,
and it
 seems only arm ever looks at that.


 Nothing in the specification for TARGET_FUNCTION_ARG (or any of the
 related functions) makes any mention of this...

 While this requirement isn't documented, it is clearly common sense
or at
 least something any kind of testing would reveal immediately.
 
 Then clearly no such tests exist in the testsuite :-(
 

Or, more precisely, no such tests existed in 2008, when the code went
in.

That just means that the code was the first that make this matter.

BTW, what do other compilers do (I suppose llvm supports the gcc extensions for 
alignment)?  Can llvm and gcc interoperate properly here? Do the cited 
testcases work with llvm?

Richard.


R.

 R.
 
 And it is nothing broken recently (except that with the SRA changes
it hits
 much more often), looking e.g. at GCC 3.2, I'm seeing that
expand_call is on
 that testcase also called with pretty random TYPE_ALIGN on the
argument
 types; we didn't have GIMPLE then, so it is nothing that GIMPLE
brought in.

 Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Jakub Jelinek 
On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for non-aggregates would
 likely not break anything that hasn't been broken already and would unbreak
 the majority of cases.

Attached (untested so far).  It indeed changes code generated for
over-aligned va_arg, but as I believe you can't properly pass those in the
... caller, this should just fix it so that va_arg handling matches the
caller (and likewise for callees for named argument passing).

 The following testcase shows that eipa_sra changes alignment even for the
 aggregates.  Change aligned (8) to aligned (4) to see another possibility.

Actually I misread it, for the aggregates esra actually doesn't change
anything, which is the reason why the testcase doesn't fail.
The problem with the scalars is that esra first changed it to the
over-aligned MEM_REFs and then later on eipa_sra used the types of the
MEM_REFs created by esra.

2015-05-05  Jakub Jelinek  ja...@redhat.com

PR target/65956
* config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type itself.

* gcc.c-torture/execute/pr65956.c: New test.

--- gcc/config/arm/arm.c.jj 2015-05-04 21:51:42.0 +0200
+++ gcc/config/arm/arm.c2015-05-05 09:20:52.481693337 +0200
@@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
 static bool
 arm_needs_doubleword_align (machine_mode mode, const_tree type)
 {
-  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
- || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
+  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
+return true;
+  if (type == NULL_TREE)
+return false;
+  if (AGGREGATE_TYPE_P (type))
+return TYPE_ALIGN (type)  PARM_BOUNDARY;
+  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
 }
 
 
--- gcc/testsuite/gcc.c-torture/execute/pr65956.c.jj2015-05-01 
10:32:34.730150257 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr65956.c   2015-05-01 
10:32:13.0 +0200
@@ -0,0 +1,67 @@
+/* PR target/65956 */
+
+struct A { char *a; int b; long long c; };
+char v[3];
+
+__attribute__((noinline, noclone)) void
+fn1 (char *x, char *y)
+{
+  if (x != v[1] || y != v[2])
+__builtin_abort ();
+  v[1]++;
+}
+
+__attribute__((noinline, noclone)) int
+fn2 (char *x)
+{
+  asm volatile ( : +g (x) : : memory);
+  return x == v[0];
+}
+
+__attribute__((noinline, noclone)) void
+fn3 (const char *x)
+{
+  if (x[0] != 0)
+__builtin_abort ();
+}
+
+static struct A
+foo (const char *x, struct A y, struct A z)
+{
+  struct A r = { 0, 0, 0 };
+  if (y.b  z.b)
+{
+  if (fn2 (y.a)  fn2 (z.a))
+   switch (x[0])
+ {
+ case '|':
+   break;
+ default:
+   fn3 (x);
+ }
+  fn1 (y.a, z.a);
+}
+  return r;
+}
+
+__attribute__((noinline, noclone)) int
+bar (int x, struct A *y)
+{
+  switch (x)
+{
+case 219:
+  foo (+, y[-2], y[0]);
+case 220:
+  foo (-, y[-2], y[0]);
+}
+}
+
+int
+main ()
+{
+  struct A a[3] = { { v[1], 1, 1LL }, { v[0], 0, 0LL }, { v[2], 2, 2LL } };
+  bar (220, a + 2);
+  if (v[1] != 1)
+__builtin_abort ();
+  return 0;
+}


Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 08:32, Jakub Jelinek wrote:
 On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for non-aggregates would
 likely not break anything that hasn't been broken already and would unbreak
 the majority of cases.
 
 Attached (untested so far).  It indeed changes code generated for
 over-aligned va_arg, but as I believe you can't properly pass those in the
 ... caller, this should just fix it so that va_arg handling matches the
 caller (and likewise for callees for named argument passing).
 
 The following testcase shows that eipa_sra changes alignment even for the
 aggregates.  Change aligned (8) to aligned (4) to see another possibility.
 
 Actually I misread it, for the aggregates esra actually doesn't change
 anything, which is the reason why the testcase doesn't fail.
 The problem with the scalars is that esra first changed it to the
 over-aligned MEM_REFs and then later on eipa_sra used the types of the
 MEM_REFs created by esra.
 
 2015-05-05  Jakub Jelinek  ja...@redhat.com
 
   PR target/65956
   * config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
   types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type itself.
 
   * gcc.c-torture/execute/pr65956.c: New test.
 
 --- gcc/config/arm/arm.c.jj   2015-05-04 21:51:42.0 +0200
 +++ gcc/config/arm/arm.c  2015-05-05 09:20:52.481693337 +0200
 @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -   || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
 +return true;

I don't think this is right (though I suspect the existing code has the
same problem).  We should only look at mode if there is no type
information.  The problem is that GCC has a nasty habit of assigning
real machine modes to things that are really BLKmode and we've run into
several cases where this has royally screwed things up.  So for
consistency in the ARM back-end we are careful to only use mode when
type is NULL (= it's a libcall).

 +  if (type == NULL_TREE)
 +return false;
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
  }
  



It ought to be possible to re-order this, though, to

 static bool
 arm_needs_doubleword_align (machine_mode mode, const_tree type)
 {
-  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
- || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
+  if (type != NULL_TREE)
+{
+  if (AGGREGATE_TYPE_P (type))
+return TYPE_ALIGN (type)  PARM_BOUNDARY;
+  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
+}
+  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY);
 }


Either way, this would need careful cross-testing against an existing
compiler.

R.

  
 --- gcc/testsuite/gcc.c-torture/execute/pr65956.c.jj  2015-05-01 
 10:32:34.730150257 +0200
 +++ gcc/testsuite/gcc.c-torture/execute/pr65956.c 2015-05-01 
 10:32:13.0 +0200
 @@ -0,0 +1,67 @@
 +/* PR target/65956 */
 +
 +struct A { char *a; int b; long long c; };
 +char v[3];
 +
 +__attribute__((noinline, noclone)) void
 +fn1 (char *x, char *y)
 +{
 +  if (x != v[1] || y != v[2])
 +__builtin_abort ();
 +  v[1]++;
 +}
 +
 +__attribute__((noinline, noclone)) int
 +fn2 (char *x)
 +{
 +  asm volatile ( : +g (x) : : memory);
 +  return x == v[0];
 +}
 +
 +__attribute__((noinline, noclone)) void
 +fn3 (const char *x)
 +{
 +  if (x[0] != 0)
 +__builtin_abort ();
 +}
 +
 +static struct A
 +foo (const char *x, struct A y, struct A z)
 +{
 +  struct A r = { 0, 0, 0 };
 +  if (y.b  z.b)
 +{
 +  if (fn2 (y.a)  fn2 (z.a))
 + switch (x[0])
 +   {
 +   case '|':
 + break;
 +   default:
 + fn3 (x);
 +   }
 +  fn1 (y.a, z.a);
 +}
 +  return r;
 +}
 +
 +__attribute__((noinline, noclone)) int
 +bar (int x, struct A *y)
 +{
 +  switch (x)
 +{
 +case 219:
 +  foo (+, y[-2], y[0]);
 +case 220:
 +  foo (-, y[-2], y[0]);
 +}
 +}
 +
 +int
 +main ()
 +{
 +  struct A a[3] = { { v[1], 1, 1LL }, { v[0], 0, 0LL }, { v[2], 2, 2LL } 
 };
 +  bar (220, a + 2);
 +  if (v[1] != 1)
 +__builtin_abort ();
 +  return 0;
 +}
 
 
   Jakub

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-05 Thread Richard Earnshaw 
On 05/05/15 11:54, Richard Earnshaw wrote:
 On 05/05/15 08:32, Jakub Jelinek wrote:
 On Mon, May 04, 2015 at 05:00:11PM +0200, Jakub Jelinek wrote:
 So at least changing arm_needs_doubleword_align for non-aggregates would
 likely not break anything that hasn't been broken already and would unbreak
 the majority of cases.

 Attached (untested so far).  It indeed changes code generated for
 over-aligned va_arg, but as I believe you can't properly pass those in the
 ... caller, this should just fix it so that va_arg handling matches the
 caller (and likewise for callees for named argument passing).

 The following testcase shows that eipa_sra changes alignment even for the
 aggregates.  Change aligned (8) to aligned (4) to see another possibility.

 Actually I misread it, for the aggregates esra actually doesn't change
 anything, which is the reason why the testcase doesn't fail.
 The problem with the scalars is that esra first changed it to the
 over-aligned MEM_REFs and then later on eipa_sra used the types of the
 MEM_REFs created by esra.

 2015-05-05  Jakub Jelinek  ja...@redhat.com

  PR target/65956
  * config/arm/arm.c (arm_needs_doubleword_align): For non-aggregate
  types check TYPE_ALIGN of TYPE_MAIN_VARIANT rather than type itself.

  * gcc.c-torture/execute/pr65956.c: New test.

 --- gcc/config/arm/arm.c.jj  2015-05-04 21:51:42.0 +0200
 +++ gcc/config/arm/arm.c 2015-05-05 09:20:52.481693337 +0200
 @@ -6063,8 +6063,13 @@ arm_init_cumulative_args (CUMULATIVE_ARG
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -  || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY)
 +return true;
 
 I don't think this is right (though I suspect the existing code has the
 same problem).  We should only look at mode if there is no type
 information.  The problem is that GCC has a nasty habit of assigning
 real machine modes to things that are really BLKmode and we've run into
 several cases where this has royally screwed things up.  So for
 consistency in the ARM back-end we are careful to only use mode when
 type is NULL (= it's a libcall).
 
 +  if (type == NULL_TREE)
 +return false;
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
  }
  
 
 
 
 It ought to be possible to re-order this, though, to
 
  static bool
  arm_needs_doubleword_align (machine_mode mode, const_tree type)
  {
 -  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
 -   || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
 +  if (type != NULL_TREE)
 +{
 +  if (AGGREGATE_TYPE_P (type))
 +return TYPE_ALIGN (type)  PARM_BOUNDARY;
 +  return TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY;
 +}
 +  return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY);
  }
 
 
 Either way, this would need careful cross-testing against an existing
 compiler.
 

It looks as though either patch would cause ABI incompatibility for

typedef int alignedint __attribute__((aligned((8;

int  __attribute__((weak)) foo (int a, alignedint b)
{return b;}

void bar (alignedint x)
{
  foo (1, x);
}

Where currently gcc uses r2 as the argument register for b in foo.

R.

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-04 Thread Richard Biener 
On Sat, 2 May 2015, Jakub Jelinek wrote:

 Hi!
 
 This is an attempt to fix the following testcase (reduced from gsoap)
 similarly how you've fixed another issue with r221795 other AAPCS
 regressions introduced with r221348 change.
 This patch passed bootstrap/regtest on
 {x86_64,i686,armv7hl,aarch64,powerpc64{,le},s390{,x}}-linux.
 
 Though, it still doesn't fix profiledbootstrap on armv7hl that is broken
 since r221348, so other issues are lurking in there, and I must say
 I'm not entirely sure about this, because it changes alignment even when
 the original access had higher alignment.
 
 I was trying something like:
 struct B { char *a, *b; };
 typedef struct B C __attribute__((aligned (8)));
 struct A { C a; int b; long long c; };
 char v[3];
 
 __attribute__((noinline, noclone)) void
 fn1 (C x, C y)
 {
   if (x.a != v[1] || y.a != v[2])
 __builtin_abort ();
   v[1]++;
 }
 
 __attribute__((noinline, noclone)) int
 fn2 (C x)
 {
   asm volatile ( : +g (x.a) : : memory);
   asm volatile ( : +g (x.b) : : memory);
   return x.a == v[0];
 }
 
 __attribute__((noinline, noclone)) void
 fn3 (const char *x)
 {
   if (x[0] != 0)
 __builtin_abort ();
 }
 
 static struct A
 foo (const char *x, struct A y, struct A z)
 {
   struct A r = { { 0, 0 }, 0, 0 };
   if (y.b  z.b)
 {
   if (fn2 (y.a)  fn2 (z.a))
   switch (x[0])
 {
 case '|':
   break;
 default:
   fn3 (x);
 }
   fn1 (y.a, z.a);
 }
   return r;
 }
 
 __attribute__((noinline, noclone)) int
 bar (int x, struct A *y)
 {
   switch (x)
 {
 case 219:
   foo (+, y[-2], y[0]);
 case 220:
   foo (-, y[-2], y[0]);
 }
 }
 
 int
 main ()
 {
   struct A a[3] = { { { v[1], v[0] }, 1, 1LL },
   { { v[0], v[0] }, 0, 0LL },
   { { v[2], v[0] }, 2, 2LL } };
   bar (220, a + 2);
   if (v[1] != 1)
 __builtin_abort ();
   return 0;
 }
 
 and this patch indeed changes the register passing, eventhough it probably
 shouldn't (though, the testcase doesn't fail).  Wouldn't it be possible to
 preserve the original type (before we call build_aligned_type on it)
 somewhere in SRA data structures, perhaps keep expr (the new MEM_REF) use
 the aligned type, but type field be the non-aligned one?

Not sure how this helps when SRA tears apart the parameter.  That is,
isn't the important thing that both the IPA modified function argument
types/decls have the same type as the types of the parameters SRA ends
up passing?  (as far as alignment goes?)

Yes, of course using natural alignment makes sure that the backend
can handle alignment properly and we don't run into oddball bugs here.

 2015-05-02  Jakub Jelinek  ja...@redhat.com
 
   PR target/65956
   * tree-sra.c (turn_representatives_into_adjustments): For
   adj.type, use TYPE_MAIN_VARIANT of repr-type with TYPE_QUALS.
 
   * gcc.c-torture/execute/pr65956.c: New test.
 
 --- gcc/tree-sra.c.jj 2015-04-20 14:35:47.0 +0200
 +++ gcc/tree-sra.c2015-05-01 01:08:34.092636496 +0200
 @@ -4427,7 +4427,11 @@ turn_representatives_into_adjustments (v
 gcc_assert (repr-base == parm);
 adj.base_index = index;
 adj.base = repr-base;
 -   adj.type = repr-type;
 +   /* Drop any special alignment on the type if it's not on the
 +  main variant.  This avoids issues with weirdo ABIs like
 +  AAPCS.  */
 +   adj.type = build_qualified_type (TYPE_MAIN_VARIANT (repr-type),
 +TYPE_QUALS (repr-type));

So - this changes the function argument type of the clone?  Does it
also change the type of the value we pass to the function?  That is,
why drop the alignment here but not avoid attaching it to repr-type
in the first place as my fix for the other issue did?

Doesn't the above just make it inconsistent by default?

There is also the correctness issue of under-aligned types (which
was what the original code using build_aligned_type cared for - before
I fixed it to also preserve over-alignment).

That said - somewhere we create the register we use for passing the
argument, and only the type of that register needs fixing IMHO.

We also have

  ptype = adj-type;
  if (is_gimple_reg_type (ptype))
{
  unsigned malign = GET_MODE_ALIGNMENT (TYPE_MODE 
(ptype));
  if (TYPE_ALIGN (ptype)  malign)
ptype = build_aligned_type (ptype, malign);

in ipa_modify_formal_parameters.  That looks odd for by-value passing
as well.  When modifying the function bodies we simply take what was
set in -new_decl which we'd populate above in 
ipa_modify_formal_parameters.  It seems to me that ipa_modify_expr
should look to preserve alignment at the callers site (for loading
into the regs we pass) for non-reference passing.  Esp.

  if (cand-by_ref)
src = build_simple_mem_ref (cand-new_decl);

looks bogus in this

Re: [PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-04 Thread Jakub Jelinek 
On Mon, May 04, 2015 at 10:11:13AM +0200, Richard Biener wrote:
 Not sure how this helps when SRA tears apart the parameter.  That is,
 isn't the important thing that both the IPA modified function argument
 types/decls have the same type as the types of the parameters SRA ends
 up passing?  (as far as alignment goes?)
 
 Yes, of course using natural alignment makes sure that the backend
 can handle alignment properly and we don't run into oddball bugs here.

On IRC we were discussing making

 /* Return true if mode/type need doubleword alignment.  */
 static bool
 arm_needs_doubleword_align (machine_mode mode, const_tree type)
 {
   return (GET_MODE_ALIGNMENT (mode)  PARM_BOUNDARY
- || (type  TYPE_ALIGN (type)  PARM_BOUNDARY));
+ || (type  TYPE_ALIGN (TYPE_MAIN_VARIANT (type))  PARM_BOUNDARY));
 }


Looking at

struct S { char a[16]; }; 
typedef struct S T;
typedef struct S U __attribute__((aligned (16))); 
struct V { U u; T v; };
typedef int N __attribute__((aligned (16)));

T t1;
U u1;
int a[3];

void
f5 (__builtin_va_list *ap)
{
  t1 = __builtin_va_arg (*ap, T);
  a[0] = __builtin_va_arg (*ap, int);
  u1 = __builtin_va_arg (*ap, U);
  a[1] = __builtin_va_arg (*ap, int);
  a[2] = __builtin_va_arg (*ap, N);
}

void f6 (int, N, int, U);

void
f7 (void)
{
  U u = {};
  f6 (0, (N) 1, 0, u);
}

and s/16/8/g output, it seems that neither i?86 nor x86_64 care about
the alignment for any passing, ppc64le cares about aggregates, but not
scalars apparently (with a warning that the passing changed), arm cares
about both.  And the f7 function shows that for non-aggregates, what arm
does is simply never going to work, because there is no way to pass down
the scalars aligned, f6 is still called with 1 in int type rather than N.

So at least changing arm_needs_doubleword_align for non-aggregates would
likely not break anything that hasn't been broken already and would unbreak
the majority of cases.

The following testcase shows that eipa_sra changes alignment even for the
aggregates.  Change aligned (8) to aligned (4) to see another possibility.

/* PR target/65956 */

struct B { char *a, *b; };
typedef struct B C __attribute__((aligned (8)));
struct A { C a; int b; long long c; };
char v[3];

__attribute__((noinline, noclone)) void
fn1 (int v, ...)
{
  __builtin_va_list ap;
  __builtin_va_start (ap, v);
  C c, d;
  c = __builtin_va_arg (ap, C);
  __builtin_va_arg (ap, int);
  d = __builtin_va_arg (ap, C);
  __builtin_va_end (ap);
  if (c.a != v[1] || d.a != v[2])
__builtin_abort ();
  v[1]++;
}

__attribute__((noinline, noclone)) int
fn2 (C x)
{
  asm volatile ( : +g (x.a) : : memory);
  asm volatile ( : +g (x.b) : : memory);
  return x.a == v[0];
}

__attribute__((noinline, noclone)) void
fn3 (const char *x)
{
  if (x[0] != 0)
__builtin_abort ();
}

static struct A
foo (const char *x, struct A y, struct A z)
{
  struct A r = { { 0, 0 }, 0, 0 };
  if (y.b  z.b)
{
  if (fn2 (y.a)  fn2 (z.a))
switch (x[0])
  {
  case '|':
break;
  default:
fn3 (x);
  }
  fn1 (0, y.a, 0, z.a);
}
  return r;
}

__attribute__((noinline, noclone)) int
bar (int x, struct A *y)
{
  switch (x)
{
case 219:
  foo (+, y[-2], y[0]);
case 220:
  foo (-, y[-2], y[0]);
}
}

int
main ()
{
  struct A a[3] = { { { v[1], v[0] }, 1, 1LL },
{ { v[0], v[0] }, 0, 0LL },
{ { v[2], v[0] }, 2, 2LL } };
  bar (220, a + 2);
  if (v[1] != 1)
__builtin_abort ();
  return 0;
}

Jakub

[PATCH] Fix eipa_sra AAPCS issue (PR target/65956)

2015-05-02 Thread Jakub Jelinek 
Hi!

This is an attempt to fix the following testcase (reduced from gsoap)
similarly how you've fixed another issue with r221795 other AAPCS
regressions introduced with r221348 change.
This patch passed bootstrap/regtest on
{x86_64,i686,armv7hl,aarch64,powerpc64{,le},s390{,x}}-linux.

Though, it still doesn't fix profiledbootstrap on armv7hl that is broken
since r221348, so other issues are lurking in there, and I must say
I'm not entirely sure about this, because it changes alignment even when
the original access had higher alignment.

I was trying something like:
struct B { char *a, *b; };
typedef struct B C __attribute__((aligned (8)));
struct A { C a; int b; long long c; };
char v[3];

__attribute__((noinline, noclone)) void
fn1 (C x, C y)
{
  if (x.a != v[1] || y.a != v[2])
__builtin_abort ();
  v[1]++;
}

__attribute__((noinline, noclone)) int
fn2 (C x)
{
  asm volatile ( : +g (x.a) : : memory);
  asm volatile ( : +g (x.b) : : memory);
  return x.a == v[0];
}

__attribute__((noinline, noclone)) void
fn3 (const char *x)
{
  if (x[0] != 0)
__builtin_abort ();
}

static struct A
foo (const char *x, struct A y, struct A z)
{
  struct A r = { { 0, 0 }, 0, 0 };
  if (y.b  z.b)
{
  if (fn2 (y.a)  fn2 (z.a))
switch (x[0])
  {
  case '|':
break;
  default:
fn3 (x);
  }
  fn1 (y.a, z.a);
}
  return r;
}

__attribute__((noinline, noclone)) int
bar (int x, struct A *y)
{
  switch (x)
{
case 219:
  foo (+, y[-2], y[0]);
case 220:
  foo (-, y[-2], y[0]);
}
}

int
main ()
{
  struct A a[3] = { { { v[1], v[0] }, 1, 1LL },
{ { v[0], v[0] }, 0, 0LL },
{ { v[2], v[0] }, 2, 2LL } };
  bar (220, a + 2);
  if (v[1] != 1)
__builtin_abort ();
  return 0;
}

and this patch indeed changes the register passing, eventhough it probably
shouldn't (though, the testcase doesn't fail).  Wouldn't it be possible to
preserve the original type (before we call build_aligned_type on it)
somewhere in SRA data structures, perhaps keep expr (the new MEM_REF) use
the aligned type, but type field be the non-aligned one?

2015-05-02  Jakub Jelinek  ja...@redhat.com

PR target/65956
* tree-sra.c (turn_representatives_into_adjustments): For
adj.type, use TYPE_MAIN_VARIANT of repr-type with TYPE_QUALS.

* gcc.c-torture/execute/pr65956.c: New test.

--- gcc/tree-sra.c.jj   2015-04-20 14:35:47.0 +0200
+++ gcc/tree-sra.c  2015-05-01 01:08:34.092636496 +0200
@@ -4427,7 +4427,11 @@ turn_representatives_into_adjustments (v
  gcc_assert (repr-base == parm);
  adj.base_index = index;
  adj.base = repr-base;
- adj.type = repr-type;
+ /* Drop any special alignment on the type if it's not on the
+main variant.  This avoids issues with weirdo ABIs like
+AAPCS.  */
+ adj.type = build_qualified_type (TYPE_MAIN_VARIANT (repr-type),
+  TYPE_QUALS (repr-type));
  adj.alias_ptr_type = reference_alias_ptr_type (repr-expr);
  adj.offset = repr-offset;
  adj.by_ref = (POINTER_TYPE_P (TREE_TYPE (repr-base))
--- gcc/testsuite/gcc.c-torture/execute/pr65956.c.jj2015-05-01 
10:32:34.730150257 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr65956.c   2015-05-01 
10:32:13.0 +0200
@@ -0,0 +1,67 @@
+/* PR target/65956 */
+
+struct A { char *a; int b; long long c; };
+char v[3];
+
+__attribute__((noinline, noclone)) void
+fn1 (char *x, char *y)
+{
+  if (x != v[1] || y != v[2])
+__builtin_abort ();
+  v[1]++;
+}
+
+__attribute__((noinline, noclone)) int
+fn2 (char *x)
+{
+  asm volatile ( : +g (x) : : memory);
+  return x == v[0];
+}
+
+__attribute__((noinline, noclone)) void
+fn3 (const char *x)
+{
+  if (x[0] != 0)
+__builtin_abort ();
+}
+
+static struct A
+foo (const char *x, struct A y, struct A z)
+{
+  struct A r = { 0, 0, 0 };
+  if (y.b  z.b)
+{
+  if (fn2 (y.a)  fn2 (z.a))
+   switch (x[0])
+ {
+ case '|':
+   break;
+ default:
+   fn3 (x);
+ }
+  fn1 (y.a, z.a);
+}
+  return r;
+}
+
+__attribute__((noinline, noclone)) int
+bar (int x, struct A *y)
+{
+  switch (x)
+{
+case 219:
+  foo (+, y[-2], y[0]);
+case 220:
+  foo (-, y[-2], y[0]);
+}
+}
+
+int
+main ()
+{
+  struct A a[3] = { { v[1], 1, 1LL }, { v[0], 0, 0LL }, { v[2], 2, 2LL } };
+  bar (220, a + 2);
+  if (v[1] != 1)
+__builtin_abort ();
+  return 0;
+}

Jakub