[llvm] r190916 - Lift alignment restrictions for load/store folding on VINSERTF128/VEXTRACTF128. Fixes PR17268. [PATCH]

[Arnold Schwaighofer]

Oh, I see. We used to use the cl::opt flag for communicating settings from the front-end.

Now, we want it to override settings from the front end (be it opt or clang):

So we want something like:

--- a/lib/Transforms/IPO/PassManagerBuilder.cpp
+++ b/lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -200,7 +200,8 @@ void PassManagerBuilder::populateModulePassManager(PassManagerBase &MPM) {
   MPM.add(createLoopIdiomPass());             // Recognize idioms like memset.
   MPM.add(createLoopDeletionPass());          // Delete dead loops
 
-  if (!LateVectorize && LoopVectorize)
+  if (!LateVectorize && LoopVectorize &&
+      (RunLoopVectorization.getPosition() ? RunLoopVectorization : true))
       MPM.add(createLoopVectorizePass(DisableUnrollLoops));
 
   if (!DisableUnrollLoops)
@@ -246,7 +247,8 @@ void PassManagerBuilder::populateModulePassManager(PassManagerBase &MPM) {
   // As an experimental mode, run any vectorization passes in a separate
   // pipeline from the CGSCC pass manager that runs iteratively with the
   // inliner.
-  if (LateVectorize && LoopVectorize) {
+  if (LateVectorize && LoopVectorize &&
+      (RunLoopVectorization.getPosition() ? RunLoopVectorization : true)) {
     // FIXME: This is a HACK! The inliner pass above implicitly creates a CGSCC
     // pass manager that we are specifically trying to avoid. To prevent this
     // we must insert a no-op module pass to reset the pass manager.

And similar for the slp vectorizer flag.

On Nov 21, 2013, at 11:28 AM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
>> From: "Nadav Rotem" <nrotem at apple.com>
>> To: "Arnold Schwaighofer" <aschwaighofer at apple.com>
>> Cc: "Hal Finkel" <hfinkel at anl.gov>, "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
>> Sent: Thursday, November 21, 2013 11:02:32 AM
>> Subject: Re: [llvm] r190916 - Lift alignment restrictions for load/store folding on VINSERTF128/VEXTRACTF128. Fixes
>> PR17268. [PATCH]
>> 
>> I don’t have an opinion on this. :)
> 
> Okay; I think that the issue is just that we need a way to turn the things off. The command-line options in the PMB change the default, and this logic in opt overrides those defaults. So we should either not set these things in opt, or we should provide cl-opts in opt to disable that behavior.
> 
> Arnold, thoughts?
> 
> -Hal
> 
>> 
>> 
>> 
>> On Nov 21, 2013, at 6:39 AM, Arnold Schwaighofer <
>> aschwaighofer at apple.com > wrote:
>> 
>> 
>> 
>> 
>> On Nov 21, 2013, at 7:29 AM, Hal Finkel < hfinkel at anl.gov > wrote:
>> 
>> 
>> 
>> ----- Original Message -----
>> 
>> 
>> From: "Pekka Jääskeläinen" < pekka.jaaskelainen at tut.fi >
>> To: "Craig Topper" < craig.topper at gmail.com >
>> Cc: "Commit Messages and Patches for LLVM" < llvm-commits at cs.uiuc.edu
>>> 
>> Sent: Thursday, November 21, 2013 4:15:12 AM
>> Subject: Re: [llvm] r190916 - Lift alignment restrictions for
>> load/store folding on VINSERTF128/VEXTRACTF128. Fixes
>> PR17268. [PATCH]
>> 
>> Hi,
>> 
>> The attached patch reverts the line that forces the SLPVectorizer
>> always on in opt. Passes "make check" in LLVM 3.4.
>> 
>> Nadav, Do you have an opinion about this? Generally, I'd say to
>> disable it in opt (since we already have a command-line flag in the
>> PMB to enable it). On the other hand, we also seem to force on the
>> loop vectorizer (in a way that matches Clang's defaults) in the line
>> above.
>> 
>> Yes, I added this when I moved the logic at which optimization level
>> to optimize out of the pass manager.
>> 
>> We should do the same for the slp vectorizer. I think it is a good
>> thing that opt -Owhatever tries to match clang -Owhatever as close
>> as possible.
>> 
>> 
>> 
>> 
>> 
>> 
>> -Hal
>> 
>> 
>> 
>> 
>> OK to commit?
>> 
>> On 11/19/2013 09:51 AM, Nick Lewycky wrote:
>> 
>> 
>> On 17 September 2013 20:55, Craig Topper < craig.topper at gmail.com
>> < mailto:craig.topper at gmail.com >> wrote:
>> 
>> Author: ctopper
>> Date: Tue Sep 17 22:55:53 2013
>> New Revision: 190916
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=190916&view=rev
>> Log:
>> Lift alignment restrictions for load/store folding on
>> VINSERTF128/VEXTRACTF128. Fixes PR17268.
>> 
>> 
>> Modified: llvm/trunk/tools/opt/opt.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=190916&r1=190915&r2=190916&view=diff
>> ==============================================================================
>> --- llvm/trunk/tools/opt/opt.cpp (original)
>> +++ llvm/trunk/tools/opt/opt.cpp Tue Sep 17 22:55:53 2013
>> @@ -462,6 +462,7 @@ static void AddOptimizationPasses(PassMa
>> DisableLoopUnrolling :
>> OptLevel == 0;
>> 
>> Builder.LoopVectorize = OptLevel > 1 && SizeLevel < 2;
>> + Builder.SLPVectorize = true;
>> 
>> 
>> This doesn't match the commit log. Was this intentional?
>> 
>> Nick
>> 
>> --
>> Pekka
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> --
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory