[PATCH] D59723: [NewPassManager] Adding pass tuning options: loop vectorize.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:288-293
+// Current defaults:
+  : InterleaveOnlyWhenForced(false),
+    VectorizeOnlyWhenForced(false) {}
+// Expected usage of defaults:
+//: InterleaveOnlyWhenForced(!SetLoopsInterleaved),
+//  VectorizeOnlyWhenForced(!RunLoopVectorization) {}
----------------
asbirlea wrote:
> asbirlea wrote:
> > chandlerc wrote:
> > > Not really sure what this commenting structure is trying to convey, but I don't think you want to use commented out code. I'd just write prose in a comment on the method that describes what's going on here.
> > I *really* don't want to leave this as is. It was meant as a discussion point. Sorry for not being clearer.
> > 
> > I wanted to use the "expected defaults", where interleaving and vectorization are enabled by default, but vectorization is disabled by default in the old pass manager.
> > Why is this?
> > [The interleaving is set to false already in the old pass manager, the `DisableUnrollLoops` variable.]
> > 
> > I guess I am asking why are having vectorization disabled by default in the old pass manager, but enabled by default when creating the LoopVectorizePass with no params?
> > 
> > We also have a strange (to me) behavior in `opt.cpp:351`: 
> > - if an opt-specific option to `disable-loop-vectorize` is set, then it's disabled.
> > - Otherwise, if it's explicitly enabled with `-vectorize-loops` or  `-loop-vectorize` (there's two?), then it's enabled.
> > - Otherwise, it assumes it's disabled (that's the default here...), and enables it if ` OptLevel > 1 && SizeLevel < 2`.
> > So I can't really change the default to enable vectorization, without some changes here.
> To make progress here, I moved the explanation to a comment block.
> This way, this patch can go in as is since it doesn't change any defaults, it's just flag refactoring and plumbing. Any change that *does* change the defaults can be a follow up to this patch.
> I guess I am asking why are having vectorization disabled by default in the old pass manager, but enabled by default when creating the LoopVectorizePass with no params?

Quite confusingly, clang enables loop vectorization in the legacy pipeline builder by default again. That is, `opt -O3` disables LoopVectorize, but `clang -O3` has it enabled. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59723