On Fri, 29 Oct 2021 19:15:13 +0200
Bernhard Reutner-Fischer <rep.dot....@gmail.com> wrote:

> > But most importantly, I really don't like these helpers at all, they
> > unnecessarily allocate memory of the remaining duration of compilation,
> > and the second one even uses heap for temporary.  

> I can easily switch the second one to use XALLOCAVEC if you'd accept
> that? Ok?

const char *
gfc_get_name_from_uop (const char *name)
{
  gcc_assert (name[0] == '.');
  const size_t len = strlen (name) - 1;
  gcc_assert (len > 1);
  gcc_assert (name[len] == '.');
  const char *ret = gfc_get_string ("%.*s", len - 1, name + 1);
  return ret;
}
if that's deemed portable enough nowadays? There seem to be preexisting
users in the tree so should be ok.
We can make these checking asserts or remove them altogether of course.

Would that be acceptable?
thanks,

> 
> > Can't you just fix the real bug and keep the code as it was otherwise
> > (with XALLOCAVEC etc.)?
> > And, there should be a testcase...  
> 
> I tried to construct a testcase yesterday but failed.
> I took udr10.f90 and experimented with not using a derived type
> (because DERIVED || CLASS bypasses the failure to lookup st).
> I tried to move the module out to its own source to no avail and gave up
> late at night.
> 
> Unrelated note:
> One thing that looked odd to my untrained eyes was in e.g. udr10.f90
> where you write:
> !$omp parallel do reduction(+:j) reduction(.localadd.:k)
>   do i = 1, 100
>     j = j .localadd. dl(i)
>     k = k + dl(i * 2)
> 
> which may of course be correct (even if + would be implemented by e.g.
> a twice_i procedure (and not addme like the user operator) but i'd have
> assumed the reduction oper to match the target var in the region, like
> !$omp parallel do reduction(.localadd.:j) reduction(+:k)
> But i admittedly know nothing about openmp syntax so it's certainly fine
> as written?
> 
> PS: you have at least
> declare-simd-3.f90:! { dg-do compile { target { lp64 && { ! lp64 } } } }
> declare-target-2.f90:! { dg-do compile { target { lp64 && { ! lp64 } } } }
> 
> and i think later on
> ! { dg-do compile { target skip-all-targets } }
> was added, presumably for this very purpose.
> 
> thanks,

Reply via email to