[PATCH] D23299: [PM] Port the always inliner to the new pass manager in a much more minimal and boring form than the old pass manager's version.

[Davide Italiano via llvm-commits]

davide added a subscriber: davide.
davide added a comment.

Some comments.


================
Comment at: lib/Passes/PassRegistry.def:40
@@ -39,2 +39,3 @@
 #endif
+MODULE_PASS("always-inliner", AlwaysInlinerPass())
 MODULE_PASS("constmerge", ConstantMergePass())
----------------
Is there a particular reason why you renamed `always-inline` to `always-inliner`?
I think we should try to keep the names of the passes consistent between old and new PM (unless there's a reason not to)/

================
Comment at: lib/Transforms/IPO/AlwaysInliner.cpp:53
@@ +52,3 @@
+        // FIXME: We really shouldn't be able to fail to inline at this point!
+        Changed |= Inlined;
+      }
----------------
`Changed |= InlineFunction(CS, IFI);` , no?

================
Comment at: lib/Transforms/IPO/AlwaysInliner.cpp:94-97
@@ -65,5 +93,6 @@
                       "Inliner for always_inline functions", false, false)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
 INITIALIZE_PASS_DEPENDENCY(CallGraphWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
+INITIALIZE_PASS_END(AlwaysInlinerLegacyPass, "always-inline",
----------------
I'm not entirely sure here, but are all these dependencies actually needed?

================
Comment at: test/Transforms/Inline/always-inline.ll:4
@@ -3,3 +3,3 @@
 ; Ensure the threshold has no impact on these decisions.
-; RUN: opt < %s -inline-threshold=20000000 -always-inline -S | FileCheck %s
-; RUN: opt < %s -inline-threshold=-20000000 -always-inline -S | FileCheck %s
+; RUN: opt < %s -inline-threshold=20000000 -always-inline -S | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-CALL
+; RUN: opt < %s -inline-threshold=-20000000 -always-inline -S | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-CALL
----------------
These tests modification (at least part of them) can be committed separately, probably.


https://reviews.llvm.org/D23299