[PATCH] D83918: [llvm] Moved InlineSizeEstimatorAnalysis test to .ll

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 16 12:13:12 PDT 2020


mehdi_amini added inline comments.


================
Comment at: llvm/test/Transforms/Inline/ML/Inputs/size-estimator.ll:28
+  ret i32 %2
+}
----------------
mtrofin wrote:
> mehdi_amini wrote:
> > 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?
> 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.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83918





More information about the llvm-commits mailing list