[PATCH] D113086: [flang] Add a semantics test for co_reduce

Katherine Rasmussen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 30 14:37:39 PST 2021


ktras added a comment.

In D113086#3145204 <https://reviews.llvm.org/D113086#3145204>, @ekieri wrote:

> It would be nice with a comment citing where in the standard the interface of this intrinsic is specified.
>
> Are you (or do you know of someone who is) planning to write the actual semantics checks as well in the near future? I am a little concerned that otherwise the developer who does that one day will be unaware of these tests, and implements additional tests of their own, leaving your tests with XFAIL.
>
> Also, your test contains trailing whitespace. Is this intentional? The coding standards [1] discourages that, and while they primarily concern the c++ code, the argument provided is no less valid for test cases in fortran. On the other hand, @klausler made the (also valid) argument in D113077 <https://reviews.llvm.org/D113077> that variations in style improves test coverage of the parser. I am fine with you doing it either way, but encourage you to make a conscious decision about it.
>
> [1] https://llvm.org/docs/CodingStandards.html#whitespace

@ekieri, in response to one of your questions, I am currently working on adding the collectives to `flang/Evaluate/intrinsics.cpp`. When I submit patches, they will include changes to the corresponding collective test to remove the XFAIL directive. However, regardless of who works on the collectives, when a test with an XFAIL directive does not fail, the test will be listed as `Unexpectedly Passed` when running the test suite. This should bring any developer's attention to these tests that @rouson is submitting.


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

https://reviews.llvm.org/D113086



More information about the llvm-commits mailing list