[flang-commits] [PATCH] D106820: [flang] Fix runtime ICE with maxloc and scalar result

[Peter Klausler via Phabricator via flang-commits]

klausler added inline comments.


================
Comment at: flang/runtime/reduction-templates.h:209
+    result.GetLowerBounds(at);
   INTERNAL_CHECK(at[0] == 1);
   using CppType = CppTypeFor<CAT, KIND>;
----------------
mleair wrote:
> mleair wrote:
> > klausler wrote:
> > > `INTERNAL_CHECK(result.rank() == 0 || at[0] == 1);`
> > Hi Peter,
> > 
> > I thought about that change, but I  think "at" is still referenced after this check. For example:
> > 
> >  // No MASK= or scalar MASK=.TRUE.
> > for (auto n{result.Elements()}; n-- > 0; result.IncrementSubscripts(at)) {
> >     accumulator.Reinitialize();
> >     ReduceDimToScalar<CppType, ACCUMULATOR>(
> >         x, dim - 1, at, result.Element<CppType>(at), accumulator);
> >   }
> > 
> > It looks like ReduceDimToScalar() still references "at". If so, then "at" should be initialized, correct? I believe this is where the UMR comes from. When at[0] is initialized to 1, the UMR goes away.
> > 
> To clarify, I think there might be a second UMR with with ReduceDimToScalar(); not just with the INTERNAL_CHECK macro.
I doubt it.  When rank == 0, the subscript values should not be used by Descriptor::SubscriptsToByteOffset() (and therefore not by Descriptor::Element) or by ReduceDimToScalar.  How do you know that ReduceDimToScalar() is referencing uninitialized memory?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106820/new/

https://reviews.llvm.org/D106820