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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Passes/PassBuilder.cpp:859
+  OptimizePM.addPass(
+      LoopVectorizePass(PassOpt.DisableUnrollLoops, !PassOpt.LoopVectorize));
 
----------------
hsaito wrote:
> asbirlea wrote:
> > chandlerc wrote:
> > > If I understand correctly, the first parameter isn't about *unrolling* but about interleaving. We should be careful to use that terminology as disabling unrolling is a potentially reasonable thing to add.
> > Yes, that's true. I used the same naming and values as the ones in the old pass manager (including not having a cl option).
> > 
> > 
> > Updated now (also means the old and new pass managers are now inconsistent as far as namings, defaults etc, but it's probably preferable to update the old one to be consistent with this?).
> @chandlerc, you are right about the terminology mess. It is the vectorizer that is "incorrectly" piggybacking on the unroll flag. If we are to start fixing the terminology here, we'd need to create DisableInterleaveLoops. If we are going to that direction, I suggest starting from setting the same value as DisableUnrollLoops so that fixing the terminology mess is NFC.
> 
> Potentially bigger mess is programmer education, to teach that interleaving and unrolling are different, as they often tend to result in identical ASM after instruction reordering --- but I fully agree that we still need to educate them (including compiler writers) to use the appropriate terminology.
> 
> 
> 
I think Alina's patch does a reasonable job of starting to fix this.

I don't think the education problem is that bad. Unrolling and interleaving produce pretty different results in LLVM.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59723