oops - this time with attachments...
> Hi,
>
> On Fri, 25 Oct 2013 12:51:13, Richard Biener wrote:
>>
>> On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
>> wrote:
>>> Hi,
>>>
Eh ... even
register struct { int i; int a[0]; } asm ("ebx");
works. Also with int a[1] but not
Hi,
On Fri, 25 Oct 2013 12:51:13, Richard Biener wrote:
>
> On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
> wrote:
>> Hi,
>>
>>> Eh ... even
>>>
>>> register struct { int i; int a[0]; } asm ("ebx");
>>>
>>> works. Also with int a[1] but not with a[2]. So just handling trailing
>>> arrays makes
> Hi,
>
> On Tue, 17 Sep 2013 01:09:45, Martin Jambor wrote:
>>> @@ -4773,6 +4738,8 @@ expand_assignment (tree to, tree from, b
>>>if (MEM_P (to_rtx)
>>>&& GET_MODE (to_rtx) == BLKmode
>>>&& GET_MODE (XEXP (to_rtx, 0)) != VOIDmode
>>> + && bitregion_start ==
Hi,
On Fri, Oct 25, 2013 at 12:51:13PM +0200, Richard Biener wrote:
> On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
> wrote:
> > Hi,
> >
> >> Eh ... even
> >>
> >> register struct { int i; int a[0]; } asm ("ebx");
> >>
> >> works. Also with int a[1] but not with a[2]. So just handling trailing
On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
wrote:
> Hi,
>
>> Eh ... even
>>
>> register struct { int i; int a[0]; } asm ("ebx");
>>
>> works. Also with int a[1] but not with a[2]. So just handling trailing
>> arrays makes this case regress as well.
>>
>> Now I'd call this use questionable ..
Hi,
> Eh ... even
>
> register struct { int i; int a[0]; } asm ("ebx");
>
> works. Also with int a[1] but not with a[2]. So just handling trailing
> arrays makes this case regress as well.
>
> Now I'd call this use questionable ... but we've likely supported that
> for decades and cannot change th
> Doesn't that also apply to arithmetic on symbols when the offset is NULL?
This can presumably be retrofitted when the offset is null or constant.
> But yes, the codegen example posted shows this kind of difference
> (though it doesn't seem to save anything for that case). I'd have expected
> a
On Fri, Oct 25, 2013 at 8:29 AM, Bernd Edlinger
wrote:
> Hi Richard,
>
>
>>> Did you just propose:
>>>
>>> --- stor-layout.c.orig 2013-10-22 10:46:49.233261818 +0200
>>> +++ stor-layout.c 2013-10-24 14:54:00.425259062 +0200
>>> @@ -471,27 +471,7 @@
>>> static enum machine_mode
>>> mode_for_array (
Hi Richard,
>> Did you just propose:
>>
>> --- stor-layout.c.orig 2013-10-22 10:46:49.233261818 +0200
>> +++ stor-layout.c 2013-10-24 14:54:00.425259062 +0200
>> @@ -471,27 +471,7 @@
>> static enum machine_mode
>> mode_for_array (tree elem_type, tree size)
>> {
>> - tree elem_size;
>> - unsigned
On Thu, Oct 24, 2013 at 3:13 PM, Jakub Jelinek wrote:
> On Thu, Oct 24, 2013 at 02:55:59PM +0200, Bernd Edlinger wrote:
>> On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
>> >
>> > On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou
>> > wrote:
>> >>> I think that is common practice to write arr
>> Did you just propose:
>>
>> --- stor-layout.c.orig 2013-10-22 10:46:49.233261818 +0200
>> +++ stor-layout.c 2013-10-24 14:54:00.425259062 +0200
>> @@ -471,27 +471,7 @@
>> static enum machine_mode
>> mode_for_array (tree elem_type, tree size)
>> {
>> - tree elem_size;
>> - unsigned HOST_WIDE_INT
On Thu, Oct 24, 2013 at 02:55:59PM +0200, Bernd Edlinger wrote:
> On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
> >
> > On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou
> > wrote:
> >>> I think that is common practice to write array[1]; at the end of the
> >>> structure, and use it as a flex
On Thu, Oct 24, 2013 at 2:55 PM, Bernd Edlinger
wrote:
> On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
>>
>> On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou
>> wrote:
I think that is common practice to write array[1]; at the end of the
structure, and use it as a flexible array. T
On Thu, Oct 24, 2013 at 12:22 PM, Richard Biener
wrote:
> On Thu, Oct 24, 2013 at 9:15 AM, Bernd Edlinger
> wrote:
>> Hi Martin,
>>
>> On Wed, 23 Oct 2013 19:11:06, Martin Jambor wrote:
>>> On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>>>
>>> ...
>>>
? And why should
On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
>
> On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou wrote:
>>> I think that is common practice to write array[1]; at the end of the
>>> structure, and use it as a flexible array. The compute_record_mode has no
>>> way to decide if that is going t
On Thu, Oct 24, 2013 at 9:15 AM, Bernd Edlinger
wrote:
> Hi Martin,
>
> On Wed, 23 Oct 2013 19:11:06, Martin Jambor wrote:
>> On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>>>
>>
>> ...
>>
>>> ? And why should the same issue not exist for
>>>
>>> struct { V a[1]; }
>>>
>>
>> inde
On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou wrote:
>> I think that is common practice to write array[1]; at the end of the
>> structure, and use it as a flexible array. The compute_record_mode has no
>> way to decide if that is going to be a flexible array or not.
>>
>> Sorry Eric, but now I h
Hi,
On Thu, 24 Oct 2013 11:56:52, Richard Biener wrote:
>
> On Thu, Oct 24, 2013 at 10:37 AM, Eric Botcazou wrote:
>>> if bitregion_start/end are used after the adjust_address call then they have
>>> to be adjusted (or bitpos left in place). In fact why we apply byte-parts
>>> of bitpos here only
> Because of the ABI-change?
While concerns about accidental ABI changes are real, they shouldn't be
overstated either. We have been saying for longer than a decade that the
back-ends must not use the type mode to implement calling conventions and
other external interfaces.
--
Eric Botcazou
On Thu, Oct 24, 2013 at 10:37 AM, Eric Botcazou wrote:
>> if bitregion_start/end are used after the adjust_address call then they have
>> to be adjusted (or bitpos left in place). In fact why we apply byte-parts
>> of bitpos here only if offset != 0 is weird.
>
> Presumably to be able to do arith
>> I think that is common practice to write array[1]; at the end of the
>> structure, and use it as a flexible array. The compute_record_mode has no
>> way to decide if that is going to be a flexible array or not.
>>
>> Sorry Eric, but now I have no other choice than to go back to my previous
>> ve
> I think that is common practice to write array[1]; at the end of the
> structure, and use it as a flexible array. The compute_record_mode has no
> way to decide if that is going to be a flexible array or not.
>
> Sorry Eric, but now I have no other choice than to go back to my previous
> version
> if bitregion_start/end are used after the adjust_address call then they have
> to be adjusted (or bitpos left in place). In fact why we apply byte-parts
> of bitpos here only if offset != 0 is weird.
Presumably to be able to do arithmetic on symbols when the offset is variable,
which can save
Hi Martin,
On Wed, 23 Oct 2013 19:11:06, Martin Jambor wrote:
> On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>>
>
> ...
>
>> ? And why should the same issue not exist for
>>
>> struct { V a[1]; }
>>
>
> indeed, it does and it's simple to trigger (on x86_64):
>
>
On, Wed, 23 Oct 2013 17:36:41Richard Biener wrote:
>
> if bitregion_start/end are used after the adjust_address call then they have
> to be adjusted (or bitpos left in place). In fact why we apply byte-parts of
> bitpos here only if offset != 0 is weird. OTOH this code is _very_ old...
> what happe
> While I was willing to discount the zero sized array as an
> insufficiently specified oddity, this seems to be much more serious
> and potentially common. It seems we really need to be able to detect
> these out-of-bounds situations and tell lower levels of expander that
> "whatever mode you thi
> That looks backwards. Why should
>
> struct { V i; V j[0]; }
>
> have a different mode than
>
> struct { V j[0]; V i; }
>
> ?
Because we support out-of-bounds access for the array in the former case and
not in the latter case.
> And why should the same issue not exist for
>
> struct { V
Hi,
On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>
...
> ? And why should the same issue not exist for
>
> struct { V a[1]; }
>
indeed, it does and it's simple to trigger (on x86_64):
/* { dg-do run } */
#include
typedef long lo
On Tue, Oct 22, 2013 at 12:25 PM, Bernd Edlinger
wrote:
> Hi,
>
>> On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote:
>>>
I agree, that assigning a non-BLKmode to structures with zero-sized arrays
should be considered a bug.
>>>
>>> Fine, then let's apply Martin's patch, on mainline at le
On Tue, Oct 22, 2013 at 3:50 PM, Bernd Edlinger
wrote:
> Hi,
>
> On Tue, 17 Sep 2013 01:09:45, Martin Jambor wrote:
>>> @@ -4773,6 +4738,8 @@ expand_assignment (tree to, tree from, b
>>>if (MEM_P (to_rtx)
>>>&& GET_MODE (to_rtx) == BLKmode
>>>&& GET_MODE (XEXP (to_r
Hi,
On Tue, 17 Sep 2013 01:09:45, Martin Jambor wrote:
>> @@ -4773,6 +4738,8 @@ expand_assignment (tree to, tree from, b
>> if (MEM_P (to_rtx)
>> && GET_MODE (to_rtx) == BLKmode
>> && GET_MODE (XEXP (to_rtx, 0)) != VOIDmode
>> + && bitregion_start == 0
>> +
Hi,
> On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote:
>>
>>> I agree, that assigning a non-BLKmode to structures with zero-sized arrays
>>> should be considered a bug.
>>
>> Fine, then let's apply Martin's patch, on mainline at least.
>>
>
> That would definitely be a good move. Maybe someone sh
> But if zero-sized arrays everywhere in a structure is valid C,
> then the attached test case is a valid test case.
Not necessarily, you can write the declaration but you cannot index the array,
i.e. this is undefined behavior. And there is nothing new, distinct fields
have been disambiguated
Hi Eric,
>> Would you agree that this "error: flexible array member"
>> should also be emitted for a zero-sized array member,
>> maybe as "error: zero-sized array member not at end of struct"?
>
> I would have answered yes when zero-sized arrays where introduced, but it's
> far less clear a couple
> Would you agree that this "error: flexible array member"
> should also be emitted for a zero-sized array member,
> maybe as "error: zero-sized array member not at end of struct"?
I would have answered yes when zero-sized arrays where introduced, but it's
far less clear a couple of decades later
Hi,
>> That would definitely be a good move. Maybe someone should approve it?
>
> Yes, but I agree that we ought to restrict it to trailing zero-sized arrays.
> That would help to allay Jakub's concerns about possible ABI fallouts.
>
> --
> Eric Botcazou
Other uses of zero-sized arrays in structu
> That would definitely be a good move. Maybe someone should approve it?
Yes, but I agree that we ought to restrict it to trailing zero-sized arrays.
That would help to allay Jakub's concerns about possible ABI fallouts.
--
Eric Botcazou
Hi,
On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote:
>
>> I agree, that assigning a non-BLKmode to structures with zero-sized arrays
>> should be considered a bug.
>
> Fine, then let's apply Martin's patch, on mainline at least.
>
That would definitely be a good move. Maybe someone should approv
> I agree, that assigning a non-BLKmode to structures with zero-sized arrays
> should be considered a bug.
Fine, then let's apply Martin's patch, on mainline at least.
> And again, this is not only a problem of structures with zero-sized
> arrays at the end. Remember my previous example code:
> O
Hi,
On Tue, 8 Oct 2013 10:01:37, Eric Botcazou wrote:
>
>> OK, what do you think of it now?
>
> My take on this is that the Proper Fix(tm) has been posted by Martin:
> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00082.html
> IMO it's a no-brainer, modulo the ABI concern. Everything else is more o
> OK, what do you think of it now?
My take on this is that the Proper Fix(tm) has been posted by Martin:
http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00082.html
IMO it's a no-brainer, modulo the ABI concern. Everything else is more or
less clever stuff to paper over this original compute_recor
Hi Eric,
On Fri, 27 Sep 2013 10:36:57, Eric Botcazou wrote:
>
>> Sure, but the modifier is not meant to force something into memory,
>> especially when it is already in an register. Remember, we are only
>> talking of structures here, and we only want to access one member.
>>
>> It is more the ot
> Sure, but the modifier is not meant to force something into memory,
> especially when it is already in an register. Remember, we are only
> talking of structures here, and we only want to access one member.
>
> It is more the other way round:
> It says: "You do not have to load the value in a re
Hi,
On Thu, 26 Sep 2013 11:34:02, Eric Botcazou wrote:
>
>> So I still think my patch does the right thing.
>>
>> The rationale is:
>>
>> = expand_expr (tem,
>> (TREE_CODE (TREE_TYPE (tem)) == UNION_TYPE
>> && COMPLETE_TYPE_P (TREE_TYPE (tem))
>> && (TREE_CODE (TYPE_SIZE (TREE_TYPE (tem)))
>> != I
> So I still think my patch does the right thing.
>
> The rationale is:
>
> = expand_expr (tem,
> (TREE_CODE (TREE_TYPE (tem)) == UNION_TYPE
> && COMPLETE_TYPE_P (TREE_TYPE (tem))
> && (TREE_CODE (TYPE_SIZE (TR
Hi,
On Wed, 25 Sep 2013 16:01:02, Martin Jambor wrote:
> Hi,
>
> On Wed, Sep 25, 2013 at 11:05:21AM +0200, Richard Biener wrote:
>> On Tue, Sep 24, 2013 at 8:06 PM, Martin Jambor wrote:
>>> Hi,
>>>
>>> On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
On Tue, Sep 24, 2013 at 4:
Hi,
On Wed, Sep 25, 2013 at 11:05:21AM +0200, Richard Biener wrote:
> On Tue, Sep 24, 2013 at 8:06 PM, Martin Jambor wrote:
> > Hi,
> >
> > On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
> >> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
> >> wrote:
> >> > Hi,
> >> >
> >> > wi
Hmm.,
On Tue, 24 Sep 2013 20:06:51, Martin Jambor wrote:
> Hi,
>
> On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
>> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
>> wrote:
>>> Hi,
>>>
>>> with the attached patch the read-side of the out of bounds accesses are
>>> fixed.
>
On Tue, Sep 24, 2013 at 8:06 PM, Martin Jambor wrote:
> Hi,
>
> On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
>> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
>> wrote:
>> > Hi,
>> >
>> > with the attached patch the read-side of the out of bounds accesses are
>> > fixed.
>> >
Hi,
On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
> wrote:
> > Hi,
> >
> > with the attached patch the read-side of the out of bounds accesses are
> > fixed.
> > There is a single new test case pr57748-3.c that is derived from M
> Index: gcc/expr.c
> ===
> --- gcc/expr.c (revision 202778)
> +++ gcc/expr.c (working copy)
> @@ -9878,7 +9878,7 @@
> && modifier != EXPAND_STACK_PARM
> ? target : NULL_RTX),
>
On Tue, 24 Sep 2013 13:13:09, Richard Biener wrote:
That is, do you see anything break with just removing the movmisalign path?
I'd rather install that (with the new testcases that then pass) separately
as
this is a somewhat fragile area and being able to more selectively
On Tue, Sep 17, 2013 at 2:08 PM, Bernd Edlinger
wrote:
> On Tue, 17 Sep 2013 12:45:40, Richard Biener wrote:
>>
>> On Tue, Sep 17, 2013 at 12:00 PM, Richard Biener
>> wrote:
>>> On Sun, Sep 15, 2013 at 6:55 PM, Bernd Edlinger
>>> wrote:
Hello Richard,
attached is my second attempt
On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
wrote:
> Hi,
>
> with the attached patch the read-side of the out of bounds accesses are fixed.
> There is a single new test case pr57748-3.c that is derived from Martin's
> test case.
> The test case does only test the read access and does not depe
On Tue, 17 Sep 2013 12:45:40, Richard Biener wrote:
>
> On Tue, Sep 17, 2013 at 12:00 PM, Richard Biener
> wrote:
>> On Sun, Sep 15, 2013 at 6:55 PM, Bernd Edlinger
>> wrote:
>>> Hello Richard,
>>>
>>> attached is my second attempt at fixing PR 57748. This time the movmisalign
>>> path is complet
On Tue, Sep 17, 2013 at 12:00 PM, Richard Biener
wrote:
> On Sun, Sep 15, 2013 at 6:55 PM, Bernd Edlinger
> wrote:
>> Hello Richard,
>>
>> attached is my second attempt at fixing PR 57748. This time the movmisalign
>> path is completely removed and a similar bug in the read handling of
>> misali
On Sun, Sep 15, 2013 at 6:55 PM, Bernd Edlinger
wrote:
> Hello Richard,
>
> attached is my second attempt at fixing PR 57748. This time the movmisalign
> path is completely removed and a similar bug in the read handling of
> misaligned
> structures with a non-BLKmode is fixed too. There are sever
Hi Martin,
On Tue, 17 Sep 2013 01:09:45, Martin Jambor wrote:
> Hi,
>
> On Sun, Sep 15, 2013 at 06:55:17PM +0200, Bernd Edlinger wrote:
>> Hello Richard,
>>
>> attached is my second attempt at fixing PR 57748. This time the
>> movmisalign path is completely removed and a similar bug in the read
>>
Hi,
On Sun, Sep 15, 2013 at 06:55:17PM +0200, Bernd Edlinger wrote:
> Hello Richard,
>
> attached is my second attempt at fixing PR 57748. This time the
> movmisalign path is completely removed and a similar bug in the read
> handling of misaligned structures with a non-BLKmode is fixed
> too. Th
Hello Richard,
attached is my second attempt at fixing PR 57748. This time the movmisalign
path is completely removed and a similar bug in the read handling of misaligned
structures with a non-BLKmode is fixed too. There are several new test cases
for the
different possible failure modes.
This p
>> While it is straight forward to remove the movmisalign path in 4.9 and 4.8,
>> this is not so simple in the 4.7 branch. The reason is that 4.7 uses
>> "to_rtx = expand_normal (tem);" while 4.8 and 4.9 use
>> "to_rtx = expand_expr (tem, NULL_RTX, VOIDmode, EXPAND_WRITE);"
>> which does almost the
> While it is straight forward to remove the movmisalign path in 4.9 and 4.8,
> this is not so simple in the 4.7 branch. The reason is that 4.7 uses
> "to_rtx = expand_normal (tem);" while 4.8 and 4.9 use
> "to_rtx = expand_expr (tem, NULL_RTX, VOIDmode, EXPAND_WRITE);"
> which does almost the same
On Wed, 11 Sep 2013 15:43:53, Richard Biener wrote:
> On Wed, Sep 11, 2013 at 3:41 PM, Bernd Edlinger
> wrote:
>> On Tue, 10 Sep 2013 21:32:29, Martin Jambor wrote:
>>> The misalignp path was added by you during the 4.7 development to fix
>>> PR 50444 which was indeed about expansion of a SRA gene
On Wed, Sep 11, 2013 at 3:41 PM, Bernd Edlinger
wrote:
> On Tue, 10 Sep 2013 21:32:29, Martin Jambor wrote:
>> Hi,
>>
>> On Fri, Sep 06, 2013 at 11:19:18AM +0200, Richard Biener wrote:
>>> On Fri, Sep 6, 2013 at 10:35 AM, Bernd Edlinger
>>> wrote:
Richard,
> But the movmisalign path
On Tue, 10 Sep 2013 21:32:29, Martin Jambor wrote:
> Hi,
>
> On Fri, Sep 06, 2013 at 11:19:18AM +0200, Richard Biener wrote:
>> On Fri, Sep 6, 2013 at 10:35 AM, Bernd Edlinger
>> wrote:
>>> Richard,
>>>
But the movmisalign path skips all this code and with the
current code thinks the act
Hi,
On Fri, Sep 06, 2013 at 11:19:18AM +0200, Richard Biener wrote:
> On Fri, Sep 6, 2013 at 10:35 AM, Bernd Edlinger
> wrote:
> > Richard,
> >
> >> But the movmisalign path skips all this code and with the
> >> current code thinks the actual access is in the mode of the
> >> whole structure. (an
On Fri, 6 Sep 2013 11:30:53, Jakub Jelinek wrote:
> On Fri, Sep 06, 2013 at 11:28:44AM +0200, Eric Botcazou wrote:
>>> this patch fixes the ICE and wrong code issues from PR57748. It contains two
>>> test cases. One for the ICE and one for the wrong code. Both test cases
>>> fail for all gcc versio
On Fri, Sep 06, 2013 at 11:28:44AM +0200, Eric Botcazou wrote:
> > this patch fixes the ICE and wrong code issues from PR57748. It contains two
> > test cases. One for the ICE and one for the wrong code. Both test cases
> > fail for all gcc versions, and are fixed with this patch.
> >
> > Bootstra
Just for completeness, these were the test examples on
this private communication:
On Fri, 6 Sep 2013 11:19:18, Richard Biener wrote:
> On Fri, Sep 6, 2013 at 10:35 AM, Bernd Edlinger
> wrote:
>> Richard,
>>
>>> But the movmisalign path skips all this code and with the
>>> current code thinks the
> this patch fixes the ICE and wrong code issues from PR57748. It contains two
> test cases. One for the ICE and one for the wrong code. Both test cases
> fail for all gcc versions, and are fixed with this patch.
>
> Bootstrapped and tested on x86_64-linux without any problems.
>
> OK for trunk a
On Fri, Sep 6, 2013 at 10:35 AM, Bernd Edlinger
wrote:
> Richard,
>
>> But the movmisalign path skips all this code and with the
>> current code thinks the actual access is in the mode of the
>> whole structure. (and also misses the address adjustment
>> as shown in the other testcase for the bug)
On Fri, Sep 6, 2013 at 9:03 AM, Bernd Edlinger
wrote:
> Hi,
>
> this patch fixes the ICE and wrong code issues from PR57748. It contains two
> test cases.
> One for the ICE and one for the wrong code. Both test cases fail for all gcc
> versions,
> and are fixed with this patch.
>
> Bootstrapped
72 matches
Mail list logo
