Re: Fix type of malloc call in trans-expr.cc
> Hi Jakub, > > Good catch! Does it fix any specific PR? > > If you don't have the time, I would be happy to apply the correction to > 13-branch through to mainline. I caught it with my WIP patch to improve tree-ssa-dce. I am not aware it can produce wrong code. It will likely lead to missed optimization. I was wondering. We are very loose on type checking of calls because K&R C is evil. Perhaps we could have flag to enable more strict type checking for gimple produced by other languages. We would need to give up with LTO, but that is fine. I think bacporting the patch is fine. It is quite obvious fix. Honza
Re: [PATCH 1/2] symtab: also change RTL decl name
> Hi Honza, Ping.
> Regtests cleanly for c,fortran,c++,ada,d,go,lto,objc,obj-c++
> Ok?
> I'd need this for attribute target_clones for the Fortran FE.
Sorry for delay here.
> > void
> > @@ -303,6 +301,10 @@ symbol_table::change_decl_assembler_name (tree decl,
> > tree name)
> > warning (0, "%qD renamed after being referenced in assembly", decl);
> >
> >SET_DECL_ASSEMBLER_NAME (decl, name);
> > + /* Set the new name in rtl. */
> > + if (DECL_RTL_SET_P (decl))
> > + XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (name);
I am not quite sure how safe this is. We generally produce DECL_RTL
when we produce assembly file. So if DECL_RTL is set then we probably
already output the original function name and it is too late to change
it.
Also RTL is shared so changing it in-place is going to rewrite all the
existing RTL expressions using it.
Why the DECL_RTL is produced for function you want to rename?
Honza
> > +
> >if (alias)
> > {
> > IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;
>
Re: [PATCH 1/2] symtab: also change RTL decl name
> On Mon, 21 Nov 2022 20:02:49 +0100
> Jan Hubicka wrote:
>
> > > Hi Honza, Ping.
> > > Regtests cleanly for c,fortran,c++,ada,d,go,lto,objc,obj-c++
> > > Ok?
> > > I'd need this for attribute target_clones for the Fortran FE.
> > Sorry for delay here.
> > > > void
> > > > @@ -303,6 +301,10 @@ symbol_table::change_decl_assembler_name (tree
> > > > decl, tree name)
> > > > warning (0, "%qD renamed after being referenced in assembly",
> > > > decl);
> > > >
> > > >SET_DECL_ASSEMBLER_NAME (decl, name);
> > > > + /* Set the new name in rtl. */
> > > > + if (DECL_RTL_SET_P (decl))
> > > > + XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER
> > > > (name);
> >
> > I am not quite sure how safe this is. We generally produce DECL_RTL
> > when we produce assembly file. So if DECL_RTL is set then we probably
> > already output the original function name and it is too late to change
> > it.
>
> AFAICS we make_decl_rtl in the fortran FE in trans_function_start.
I see, it may be a relic of something that is no longer necessary. Can
you see why one needs DECL_RTL so early?
>
> >
> > Also RTL is shared so changing it in-place is going to rewrite all the
> > existing RTL expressions using it.
> >
> > Why the DECL_RTL is produced for function you want to rename?
>
> I think the fortran FE sets it quite early when lowering a function.
> Later, when the ME creates the target_clones, it wants to rename the
> initial function to initial_fun.default for the default target.
> That's where the change_decl_assembler_name is called (only on the
> decl).
> But nobody changes the RTL name, so the ifunc (which should be the
> initial, unchanged name) is properly emitted but
> assemble_start_function uses the same, unchanged, initial fnname it
> later obtains by get_fnname_from_decl which fetches the (wrong) initial
> name where it should use the .default target name.
> See
> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605081.html
>
> I'm open to other suggestions to make this work in a different way, of
> course. Maybe we're missing some magic somewhere that might share the
> name between the fndecl and the RTL XSTR so the RTL is magically
> updated by that single SET_ECL_ASSEMBLER_NAME in
> change_decl_assembler_name? But i didn't quite see where that'd be?
I think we should start by understanding why Fortran FE produces
DECL_RTL early. It was written before symbol table code emerged, it may
be simply an oversight I made while converting FE to symbol table.
Honza
>
> thanks,
>
> > Honza
> > > > +
> > > >if (alias)
> > > > {
> > > > IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;
> > >
>
Re: [Patch] (was: Re: [r12-4457 Regression] FAIL: gfortran.dg/deferred_type_param_6.f90 -Os execution test on Linux/x86_64)
>
> Fortran has for a long time 'character(len=5), allocatable" or
> "character(len=*)". In the first case, the "5" can be ignored as both
> caller and callee know the length. In the second case, the length is
> determined by the argument, but it cannot be changed.
>
> Since a not-that-short while, 'len=:' together with allocatable/pointer
> is supported.
>
> In the latter case, the value can be change when the array
> association/allocation is changed.
>
> I attached a patch, which was not tested. I am not quite sure whether
> the pointer address can actually escape or not - I think cannot but I
> played safe.
>
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
> München, HRB 106955
> Fortran: Fix fn spec for character-returning functions
>
> gcc/fortran/ChangeLog
> * trans-types.c (create_fn_spec): For character-returning functions,
> set the hidden string-length argument to 'R' only when the "len=:",
> i.e. deferred length which goes alongside with allocatable/pointer.
>
> diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
> index 220976babb8..637d2c71d01 100644
> --- a/gcc/fortran/trans-types.c
> +++ b/gcc/fortran/trans-types.c
> @@ -3008,7 +3008,14 @@ create_fn_spec (gfc_symbol *sym, tree fntype)
> }
>if (sym->ts.type == BT_CHARACTER)
> {
> - spec[spec_len++] = 'R';
> + if (!sym->ts.u.cl->length
> + && ((sym->attr.allocatable && sym->attr.target)
> + || sym->attr.pointer))
> + spec[spec_len++] = '.';
> + if (!sym->ts.u.cl->length && sym->attr.allocatable)
> + spec[spec_len++] = 'w';
> + else
> + spec[spec_len++] = 'R';
> spec[spec_len++] = ' ';
Thanks a lot! I was just looking into that function and was quite
confused on what is going there. Are you going to commit the patch?
Also escaping is quite important bit of information so it would be
good to figure out if it really can escape rather than playing safe.
Honza
> }
> }
Re: [PATCH] vect: Remove vec_outside/inside_cost fields
> > Now afunc writes to __var_5_mma only indirectly so I think it is correct
> > that
> > we optimize the conditional out.
> >
> > Easy fix would be to add -fno-ipa-modref, but perhaps someone with
> > better understanding of Fortran would help me to improve the testcase so
> > the calls to matmul_r4 remains reachable?
>
> I think the two matmul_r4 cases were missed optimizations before so just
> changing the expected number of calls to zero is the correct fix here. Indeed
> we can now statically determine the matrices are not large and so only
> keep the inline copy.
I have updated the matmul as follows.
gcc/testsuite/ChangeLog:
2021-11-11 Jan Hubicka
* gfortran.dg/inline_matmul_17.f90: Fix template
diff --git a/gcc/testsuite/gfortran.dg/inline_matmul_17.f90
b/gcc/testsuite/gfortran.dg/inline_matmul_17.f90
index d2ca8e2948a..cff4b6ce5e2 100644
--- a/gcc/testsuite/gfortran.dg/inline_matmul_17.f90
+++ b/gcc/testsuite/gfortran.dg/inline_matmul_17.f90
@@ -45,4 +45,4 @@ program main
c = matmul(a, bfunc())
if (any(c-d /= 0)) STOP 6
end program main
-! { dg-final { scan-tree-dump-times "matmul_r4" 2 "optimized" } }
+! { dg-final { scan-tree-dump-not "matmul_r4" "optimized" } }
