[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