This is an archive of the discontinued LLVM Phabricator instance.

[flang] Lower F08 NORM2 intrinsic
ClosedPublic

Authored by tarunprabhu on Nov 16 2022, 1:13 PM.

Details

Summary

Lower the F08 NORM2 intrinsic.

The implementation follows the pattern used in comparable intrinsics. Change the runtime API for Norm2 so it does not expect an (optional) mask argument since the Norm2 intrinsic does not accept a mask in Fortran.

Diff Detail

Event Timeline

tarunprabhu created this revision.Nov 16 2022, 1:13 PM
Herald added a project: Restricted Project. · View Herald Transcript
tarunprabhu requested review of this revision.Nov 16 2022, 1:13 PM

I think we may want to change the runtime API here, otherwise your code looks good to me.

flang/lib/Lower/IntrinsicCall.cpp
4122–4123

If the mask argument of the runtime is not needed for NORM2, I would rather prefer we remove it from the runtime signature if @klausler can confirm it had no purpose.

I think we may want to change the runtime API here, otherwise your code looks good to me.

The runtime implementations of Norm2 eventually use DoTotalReduction which needs a mask argument. We would need to provide an alternative to/overload of DoTotalReduction that does not require a mask.

I think we may want to change the runtime API here, otherwise your code looks good to me.

The runtime implementations of Norm2 eventually use DoTotalReduction which needs a mask argument. We would need to provide an alternative to/overload of DoTotalReduction that does not require a mask.

Couldn't Norm2 runtime implementation just pass a nullptr mask to DoTotalReduction instead ?

Couldn't Norm2 runtime implementation just pass a nullptr mask to DoTotalReduction instead ?

Oh, I didn't realize that one could do that. I'll try it.

Change the runtime API to not require a mask argument since a mask is not used in the Norm2 intrinsic. Instead, pass a null parameter to the internal runtime routines that actually carry out the computation.

Update tests to reflect this change.

tarunprabhu marked an inline comment as done.Nov 30 2022, 9:53 AM

Change the runtime API to not require a mask argument since a mask is not used in the Norm2 intrinsic. Instead, pass a null parameter to the internal runtime routines that actually carry out the computation.

Update tests to reflect this change.

Did you push the patch with the runtime API update ? I do not see a change to flang/include/flang/Runtime/reduction.h.

tarunprabhu edited the summary of this revision. (Show Details)

@jeanPerier, in the previous patch, I said that I had changed the runtime API when all I did was create a null value and pass that along to the runtime. Sorry, that was a silly thing to do and say, This one _actually_ changes the runtime API.

  • Change the runtime API to not require (even optionally) a mask since the norm2 intrinsic in Fortran does not accept a mask.
  • Fix the tests to not explicitly check for fastmath<contract> but instead match against any fastmath specification that appears.
jeanPerier accepted this revision.EditedDec 2 2022, 12:24 AM

Please just make sure to run clang-format on your changes in flang/runtime/extrema.cpp. Otherwise, looks great, thanks for solving my comments.

This revision is now accepted and ready to land.Dec 2 2022, 12:24 AM
This revision was landed with ongoing or failed builds.Dec 5 2022, 12:56 PM
This revision was automatically updated to reflect the committed changes.