reorganize tests

[AlexanderViand-Intel]

@asraa So I ended up giving this a shot after all, and I think for the most part it works out.

I came up with the following structure, based on what we had already thought up during the last meeting + came up with some suggestions to guide future usage (obviously very much up for discussion):

/tests

• Dialects/
  • <DialectName>/ (should match the folder name used for the Dialect in lib). Note that there might also be test folders for dialects defined upstream (e.g., Polynomial)
    • IR/ Simple syntax and verification tests. Tests in this folder should not call any passes on heir-opt or pass any arguments to it (except --verify-diagnostics where necessary)
      • syntax.mlir (or ops.mlir and types.mlir? we currently have both approaches)
      • verifiers.mlir (or invalid.mlir which is used in a few places?) Note: we currently also see some dialects with multiple verify_<thing>.mlir tests
    • Conversions I think it makes sense to have the folder names here reflect the cli argument string, which people interact with regularly, rather than the class name of the pass, which is used only in a very few places (e.g., registering the pass in heir-opt.cpp)
      • <dialectname-to-otherdialect>/ The tests in here should ideally start at roughly the level of abstraction of <DialectName> and --<dialectname-to-otherdialect> should be (one of) the first arguments passed to heir-opt as part of the RUN line. Reasonable exceptions might be "convenience" passes such --secretize/etc that make it easier to write readable tests. Note that I'm only suggesting limiting where these tests start, not where they end. While most tests will probably be expressed in terms of the expected <otherdialect> output, this would also be the place to put tests that go down multiple levels, including all the way to LLVM.
        • <some_sensible_name>.mlir
        • runner/ Most tests are FileCheck tests, which are purely lexical matching, but we also have several tests that use mlir-runner (or OpenFHE or whatever) to actually run a program and check the outputs. Right now, these are, by prior convention, in a folder called runner/, but I'm guessing there might be a better name.
    • Transforms/ Tests that check how a given pass (whether designed specifically for this dialect or not) interact with this dialect. Subfolders, where necessary, follow the same naming convention as for conversions (i.e., cli-argument-pass-name). Currently, this is also where --canonicalize tests go, but we could also consider moving those to IR/ (though canonicalize is technically just another pass/transform).
    • Emitters/ Tests of heir-translate (rather than -opt) that start at the abstraction level of <DialectName>.

In general, I'd suggest that if a conversion/transform/emitter has only a single test file, that test file should be named <cli-argument-name>.mlir. As soon as there are multiple test files for the same thing, they should go into a folder called <cli-argument-name>/ (and can then be called anything reasonable).

• Transforms/ For tests of non-dialect-specific passes, using the same test/subfolder naming convention as above.

• Examples/ Intended to eventually contain nice end-to-end examples of entire pieplines, but for now I'm a lot less sure of what should go in there and how things there should be structured.

I've also noticed we have have a bunch of "checking-that-llvm/mlir-works-as-expected" tests, and I'm not quite sure where to put them. In general, there's still a bunch of tests in tests/old/, most of which I'm not super sure on where they should go, though for some I just simply haven gotten around to moving them.

PS: I've not paid attention to BUILD files/etc for now unless it's a test with a custom bazel setup, as I think it's easier to add the generic test BUILD file back everywhere after the reorg is done.

[asraa]

Thank you for starting this! Cloned it and I''ll push some changes to make everything work with bazel.

cli argument string, which people interact with regularly,

Agree, although I'll uniformly translate this so that they're underscores.

[AlexanderViand-Intel]

Thanks! Btw, I still see two files in tests/old- did you push all of your most recent changes?

@ polynomial runner: I think my idea was that Examples/ should really be for tests that start with things like TF/TOSA and/or "heirlang"/standard-mlir and demonstrate full pipeline(s), whereas, iirc, the polynomial tests are more basic checks that just happen to make use of "mlir-runner" to achieve functionality checking. Essentially grouping by purpose rather than mechanism.

@ benchmark / macros: I think it might be helpful to have tests/utils for all the *.bzl (and other) helpers? Not sure if it's asking too much of bazel, but maybe this could (eventually) be a common place for (more generic versions of) all the mlir-runner/openFHE/etc bazel functions to live, with specific tests just referencing that?

[asraa]

Thanks! Btw, I still see two files in tests/old- did you push all of your most recent changes?

Oops! Rebased and pushed now.

@ polynomial runner: I think my idea was that Examples/ should really be for tests that start with things like TF/TOSA and/or "heirlang"/standard-mlir and demonstrate full pipeline(s), whereas, iirc, the polynomial tests are more basic checks that just happen to make use of "mlir-runner" to achieve functionality checking. Essentially grouping by purpose rather than mechanism.

I see. In that case, I think that probably this will make more sense once we get to the point of having more tests that are running

@ benchmark / macros: I think it might be helpful to have tests/utils for all the *.bzl (and other) helpers? Not sure if it's asking too much of bazel, but maybe this could (eventually) be a common place for (more generic versions of) all the mlir-runner/openFHE/etc bazel functions to live, with specific tests just referencing that?

Right! There is also some in tools/*.bzl now too, but I don't really see a difference in marking these macros as test vs non-test. I think we probably want something like tools/bazel/*.bzl and put all of them there?

[AlexanderViand-Intel]

Right! There is also some in tools/*.bzl now too, but I don't really see a difference in marking these macros as test vs non-test. I think we probably want something like tools/bazel/*.bzl and put all of them there?

Sounds good to me! (doesn't have to be done now, though) Once build-and-test is happy again, this would be good to go from my side 👍

[AlexanderViand-Intel]

@asraa Can you take a look at why the verilog tests fail? I don't see any obvious changes from how things were set up before.

[asraa]

@asraa Can you take a look at why the verilog tests fail? I don't see any obvious changes from how things were set up before.

Done! The lit runner needed the updated path to the run_verilog command.

[asraa]

Right! There is also some in tools/.bzl now too, but I don't really see a difference in marking these macros as test vs non-test. I think we probably want something like tools/bazel/.bzl and put all of them there?

I'm going to file a follow-up issue for this.

[AlexanderViand-Intel]

Thanks! I just squashed & rebased, LGTM now :)

[AlexanderViand-Intel]

I like this organization, I can't wait for it to be merged!

Crossing my fingers that when it's merged it's a seamless transition for any open PRs :D

Did you already start the internal process or should I update with mod_arith tests and rebase? EDIT: I've left this branch as-is, but the updated/rebased version of this PR is the first commit on the branch src_reorg

[asraa]

Did you already start the internal process or should I update with mod_arith tests and rebase?

I'll continue to rebase this, the internal transforms had to change to import these. Working on it now.