[PATCH] D68535: Fix loop unrolling initialization in the new pass manager

Sat Oct 5 23:04:10 PDT 2019

chandlerc accepted this revision.
chandlerc added a reviewer: asbirlea.
chandlerc added a comment.
This revision is now accepted and ready to land.

Adding Alina so she is aware of the change and can comment if she spots anything I'm missing...

I think this is fine to go in as-is to fix the immediate issues. It'd be good to find a way to write an LLVM test for the unrolling functionality change and the unroll-and-jam change I comment on below. But I'm OK w/ those landing in follow-up changes, no need to hold up the simple fix here.

================
Comment at: clang/test/Misc/loop-opt-setup.c:2
+// RUN: %clang -O1 -fexperimental-new-pass-manager -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+// RUN: %clang -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+extern int a[16];
----------------
I'd use `-fno-experimental-new-pass-manager` here so we don't lose coverage when flipping defaults.

================
Comment at: llvm/lib/Passes/PassBuilder.cpp:953
   // We do UnrollAndJam in a separate LPM to ensure it happens before unroll
-  if (EnableUnrollAndJam) {
+  if (EnableUnrollAndJam && PTO.LoopUnrolling) {
     OptimizePM.addPass(
----------------
We don't have a test for this sadly.... Not sure the best way to add one.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68535