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