bef2bb34bfadd61cf399a15f096c84980121279f

• Added comments in the runtime functions indicating that the non-negativity constraint on n1 and n2 that is required by the standard is not enforced.

Significant changes to the patch:

@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 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.

In D138150#3959346, @jeanPerier wrote:

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

In D138150#3936115, @jeanPerier wrote:

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

Yes, I am aware of the other patch which also adds test/Driver/pass-plugin.f90. I will keep an eye out for it.

Previous revision was missing the tests.

Only add a dependence to the Bye plugin when plugins are enabled.

Added a dependence for the flang tests on LLVM's Bye plugin. This results in the Bye plugin being built when check-flang is run, even if building examples has been explicitly disabled.

In D129156#3851843, @awarzynski wrote:

@tarunprabhu Could you confirm that with the build command above "pass-plugin.f90" is failing for you? It is for me.

In D129156#3851843, @awarzynski wrote:

@tarunprabhu Could you confirm that with the build command above "pass-plugin.f90" is failing for you? It is for me.

In D129156#3848728, @awarzynski wrote:

Thanks Valentin! Switching between generators will definitely change the order in which libraries will built (and, AFAIK, the order is non-deterministic to begin with). I will try to experiment later today.

I tried an out-of-tree flang build. check-flang passes in that case too.

I still cannot reproduce the build failure on my end. @MatsPetersson tested this patch and the tests passed. Could someone else test this on their build and let me know if it works - especially if the previous version failed for you.

In D129156#3836838, @clementval wrote:

I probably didn't copy the error correctly .. this is the correct libs that is not found llvm-project/build/lib/Bye.so

I think there is a typo somehow. It should be "Bye.so". Not sure how that happened. I'm looking into this.

Thanks for the review and the comments. :-)

I noticed that clang-format changed several other lines in clang/lib/Driver/ToolChains/Flang.cpp. I undid those and only left in the line relevant to this patch. Do you think that those lines should be run on that file as a separate patch?

Ran clang-format.

Potentially fixed regex to correctly match round brackets.

Used wrong bracket type in previous revision.

Potential fix to failing test.

Potential fix to failing test. It seems that on some platforms, the output is `{anonymous}::Bye`` while on others it is `{anonymous namespace}::Bye``. The test has been fixed to allow for both. I was not able to test this locally, so will see if the buildbot is happy with it.

Made it clear that dynamically loading passes from a pass plugin would add the pass to the optimization pipeline.

Added a sentence in the driver documentation stating that LLVM plugins are not officially supported on Windows.

Removed the pass to test the option. Instead, the Bye plugin provided in LLVM is used to test that the option works. It is not clear if loading passes dynamically is supported on Windows and the test is designed to fail on that platform.

In D129616#3798995, @jeanPerier wrote:

Thanks @tarunprabhu for addressing the previous comments, I agree with @peixin that some lit tests should be added in flang/test/Lower/Intrinsics and that it would make sense to move the EXPAND_AND_QUOTE macro definition in flang/Optimizer/Builder/Runtime/RTBuilder.h.

Otherwise LGTM.

Renamed the EXPAND_AND_QUOTE macro to EXPAND_AND_QUOTE_KEY and moved it to RTBuilder.h

Handle the kind=16 case by manually generating the call to the runtime function.

Addressed @jeanPerier's comment and used asBox instead of asAddr.

In D129584#3673356, @PeteSteinfeld wrote:

@tarunprabhu , this implementation is failing NAG tests. Do you have access to them?

In D129316#3673354, @PeteSteinfeld wrote:

@tarunprabhu , this implementation is causing failures in the NAG tests. Do you have access to them?

All's well that ends well ... :-)

Noted. In my defense, there was a LGTM from @jeanPerier.

Oh, I'm sorry. I should have asked for acceptance before committing.

@PeteSteinfeld, No. I do not have access to the NAG tests.

LGTM. Thanks!

I'd like to second @kiranchandramohan on having a document for features if possible. Also, as @awarzynski said, it would be good to get a sense of what features require a design document.

In D127805#3662927, @awarzynski wrote:

In D127805#3662896, @tarunprabhu wrote:

Add flang driver to tests.

Thanks! What e-mail address should I use when committing this change?

Fix test that was causing build failure.

Add flang driver to tests.

Updated tests to use flang driver in addition to bbc.

Added RUN: %flang_fc1 to tests

In D129616#3662106, @kiranchandramohan wrote:

The lowering functions for parity, iall and iany seem very similar. Is it possible to share some code?

LGTM. Thanks.

Apologies. There was a bug in the test that caused it to always pass.

Fixed typo.

Updated file name and comment.

Added flang driver in addition to BBC in the tests.