Hi Harald,
merged as gcc-16-1096-gafa2de8093a. Thanks again for the review.
Regards,
Andre
On Tue, 3 Jun 2025 21:59:52 +0200
Harald Anlauf <anl...@gmx.de> wrote:
> Hi Andre,
>
> On 6/3/25 13:31, Andre Vehreschild wrote:
> > Hi all,
> >
> > thanks for the explanations, Christophe. This is very much appreciated. And
> > sorry, I can't follow all presentations, conferences and publications.
> > There is meanwhile way too much for me to process out there.
> >
> > Anyway, the regression I produced in gomp should be fixed by the new
> > version of patch attached. I only can speculate what is going wrong with
> > the first version of the patch. Obviously the type of array the first patch
> > introduced can not be layed out correctly. There is a mult of arg 0 NOP arg
> > 0 NOP in there which is more than odd. I would expect something like mult
> > arg 0 .len arg 1 char_size.
> >
> > The second version of the patch is some what longer, because it no longer
> > tries to change the type of the static allocatable string to a char array,
> > but sticks to the pointer to char there. In the gfc_conv_substring pointer
> > arithmetic is applied to compute the correct offset of the reference.
>
> I am still struggling with the observation that the SAVE attribute
> should change the variable declaration in such a fundamental way.
> So maybe we had it wrong for deferred-length character before, and
> you're finally fixing that! OTOH I don't see anything wrong with
> your patch, so it is OK for mainline and 15-branch unless anybody
> objects.
>
> Thanks for the patch!
>
> Harald
>
> > Regtests ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
> >
> > Regards,
> > Andre
> >
> > On Tue, 3 Jun 2025 10:14:29 +0200
> > Christophe Lyon <christophe.l...@linaro.org> wrote:
> >
> >> Hi!
> >>
> >>
> >> On Mon, 2 Jun 2025 at 20:53, Andre Vehreschild <ve...@gmx.de> wrote:
> >>>
> >>> Hi Thomas,
> >>>
> >>> thanks for the ok. Unfortunately does the patch regress in gomp (test case
> >>> gomp/pr104382 when I am not mistaken ; the one with the lone 'save'
> >>> statement). This was reported by the regression testing host at first for
> >>> arm, but also occurs on x86_64. Since when are proposed patches checked by
> >>> a CI? That's fantastic!
> >>
> >> On the Linaro side, we put "postcommit" CI in production e/o summer
> >> 2023, and "precommit" CI a few weeks/months later. We made
> >> presentations during the GNU Cauldron 2023 and 2024, as well as during
> >> Linaro Connect 2024 :-) In summary we test various configurations of
> >> "arm" and "aarch64" targets.
> >>
> >> If you didn't notice before, it's because your patches were regression-free
> >> :-)
> >>
> >> Always happy to read positive feedback!
> >>
> >> Thanks
> >>
> >> Christophe
> >>
> >>> I will continue to investigate how to fix that issue.
> >>>
> >>> Regards,
> >>> Andre
> >>> Andre Vehreschild * ve...@gmx.de
> >>>
> >>> Am 2. Juni 2025 20:10:06 schrieb Thomas Koenig <tkoe...@netcologne.de>:
> >>>
> >>>> Hi Andre,
> >>>>
> >>>>
> >>>>> attached patch fixes a missing substring ref on a saved allocatable
> >>>>> string. The issue seems to be, that the variable is declared to be a
> >>>>> character pointer and not a character array. When using the latter (why
> >>>>> not), it works as expected and does not produce any regressions.
> >>>>>
> >>>>> Regtests ok on x86_64-pc-linux-gnu / F41. Ok for mainlines?
> >>>>
> >>>>
> >>>> OK for trunk and also for backporting to gcc 15 (it is a 15/16
> >>>> regression).
> >>>>
> >>>> Best regards
> >>>>
> >>>> Thomas
> >>>
> >>>
> >
> >
>
--
Andre Vehreschild * Email: vehre ad gmx dot de
