[PATCH] D87132: [Partial Inliner] Compute intrinsic cost through TTI

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 6 07:25:22 PDT 2020


fhahn added a comment.

It would be great if you could upload the patch with more context in the diff (see https://llvm.org/docs/Phabricator.html#phabricator-request-review-web, e.g. use `git show HEAD -U999999 > mypatch.patch` if you use `git` to generate the diff).



================
Comment at: llvm/test/Transforms/PartialInlining/intrinsic-call-cost.ll:1
+; Test case is expected to get through with out assert fail.
+;   assert(OutlinedFunctionCost >= Cloner.OutlinedRegionCost &&
----------------
Thanks for adding the test case. Please also use `FileCheck` to make sure the resulting IR is sensible.


================
Comment at: llvm/test/Transforms/PartialInlining/intrinsic-call-cost.ll:11
+  tail call void @delete_variable_part()
+  unreachable
+}
----------------
Can we just return here instead of `unreachable`?


================
Comment at: llvm/test/Transforms/PartialInlining/intrinsic-call-cost.ll:14
+
+define dso_local void @delete_variable_part() local_unnamed_addr {
+bb:
----------------
nit: `dso_local` / `local_unnamed_addr` may be not needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87132



More information about the llvm-commits mailing list