On Tue, Jan 7, 2014 at 5:31 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hello,
Ping...
We still need a decision how to fix this.
There are two alternative patches:
1. My latest proposal: http://gcc.gnu.org/ml/gcc-patches/2013-12/msg01675.html
2. Eric's latest proposal:
Hello,
Ping...
We still need a decision how to fix this.
There are two alternative patches:
1. My latest proposal: http://gcc.gnu.org/ml/gcc-patches/2013-12/msg01675.html
2. Eric's latest proposal:
http://gcc.gnu.org/ml/gcc-patches/2013-12/msg01667.html
Thanks
Bernd.
Hi,
On Mon, 16 Dec 2013 17:22:59, Eric Botcazou wrote:
That sounds less conservative though. Anyway, that was just some thought
to fix the eventual fallout of making more types have BLKmode.
In the end I was too optimistic... Testing a revised patch on x86 revealed a
obscure case where the
In general I like the comment, and I am open for other suggestions how
to call the parameter.
I think that using EXPAND in the parameter's name is confusing because it
needs to be distinguished from MODIFIER and its enumeration type. And since
it would be true only after calling
Hi,
On Thu, 19 Dec 2013 11:55:03, Eric Botcazou wrote:
In general I like the comment, and I am open for other suggestions how
to call the parameter.
I think that using EXPAND in the parameter's name is confusing because it
needs to be distinguished from MODIFIER and its enumeration type.
On Mon, 16 Dec 2013, Eric Botcazou wrote:
which of course blatantly violates the do-not-rely-on-mode rule. Although
the
layout change apparently occurs very rarely, I think that this rules out the
direct mode change in stor-layout.c... snip
Well - makes such a change unsuitable for 4.9.
That sounds less conservative though. Anyway, that was just some thought
to fix the eventual fallout of making more types have BLKmode.
In the end I was too optimistic... Testing a revised patch on x86 revealed a
obscure case where the new BLKmode triggered a layout change (I presume that
Does this catch C99 VLAs? I vaguely recall we have a different internal
representation of those?!? And don't those have the same problem?
No, VLAs have explicit variable size so they are not problematic here.
--
Eric Botcazou
Hi,
with Eric's patch this test case does no longer compile, while it did compile
with my patch:
$ cat test.c
struct s{ char x[8]; };
int main()
{
volatile register struct s x __asm__(eax);
x.x[0] = 1;
return x.x[0];
}
$ gcc -O3 -S test.c
test.c: In function 'main':
test.c:4:30: error:
with Eric's patch this test case does no longer compile, while it did
compile with my patch:
$ cat test.c
struct s{ char x[8]; };
int main()
{
volatile register struct s x __asm__(eax);
x.x[0] = 1;
return x.x[0];
}
$ gcc -O3 -S test.c
test.c: In function 'main':
On Wed, Dec 11, 2013 at 8:19 PM, Eric Botcazou ebotca...@adacore.com wrote:
Yes we do, even for struct { struct { int a; char a[1] } }; (note the not
really trailing as there is padding after the trailing array). We do
take size limitations from a DECL (if we see one) into account to limit the
Instead I'd suggest to keep a 'last_field_array_p' flag that you can
check at the end of the loop.
OK, I can do that.
+ TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE
+ !integer_zerop (TYPE_SIZE (TREE_TYPE (TREE_TYPE (field)
+return;
+
Why does this exclude zero-sized
On Fri, Dec 13, 2013 at 12:08 PM, Eric Botcazou ebotca...@adacore.com wrote:
Instead I'd suggest to keep a 'last_field_array_p' flag that you can
check at the end of the loop.
OK, I can do that.
+ TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE
+ !integer_zerop (TYPE_SIZE (TREE_TYPE
OK, so we want the attached patch? FWIW it passed
make -k check-c check-c++ RUNTESTFLAGS=compat.exp struct-layout-1.exp
on x86/Linux, x86-64/Linux, PowerPC/Linux [*], IA-64/Linux, SPARC/Solaris
and SPARC64/Solaris with ALT_CC_UNDER_TEST set to the unpatched compiler.
As well as on
On 12/11/13 12:19, Eric Botcazou wrote:
Yes we do, even for struct { struct { int a; char a[1] } }; (note the not
really trailing as there is padding after the trailing array). We do
take size limitations from a DECL (if we see one) into account to limit the
effect of this
Hi,
On Tue, 10 Dec 2013 16:14:43, Richard Biener wrote:
On Tue, Dec 10, 2013 at 4:02 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou ebotca...@adacore.com
wrote:
What we support is out of bounds accesses for heap vars if the var's type
Yes we do, even for struct { struct { int a; char a[1] } }; (note the not
really trailing as there is padding after the trailing array). We do
take size limitations from a DECL (if we see one) into account to limit the
effect of this trailing-array-supporting, so it effectively only applies
What are the transformations that are enabled by making something not
BLKmode?
On the gimple level I cannot think of one..
On the RTL level, it's simple: anything BLKmode is forced to memory instead of
being loaded into registers.
This could work as well, although I'd restrict this to
On Tue, Dec 10, 2013 at 11:04:53AM +0100, Eric Botcazou wrote:
What are the transformations that are enabled by making something not
BLKmode?
On the gimple level I cannot think of one..
On the RTL level, it's simple: anything BLKmode is forced to memory instead
of
being loaded into
What we support is out of bounds accesses for heap vars if the var's type
has flexible array member or something we treat similarly and there is the
possibility that there could be payload after the heap var that could be
accessed from the flexible array members or similar arrays.
My
On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou ebotca...@adacore.com wrote:
What we support is out of bounds accesses for heap vars if the var's type
has flexible array member or something we treat similarly and there is the
possibility that there could be payload after the heap var that could
On Tue, Dec 10, 2013 at 4:02 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou ebotca...@adacore.com wrote:
What we support is out of bounds accesses for heap vars if the var's type
has flexible array member or something we treat similarly and
On 12/07/13 03:44, Eric Botcazou wrote:
I'd certainly be concerned. Ports have (for better or worse) keyed on
BLKmode rather than looking at the underlying types. So if something
which was previously SImode or DImode is now BLKmode, there's a nonzero
chance we're going to change how it gets
On Mon, Dec 9, 2013 at 10:07 AM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
On 12/07/13 03:44, Eric Botcazou wrote:
I'd certainly be concerned. Ports have (for better or worse) keyed on
BLKmode rather than looking at the underlying types. So if something
which was previously SImode or
Eric Botcazou ebotca...@adacore.com wrote:
It's not fully fixing the issue as _all_ aggregates that may be
accessed beyond their declarations size are broken.
Sure, but we don't need to support such nonsense in the general case.
And not
every language allows it, for example in Ada you cannot
Eric Botcazou ebotca...@adacore.com wrote:
That being said, the concern is certainly valid so we may want to go
for a
kludge instead of the fix. The point is that the kludge should do
exactly
what the fix would have done in the RTL expander and nothing more;
it's out
of question to pessimize
On 12/07/13 03:44, Eric Botcazou wrote:
I'd certainly be concerned. Ports have (for better or worse) keyed on
BLKmode rather than looking at the underlying types. So if something
which was previously SImode or DImode is now BLKmode, there's a nonzero
chance we're going to change how it gets
It's not fully fixing the issue as _all_ aggregates that may be
accessed beyond their declarations size are broken.
Sure, but we don't need to support such nonsense in the general case. And not
every language allows it, for example in Ada you cannot do that of course.
I'd say we should
I'd certainly be concerned. Ports have (for better or worse) keyed on
BLKmode rather than looking at the underlying types. So if something
which was previously SImode or DImode is now BLKmode, there's a nonzero
chance we're going to change how it gets passed.
Well, we have been saying that
That being said, the concern is certainly valid so we may want to go for a
kludge instead of the fix. The point is that the kludge should do exactly
what the fix would have done in the RTL expander and nothing more; it's out
of question to pessimize all the other languages and all the other
Hi,
On Thu, 5 Dec 2013 22:16:15, Jeff Law wrote:
On 12/04/13 01:16, Bernd Edlinger wrote:
Looking for some more time your patch may be indeed the easiest
without big re-factoring.
Richard (or Bernd), can you comment on why? Something seems off here.
Why do we need to handle inner
one test case is this one:
pr57748-3.c:
/* PR middle-end/57748 */
/* { dg-do run } */
/* wrong code in expand_expr_real_1. */
#include stdlib.h
extern void abort (void);
typedef long long V
__attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
typedef struct S {
Here's the Correct Fix(tm). We may or may not decide to go for it because
of concerns about ABI changes; in the latter case, any kludge that we'll
put in place instead must be restricted to the cases caught by this patch.
* stor-layout.c (compute_record_mode): Return BLKmode for a
On Fri, Dec 6, 2013 at 10:11 AM, Eric Botcazou ebotca...@adacore.com wrote:
Here's the Correct Fix(tm). We may or may not decide to go for it because
of concerns about ABI changes; in the latter case, any kludge that we'll
put in place instead must be restricted to the cases caught by this
On Fri, Dec 6, 2013 at 6:16 AM, Jeff Law l...@redhat.com wrote:
On 12/04/13 01:16, Bernd Edlinger wrote:
Looking for some more time your patch may be indeed the easiest
without big re-factoring.
Richard (or Bernd), can you comment on why? Something seems off here.
Why do we need to
On 12/06/13 03:06, Richard Biener wrote:
The issue is that we handle expansion of misaligned moves in the code
we recurse to - but that misaligned move handling can only work at
the outermost level of the recursion as it is supposed to see the
real access (alignment and mode of the memory
On 12/06/13 01:51, Bernd Edlinger wrote:
As for the patch itself. In a few places within expand_expr_real_1 you
changed calls to expand_expr to instead call expand_expr_real. ISTM
you could have gotten the same effect by just adding your extra argument
to the existing code?
Yes, but one goal
On 12/06/13 02:11, Eric Botcazou wrote:
Here's the Correct Fix(tm). We may or may not decide to go for it because
of concerns about ABI changes; in the latter case, any kludge that we'll
put in place instead must be restricted to the cases caught by this patch.
* stor-layout.c
On 12/04/13 01:16, Bernd Edlinger wrote:
Looking for some more time your patch may be indeed the easiest
without big re-factoring.
Richard (or Bernd), can you comment on why? Something seems off here.
Why do we need to handle inner references here specially? If feels
like we're catering
On Tue, 3 Dec 2013 15:12:05, Richard Biener wrote:
On Tue, Dec 3, 2013 at 2:10 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hi Jeff,
please find attached the patch (incl. test cases) for the unaligned
Hi Jeff,
please find attached the patch (incl. test cases) for the unaligned read BUG
that I found while investigating
on PR#57748: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57748
one test case is this one:
pr57748-3.c:
/* PR middle-end/57748 */
/* { dg-do run } */
/* wrong code in
On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hi Jeff,
please find attached the patch (incl. test cases) for the unaligned read BUG
that I found while investigating
on PR#57748: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57748
one test case is this one:
On Tue, Dec 3, 2013 at 2:10 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hi Jeff,
please find attached the patch (incl. test cases) for the unaligned read BUG
that I found while investigating
on
Date: Tue, 3 Dec 2013 14:51:21 +0100
Subject: Re: [REPOST] Invalid Code when reading from unaligned zero-sized
array
From: richard.guent...@gmail.com
To: bernd.edlin...@hotmail.de
CC: l...@redhat.com; gcc-patches@gcc.gnu.org; ja...@redhat.com
On Tue, Dec 3, 2013 at 2:10 PM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hi Jeff,
please find attached the patch (incl. test cases) for the unaligned read BUG
that I found while investigating
on
The patch does add a boolean expand_reference parameter to
expand_expr_real and expand_expr_real_1. I pass true when I intend to use
the returned memory context as an array reference, instead of a value. At
places where mis-aligned values are extracted, I do not return a register
with the
On Tue, 3 Dec 2013 16:14:49, Eric Botcazou wrote:
The patch does add a boolean expand_reference parameter to
expand_expr_real and expand_expr_real_1. I pass true when I intend to use
the returned memory context as an array reference, instead of a value. At
places where mis-aligned values are
On 12/03/13 11:40, Bernd Edlinger wrote:
To me it does not feel like a hack at all, if the expansion needs to know in
what
context the result will be used, if it is in an LHS or in a RHS or if the
address of
the memory is needed.
Expansion needs some context and it's currently passed into
On Tue, 3 Dec 2013 12:16:39, Jeff Law wrote:
On 12/03/13 11:40, Bernd Edlinger wrote:
To me it does not feel like a hack at all, if the expansion needs to know in
what
context the result will be used, if it is in an LHS or in a RHS or if the
address of
the memory is needed.
Expansion
49 matches
Mail list logo
