[PATCH] D72386: [ThinLTO] pass UnrollLoops/VectorizeLoop/VectorizeSLP in CGOpts down to pass builder in ltobackend

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

I think the old PM has some issues as well, although more in the opposite direction. The code in LTOBackend.cpp:runOldPMPasses is simply hardcoding loop and slp vectorization flags to true, but presumably should use whatever is passed through the lto Config as well. Not sure about LoopsInterleaved, ah looks like it is set to the value of the internal option which is enabled by default. That and DisableUnrollLoops should also use whatever is passed through here (similar to how they are set in clang BackendUtil.cpp when configuring the old PM).



================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1443
   Conf.SampleProfile = std::move(SampleProfile);
+  Conf.PTO.LoopUnrolling = CGOpts.UnrollLoops;
+  // For historical reasons, loop interleaving is set to mirror setting for loop
----------------
Looks like the PTO constructor sets this to true, so we were getting lucky here.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1446
+  // unrolling.
+  Conf.PTO.LoopInterleaving = CGOpts.UnrollLoops;
+  Conf.PTO.LoopVectorization = CGOpts.VectorizeLoop;
----------------
Looks like this only affects the values passed to LoopVectorizePass. Is there a way to test that it is being enabled properly now?


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1447
+  Conf.PTO.LoopInterleaving = CGOpts.UnrollLoops;
+  Conf.PTO.LoopVectorization = CGOpts.VectorizeLoop;
+  Conf.PTO.SLPVectorization = CGOpts.VectorizeSLP;
----------------
Ditto.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1448
+  Conf.PTO.LoopVectorization = CGOpts.VectorizeLoop;
+  Conf.PTO.SLPVectorization = CGOpts.VectorizeSLP;
 
----------------
Is this enabled by default at -O3? I looked at clang but couldn't tell where that would be happening. If so, then llvm/test/Other/new-pm-thinlto-defaults.ll should be fixed to check that it is added to the postlink pipeline at -O3 to make sure we don't regress here again.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D72386