* Re: Possible patch for pr62242 -- follow-up
@ 2015-09-15 13:36 Dominique d'Humières
2015-09-16 8:59 ` Paul Richard Thomas
0 siblings, 1 reply; 5+ messages in thread
From: Dominique d'Humières @ 2015-09-15 13:36 UTC (permalink / raw)
To: louis.krupp; +Cc: GNU GFortran, GCC Patches
Louis,
A few comments:
(1) your patch fixes pr62242 and, as said in a previous mail, pr62246, pr60110, and some variants of pr52332. I have updated pr52332 and marked pr62246 and pr60110 as duplicate. Could you please add
PR fortran/52332
to the ChangeLog entry?
(2) You need a ChangeLog entry for the tests in gcc/testsuite.
(3) The usual way to attract attention is a PING in the subject.
(4) AFAICT your patch boils down to
--- ../_clean/gcc/fortran/trans-array.c 2015-08-29 10:47:51.000000000 +0200
+++ gcc/fortran/trans-array.c 2015-09-15 15:21:16.000000000 +0200
@@ -1835,8 +1835,6 @@ get_array_ctor_all_strlen (stmtblock_t *
gfc_add_block_to_block (block, &se.pre);
gfc_add_block_to_block (block, &se.post);
-
- e->ts.u.cl->backend_decl = *len;
}
}
@@ -2226,6 +2224,7 @@ trans_array_constructor (gfc_ss * ss, lo
if (expr->ts.type == BT_CHARACTER)
{
bool const_string;
+ gfc_charlen *new_cl;
/* get_array_ctor_strlen walks the elements of the constructor, if a
typespec was given, we already know the string length and want the one
@@ -2251,7 +2250,9 @@ trans_array_constructor (gfc_ss * ss, lo
and not end up here. */
gcc_assert (ss_info->string_length);
- expr->ts.u.cl->backend_decl = ss_info->string_length;
+ new_cl = gfc_new_charlen (gfc_current_ns, expr->ts.u.cl);
+ new_cl->backend_decl = ss_info->string_length;
+ expr->ts.u.cl = new_cl;
type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
if (const_string)
@@ -2589,7 +2590,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loo
if (expr->ts.type == BT_CHARACTER
&& ss_info->string_length == NULL
&& expr->ts.u.cl
- && expr->ts.u.cl->length)
+ && expr->ts.u.cl->length
+ && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
{
gfc_init_se (&se, NULL);
gfc_conv_expr_type (&se, expr->ts.u.cl->length,
with two questions:
(a) Is the line
e->ts.u.cl->backend_decl = *len;
needed? It does not for my own tests (although this may be masked by errors or other ICEs)
(b) A long question for the hunk at 2251 I don’t understand (thus cannot answer).
Thanks for the patch,
Dominique
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Possible patch for pr62242 -- follow-up
2015-09-15 13:36 Possible patch for pr62242 -- follow-up Dominique d'Humières
@ 2015-09-16 8:59 ` Paul Richard Thomas
2015-09-16 11:32 ` Louis Krupp
2015-09-17 7:52 ` Louis Krupp
0 siblings, 2 replies; 5+ messages in thread
From: Paul Richard Thomas @ 2015-09-16 8:59 UTC (permalink / raw)
To: Dominique d'Humières; +Cc: Louis Krupp, GNU GFortran
Dear Louis and Dominique,
I will take a look at your patch tonight and will attempt to answer
Dominique's two questions in the process.
A comment: Does the assertion in the hunk at 2251 not cast such a wide
net as to make it redundant?
Cheers
Paul
On 15 September 2015 at 15:35, Dominique d'Humières <dominiq@lps.ens.fr> wrote:
> Louis,
>
> A few comments:
>
> (1) your patch fixes pr62242 and, as said in a previous mail, pr62246, pr60110, and some variants of pr52332. I have updated pr52332 and marked pr62246 and pr60110 as duplicate. Could you please add
>
> PR fortran/52332
>
> to the ChangeLog entry?
>
> (2) You need a ChangeLog entry for the tests in gcc/testsuite.
>
> (3) The usual way to attract attention is a PING in the subject.
>
> (4) AFAICT your patch boils down to
>
> --- ../_clean/gcc/fortran/trans-array.c 2015-08-29 10:47:51.000000000 +0200
> +++ gcc/fortran/trans-array.c 2015-09-15 15:21:16.000000000 +0200
> @@ -1835,8 +1835,6 @@ get_array_ctor_all_strlen (stmtblock_t *
>
> gfc_add_block_to_block (block, &se.pre);
> gfc_add_block_to_block (block, &se.post);
> -
> - e->ts.u.cl->backend_decl = *len;
> }
> }
>
> @@ -2226,6 +2224,7 @@ trans_array_constructor (gfc_ss * ss, lo
> if (expr->ts.type == BT_CHARACTER)
> {
> bool const_string;
> + gfc_charlen *new_cl;
>
> /* get_array_ctor_strlen walks the elements of the constructor, if a
> typespec was given, we already know the string length and want the one
> @@ -2251,7 +2250,9 @@ trans_array_constructor (gfc_ss * ss, lo
> and not end up here. */
> gcc_assert (ss_info->string_length);
>
> - expr->ts.u.cl->backend_decl = ss_info->string_length;
> + new_cl = gfc_new_charlen (gfc_current_ns, expr->ts.u.cl);
> + new_cl->backend_decl = ss_info->string_length;
> + expr->ts.u.cl = new_cl;
>
> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
> if (const_string)
> @@ -2589,7 +2590,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loo
> if (expr->ts.type == BT_CHARACTER
> && ss_info->string_length == NULL
> && expr->ts.u.cl
> - && expr->ts.u.cl->length)
> + && expr->ts.u.cl->length
> + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
> {
> gfc_init_se (&se, NULL);
> gfc_conv_expr_type (&se, expr->ts.u.cl->length,
>
> with two questions:
>
> (a) Is the line
>
> e->ts.u.cl->backend_decl = *len;
>
> needed? It does not for my own tests (although this may be masked by errors or other ICEs)
>
> (b) A long question for the hunk at 2251 I don’t understand (thus cannot answer).
>
> Thanks for the patch,
>
> Dominique
>
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Re: Possible patch for pr62242 -- follow-up
2015-09-16 8:59 ` Paul Richard Thomas
@ 2015-09-16 11:32 ` Louis Krupp
2015-09-17 7:52 ` Louis Krupp
1 sibling, 0 replies; 5+ messages in thread
From: Louis Krupp @ 2015-09-16 11:32 UTC (permalink / raw)
To: Paul Richard Thomas; +Cc: "Dominique d'Humières", GNU GFortran
Paul & Dominique,
I've been taking another look at this, too, and I'd like to have another go at understanding what's happening.
On mature reflection, my "if (0)" near line 1836 seems like a bad idea.
The assertion at 2251 is copied from expr.c, and it's in a comment that attempts to explain why the code is creating a new gfc_charlen structure. Unless I can figure out a simpler way to fix this, I'd like to leave the comment as it is, because I think someone might have to revisit this code sooner or later.
Louis
---- On Wed, 16 Sep 2015 01:58:59 -0700 Paul Richard Thomas wrote ----
>Dear Louis and Dominique,
>
>I will take a look at your patch tonight and will attempt to answer
>Dominique's two questions in the process.
>
>A comment: Does the assertion in the hunk at 2251 not cast such a wide
>net as to make it redundant?
>
>Cheers
>
>Paul
>
>On 15 September 2015 at 15:35, Dominique d'Humières <dominiq@lps.ens.fr> wrote:
>> Louis,
>>
>> A few comments:
>>
>> (1) your patch fixes pr62242 and, as said in a previous mail, pr62246, pr60110, and some variants of pr52332. I have updated pr52332 and marked pr62246 and pr60110 as duplicate. Could you please add
>>
>> PR fortran/52332
>>
>> to the ChangeLog entry?
>>
>> (2) You need a ChangeLog entry for the tests in gcc/testsuite.
>>
>> (3) The usual way to attract attention is a PING in the subject.
>>
>> (4) AFAICT your patch boils down to
>>
>> --- ../_clean/gcc/fortran/trans-array.c 2015-08-29 10:47:51.000000000 +0200
>> +++ gcc/fortran/trans-array.c 2015-09-15 15:21:16.000000000 +0200
>> @@ -1835,8 +1835,6 @@ get_array_ctor_all_strlen (stmtblock_t *
>>
>> gfc_add_block_to_block (block, &se.pre);
>> gfc_add_block_to_block (block, &se.post);
>> -
>> - e->ts.u.cl->backend_decl = *len;
>> }
>> }
>>
>> @@ -2226,6 +2224,7 @@ trans_array_constructor (gfc_ss * ss, lo
>> if (expr->ts.type == BT_CHARACTER)
>> {
>> bool const_string;
>> + gfc_charlen *new_cl;
>>
>> /* get_array_ctor_strlen walks the elements of the constructor, if a
>> typespec was given, we already know the string length and want the one
>> @@ -2251,7 +2250,9 @@ trans_array_constructor (gfc_ss * ss, lo
>> and not end up here. */
>> gcc_assert (ss_info->string_length);
>>
>> - expr->ts.u.cl->backend_decl = ss_info->string_length;
>> + new_cl = gfc_new_charlen (gfc_current_ns, expr->ts.u.cl);
>> + new_cl->backend_decl = ss_info->string_length;
>> + expr->ts.u.cl = new_cl;
>>
>> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
>> if (const_string)
>> @@ -2589,7 +2590,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loo
>> if (expr->ts.type == BT_CHARACTER
>> && ss_info->string_length == NULL
>> && expr->ts.u.cl
>> - && expr->ts.u.cl->length)
>> + && expr->ts.u.cl->length
>> + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
>> {
>> gfc_init_se (&se, NULL);
>> gfc_conv_expr_type (&se, expr->ts.u.cl->length,
>>
>> with two questions:
>>
>> (a) Is the line
>>
>> e->ts.u.cl->backend_decl = *len;
>>
>> needed? It does not for my own tests (although this may be masked by errors or other ICEs)
>>
>> (b) A long question for the hunk at 2251 I don’t understand (thus cannot answer).
>>
>> Thanks for the patch,
>>
>> Dominique
>>
>
>
>
>--
>Outside of a dog, a book is a man's best friend. Inside of a dog it's
>too dark to read.
>
>Groucho Marx
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Re: Possible patch for pr62242 -- follow-up
2015-09-16 8:59 ` Paul Richard Thomas
2015-09-16 11:32 ` Louis Krupp
@ 2015-09-17 7:52 ` Louis Krupp
2015-09-17 8:48 ` Paul Richard Thomas
1 sibling, 1 reply; 5+ messages in thread
From: Louis Krupp @ 2015-09-17 7:52 UTC (permalink / raw)
To: Paul Richard Thomas; +Cc: "Dominique d'Humières", GNU GFortran
I'm testing what I think is a better patch. I should have something by late this evening (GMT-6).
Louis
---- On Wed, 16 Sep 2015 01:58:59 -0700 Paul Richard Thomas wrote ----
>Dear Louis and Dominique,
>
>I will take a look at your patch tonight and will attempt to answer
>Dominique's two questions in the process.
>
>A comment: Does the assertion in the hunk at 2251 not cast such a wide
>net as to make it redundant?
>
>Cheers
>
>Paul
>
>On 15 September 2015 at 15:35, Dominique d'Humières <dominiq@lps.ens.fr> wrote:
>> Louis,
>>
>> A few comments:
>>
>> (1) your patch fixes pr62242 and, as said in a previous mail, pr62246, pr60110, and some variants of pr52332. I have updated pr52332 and marked pr62246 and pr60110 as duplicate. Could you please add
>>
>> PR fortran/52332
>>
>> to the ChangeLog entry?
>>
>> (2) You need a ChangeLog entry for the tests in gcc/testsuite.
>>
>> (3) The usual way to attract attention is a PING in the subject.
>>
>> (4) AFAICT your patch boils down to
>>
>> --- ../_clean/gcc/fortran/trans-array.c 2015-08-29 10:47:51.000000000 +0200
>> +++ gcc/fortran/trans-array.c 2015-09-15 15:21:16.000000000 +0200
>> @@ -1835,8 +1835,6 @@ get_array_ctor_all_strlen (stmtblock_t *
>>
>> gfc_add_block_to_block (block, &se.pre);
>> gfc_add_block_to_block (block, &se.post);
>> -
>> - e->ts.u.cl->backend_decl = *len;
>> }
>> }
>>
>> @@ -2226,6 +2224,7 @@ trans_array_constructor (gfc_ss * ss, lo
>> if (expr->ts.type == BT_CHARACTER)
>> {
>> bool const_string;
>> + gfc_charlen *new_cl;
>>
>> /* get_array_ctor_strlen walks the elements of the constructor, if a
>> typespec was given, we already know the string length and want the one
>> @@ -2251,7 +2250,9 @@ trans_array_constructor (gfc_ss * ss, lo
>> and not end up here. */
>> gcc_assert (ss_info->string_length);
>>
>> - expr->ts.u.cl->backend_decl = ss_info->string_length;
>> + new_cl = gfc_new_charlen (gfc_current_ns, expr->ts.u.cl);
>> + new_cl->backend_decl = ss_info->string_length;
>> + expr->ts.u.cl = new_cl;
>>
>> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
>> if (const_string)
>> @@ -2589,7 +2590,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loo
>> if (expr->ts.type == BT_CHARACTER
>> && ss_info->string_length == NULL
>> && expr->ts.u.cl
>> - && expr->ts.u.cl->length)
>> + && expr->ts.u.cl->length
>> + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
>> {
>> gfc_init_se (&se, NULL);
>> gfc_conv_expr_type (&se, expr->ts.u.cl->length,
>>
>> with two questions:
>>
>> (a) Is the line
>>
>> e->ts.u.cl->backend_decl = *len;
>>
>> needed? It does not for my own tests (although this may be masked by errors or other ICEs)
>>
>> (b) A long question for the hunk at 2251 I don’t understand (thus cannot answer).
>>
>> Thanks for the patch,
>>
>> Dominique
>>
>
>
>
>--
>Outside of a dog, a book is a man's best friend. Inside of a dog it's
>too dark to read.
>
>Groucho Marx
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Re: Possible patch for pr62242 -- follow-up
2015-09-17 7:52 ` Louis Krupp
@ 2015-09-17 8:48 ` Paul Richard Thomas
0 siblings, 0 replies; 5+ messages in thread
From: Paul Richard Thomas @ 2015-09-17 8:48 UTC (permalink / raw)
To: Louis Krupp; +Cc: Dominique d'Humières, GNU GFortran
Dear Louis,
OK, I'll hang on in there :-)
Paul
On 17 September 2015 at 09:52, Louis Krupp <louis.krupp@zoho.com> wrote:
> I'm testing what I think is a better patch. I should have something by late this evening (GMT-6).
>
> Louis
>
> ---- On Wed, 16 Sep 2015 01:58:59 -0700 Paul Richard Thomas wrote ----
>>Dear Louis and Dominique,
>>
>>I will take a look at your patch tonight and will attempt to answer
>>Dominique's two questions in the process.
>>
>>A comment: Does the assertion in the hunk at 2251 not cast such a wide
>>net as to make it redundant?
>>
>>Cheers
>>
>>Paul
>>
>>On 15 September 2015 at 15:35, Dominique d'Humières <dominiq@lps.ens.fr> wrote:
>>> Louis,
>>>
>>> A few comments:
>>>
>>> (1) your patch fixes pr62242 and, as said in a previous mail, pr62246, pr60110, and some variants of pr52332. I have updated pr52332 and marked pr62246 and pr60110 as duplicate. Could you please add
>>>
>>> PR fortran/52332
>>>
>>> to the ChangeLog entry?
>>>
>>> (2) You need a ChangeLog entry for the tests in gcc/testsuite.
>>>
>>> (3) The usual way to attract attention is a PING in the subject.
>>>
>>> (4) AFAICT your patch boils down to
>>>
>>> --- ../_clean/gcc/fortran/trans-array.c 2015-08-29 10:47:51.000000000 +0200
>>> +++ gcc/fortran/trans-array.c 2015-09-15 15:21:16.000000000 +0200
>>> @@ -1835,8 +1835,6 @@ get_array_ctor_all_strlen (stmtblock_t *
>>>
>>> gfc_add_block_to_block (block, &se.pre);
>>> gfc_add_block_to_block (block, &se.post);
>>> -
>>> - e->ts.u.cl->backend_decl = *len;
>>> }
>>> }
>>>
>>> @@ -2226,6 +2224,7 @@ trans_array_constructor (gfc_ss * ss, lo
>>> if (expr->ts.type == BT_CHARACTER)
>>> {
>>> bool const_string;
>>> + gfc_charlen *new_cl;
>>>
>>> /* get_array_ctor_strlen walks the elements of the constructor, if a
>>> typespec was given, we already know the string length and want the one
>>> @@ -2251,7 +2250,9 @@ trans_array_constructor (gfc_ss * ss, lo
>>> and not end up here. */
>>> gcc_assert (ss_info->string_length);
>>>
>>> - expr->ts.u.cl->backend_decl = ss_info->string_length;
>>> + new_cl = gfc_new_charlen (gfc_current_ns, expr->ts.u.cl);
>>> + new_cl->backend_decl = ss_info->string_length;
>>> + expr->ts.u.cl = new_cl;
>>>
>>> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
>>> if (const_string)
>>> @@ -2589,7 +2590,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loo
>>> if (expr->ts.type == BT_CHARACTER
>>> && ss_info->string_length == NULL
>>> && expr->ts.u.cl
>>> - && expr->ts.u.cl->length)
>>> + && expr->ts.u.cl->length
>>> + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
>>> {
>>> gfc_init_se (&se, NULL);
>>> gfc_conv_expr_type (&se, expr->ts.u.cl->length,
>>>
>>> with two questions:
>>>
>>> (a) Is the line
>>>
>>> e->ts.u.cl->backend_decl = *len;
>>>
>>> needed? It does not for my own tests (although this may be masked by errors or other ICEs)
>>>
>>> (b) A long question for the hunk at 2251 I don’t understand (thus cannot answer).
>>>
>>> Thanks for the patch,
>>>
>>> Dominique
>>>
>>
>>
>>
>>--
>>Outside of a dog, a book is a man's best friend. Inside of a dog it's
>>too dark to read.
>>
>>Groucho Marx
>>
>
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-17 8:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-15 13:36 Possible patch for pr62242 -- follow-up Dominique d'Humières
2015-09-16 8:59 ` Paul Richard Thomas
2015-09-16 11:32 ` Louis Krupp
2015-09-17 7:52 ` Louis Krupp
2015-09-17 8:48 ` Paul Richard Thomas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).