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

[Hal Finkel]

----- Original Message -----
> From: "Arnold Schwaighofer" <aschwaighofer at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>, "Nadav Rotem" <nrotem at apple.com>
> Sent: Thursday, November 21, 2013 11:54:05 AM
> Subject: Re: [llvm] r190916 - Lift alignment restrictions for load/store folding on VINSERTF128/VEXTRACTF128. Fixes
> PR17268. [PATCH]
> 
> 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:

Yes. Although we may want to check for getNumOccurrences() > 0 instead of getPosition() just to be clearer.

 -Hal

> 
> --- 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
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory