[PATCH] D109234: [PGO] Change ThinLTO test for targets with loop unrolling disabled

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.


================
Comment at: clang/test/CodeGen/pgo-sample-thinlto-summary.c:3
+// RUN: %clang_cc1 -mllvm -debug-pass=Structure -O2 -fno-experimental-new-pass-manager -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO-OLDPM
+// RUN: %clang_cc1 -mllvm -debug-pass=Structure -O2 -fexperimental-new-pass-manager -fdebug-pass-manager -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=SAMPLEPGO
+// RUN: %clang_cc1 -mllvm -debug-pass=Structure -O2 -fexperimental-new-pass-manager -fdebug-pass-manager -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO
----------------
thopre wrote:
> While we now need to explicitely request the old pass manager, the new pass manager is the default so we don't need to be explicit.
For the newPM, remove -mllvm -debug-pass=Structure since it isn't doing anything, and you are getting the printing from -fdebug-pass-manager. Also suggest moving the -fdebug-pass-manager into the position where you currently have  -mllvm -debug-pass=Structure so the printing options are in the equivalent place for both PM invocations.


================
Comment at: clang/test/CodeGen/pgo-sample-thinlto-summary.c:27
+// THINLTO-OLDPM-NOT:       PGOIndirectCallPromotion
+// THINLTO-OLDPM:           Unroll loops
+// THINLTO-OLDPM-NOT:       Unroll loops
----------------
sherwin-dc wrote:
> When printing out passes with the old PM 'Unroll loops' is printed out twice with sample PGO and once with thin LTO.
I looked at the old PM, the first one is the createSimpleLoopUnrollPass that does some full unrolling and peeling of small constant trip count loops. Can you just add a comment above the check for the first Unroll loops here for ThinLTO to note this. It's the second invocation that we delay until the ThinLTO backends.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109234