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
> 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 get_inner_r
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 wh
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...
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
On Fri, Dec 13, 2013 at 12:08 PM, Eric Botcazou 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 (TREE_
> 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 z
On Wed, Dec 11, 2013 at 8:19 PM, 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
> 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
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
> 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
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 trailing-array-suppo
> 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
> 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 appl
Hi,
On Tue, 10 Dec 2013 16:14:43, Richard Biener wrote:
>
> On Tue, Dec 10, 2013 at 4:02 PM, Richard Biener
> wrote:
>> On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou
>> wrote:
What we support is out of bounds accesses for heap vars if the var's type
has flexible array member or some
On Tue, Dec 10, 2013 at 4:02 PM, Richard Biener
wrote:
> On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou 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 c
On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou 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 be
>> accessed
> 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 ques
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
> 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 t
On Mon, Dec 9, 2013 at 10:07 AM, Bernd Edlinger
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 DImode is now B
> 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 h
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 pas
Eric Botcazou 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 all the oth
Eric Botcazou 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 do that of
>co
> 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 othe
> 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 th
> 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 si
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 (compute
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 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 ac
On Fri, Dec 6, 2013 at 6:16 AM, 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 handl
On Fri, Dec 6, 2013 at 10:11 AM, 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.
>>
>>
>>
> 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 fo
> one test case is this one:
>
> pr57748-3.c:
> /* PR middle-end/57748 */
> /* { dg-do run } */
> /* wrong code in expand_expr_real_1. */
>
> #include
>
> extern void abort (void);
>
> typedef long long V
> __attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
>
> typedef stru
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 hand
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 caterin
On Tue, 3 Dec 2013 15:12:05, Richard Biener wrote:
>
> On Tue, Dec 3, 2013 at 2:10 PM, Richard Biener
> wrote:
>> On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
>> wrote:
>>> Hi Jeff,
>>>
>>> please find attached the patch (incl. test cases) for the unaligned read
>>> BUG that I found while inve
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 need
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 ex
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 v
> 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 t
On Tue, Dec 3, 2013 at 2:10 PM, Richard Biener
wrote:
> On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
> 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.
> 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
On Tue, Dec 3, 2013 at 2:10 PM, Richard Biener
wrote:
> On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
> 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.
On Tue, Dec 3, 2013 at 1:48 PM, Bernd Edlinger
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:
>
> pr57748-3.c:
> /
46 matches
Mail list logo
