Hi Harald,
I don't think it is necessarily the save attribute, but rather it's
representation in "assembler". To my understanding the type's size is
needed to allocate space in the .RSS or .data segment of the binary. To
manage this the size needs to be compile time computable, which it is not
when its bounds are represented by a variable [1:_F.x]. The type layouting
is confused by that and messes up the tree expression for the type.
This is how I explained it to me. May be someone with more insight into the
type layouting process or middle-end/gimple can correct me.
Thanks for the ok, I will merge tomorrow first thing in the morning.
Regards,
Andre
Andre Vehreschild * ve...@gmx.de
Am 3. Juni 2025 22:00:00 schrieb Harald Anlauf <anl...@gmx.de>:
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
