This is an archive of the discontinued LLVM Phabricator instance.

[llvm] Moved InlineSizeEstimatorAnalysis test to .ll
ClosedPublic

Authored by mtrofin on Jul 15 2020, 4:04 PM.

Diff Detail

Event Timeline

mtrofin created this revision.Jul 15 2020, 4:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2020, 4:04 PM
mehdi_amini accepted this revision.Jul 16 2020, 9:47 AM

Awesome! Thanks

llvm/test/Transforms/Inline/ML/Inputs/size-estimator.ll
28

I'm curious why you have the IR as a separate input file instead of having it inside the lit test? Is this because you wanted to not duplicate the IR in the two tests?
Merging the two tests with different check-prefix was reducing readability?

This revision is now accepted and ready to land.Jul 16 2020, 9:47 AM
mtrofin marked 2 inline comments as done.Jul 16 2020, 9:51 AM
mtrofin added inline comments.
llvm/test/Transforms/Inline/ML/Inputs/size-estimator.ll
28

It's because I needed 2 tests, one for the 'default' build case (no TF available/enabled), the other for the TF case. This is because my understanding is that the REQUIRES statement is per file. Then I wanted to avoid duplication. If it's the preferred style, I can copy the IR in each of the 2 tests.

mehdi_amini added inline comments.Jul 16 2020, 12:13 PM
llvm/test/Transforms/Inline/ML/Inputs/size-estimator.ll
28

Ah, I missed that this was because of the "REQUIRE", makes sense.

I would have duplicated the IR in the two files, but that's fine as is as well I think.

This revision was automatically updated to reflect the committed changes.
mtrofin marked an inline comment as done.
llvm/test/lit.site.cfg.py.in