[classic-flang] RFC for pull request validation process for Classic Flang

The goal of the Flang Community Pull Request Process is to ensure that only well-tested patches are incorporated into the github.com/flang-compiler/flang master branch. Most Fortran tests, benchmarks, and applications are proprietary, and therefore are not part of the flang public test suite, and these are the tests that need to be run in order to continue to maintain the quality of flang.

To that end, a group of individuals and organizations have volunteered to test and discuss pull requests. The process allows just two assenting votes before a gatekeeper is allowed to merge a pull request into the master branch.

Current volunteers (reviewers) include:

  • Kiran Chandramohan, Rich Barton, Carol Concatto, Andrzej Warzynski (Arm)
  • Gary Klimowicz, Jie Li, Varun Jayathirtha (NVIDIA)
  • Shivaram Rao (AMD)

If you would like to be a volunteer, please submit your request to flang-dev.

Current gatekeepers:

  • Gary Klimowicz and Steve Scalpone, NVIDIA
  • Kiran Chandramohan and Rich Barton, Arm
  • Shivaram Rao, AMD

Gatekeepers are chosen because of their involvement in flang development. If you would like to be a gatekeeper, please submit your request to flang-dev.

The proposed process:

Hi all,

I am proposing two minor amendments to the Pull Request Validation process.

  1. Currently, for merging a Pull Request two volunteers are required to run the tests and approve. But this has to be done for three platforms. We are finding that this requirement for three platforms is slowing down the processing since a gatekeeper do not have access to all three platforms. I propose that we change the requirement to atleast two of Arm, X86, OpenPower.

  2. Currently, the process requires testing on LLVM 9.0. But we are currently setting up support for LLVM 10.0 and vendors using Flang are moving to 10.0/11.0. I propose that we change the requirement to LLVM 9.0 or later versions supported by Flang.

The proposed changes will convert the following point,

Even for the most trivial patch, at least two volunteers must apply the patch, run their internal tests, and give feedback on the patch. The patch should be run on at least Arm, x86 and OpenPOWER on LLVM 9.0.

to

Even for the most trivial patch, at least two volunteers must apply the patch, run their internal tests, and give feedback on the patch. The patch should be run on at least two of Arm, X86, OpenPower on LLVM (9.0 or later versions supported by flang).

We will discuss these two points in the Classic Flang Technical call on Wednesday. Meeting link below. If you need a meeting invite then please let me know.
https://teams.microsoft.com/l/meetup-join/19%3ameeting_YjM5YjlmNGEtMjA0MS00MTRlLTg5ZjUtOTM1ZGIxOTU2NWQy%40thread.v2/0?context=%7b%22Tid%22%3a%22f34e5979-57d9-4aaa-ad4d-b122a662184d%22%2c%22Oid%22%3a%223641875c-ef5b-4767-8105-0787a195852f%22%7d

Thanks,
Kiran

Hi all,

I am proposing two minor amendments to the Pull Request Validation process.

  1. Currently, for merging a Pull Request two volunteers are required to run the tests and approve. But this has to be done for three platforms. We are finding that this requirement for three platforms is slowing down the processing since a gatekeeper do not have access to all three platforms. I propose that we change the requirement to atleast two of Arm, X86, OpenPower.

I’m OK with this as long as one of the two platforms is OpenPower. :slight_smile:

The OpenPower builds don’t get exercised as much as the Arm and X86 builds seem to, as there is no commercial compiler that is dependent on OpenPower. But OpenPower is an important platform for the flang community, so I want to make sure that pull requests are validated on it.

For example, I’ve just started to investigate troubles with building OpenPower on clang, which doesn’t seem to be an issue with Arm and X86. (It may be nothing important, but my initial attempts didn’t build correctly.)

  1. Currently, the process requires testing on LLVM 9.0. But we are currently setting up support for LLVM 10.0 and vendors using Flang are moving to 10.0/11.0. I propose that we change the requirement to LLVM 9.0 or later versions supported by Flang.

I don’t have a problem with this. I will try to build on LLVM 9 and the later releases as it’s easy for me to do it once each build is working.

The proposed changes will convert the following point,

Even for the most trivial patch, at least two volunteers must apply the patch, run their internal tests, and give feedback on the patch. The patch should be run on at least Arm, x86 and OpenPOWER on LLVM 9.0.

to

Even for the most trivial patch, at least two volunteers must apply the patch, run their internal tests, and give feedback on the patch. The patch should be run on at least two of Arm, X86, OpenPower on LLVM (9.0 or later versions supported by flang).

I’d just delete the “two of”, for the reasons above.