This is an archive of the discontinued LLVM Phabricator instance.

Differential D75354

Add InjectTLIMappings pass to new pass manager
ClosedPublic

Authored by masoud.ataei on Feb 28 2020, 8:07 AM.

Details

Reviewers
fpetrogalli
nemanjai
Whitney
pjeeva01
Commits
rG9ed0612ccaf0: Add InjectTLIMappings pass to new pass manager
Summary

This pass is created in https://reviews.llvm.org/rGd6de5f12d485a85504bc99d384a85634574a27e2 and tested for new and legacy pass manager but never added to new pass manager pipeline. I am adding it to new pass manager pipeline.

This pass is get used in Vector Function Database (VFDatabase) and without this pass in new pass manager pipeline, none of the vector libraries are working with new pass manager.

Related passes:
https://reviews.llvm.org/rG66c120f02560ef528a60924104ead66f330190f1
https://reviews.llvm.org/D74944

Diff Detail

Event Timeline

masoud.ataei created this revision.Feb 28 2020, 8:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2020, 8:07 AM
Herald added subscribers: llvm-commits, dexonsmith, steven_wu, hiraditya. · View Herald Transcript
masoud.ataei edited the summary of this revision. (Show Details)Feb 28 2020, 8:13 AM
Whitney added a comment.Feb 28 2020, 8:29 AM

The change LGTM, how have it been tested?

fpetrogalli added a comment.Feb 28 2020, 8:35 AM

Hi @masoud.ataei , thank you for taking care of this.

Does your patch fix this bug report? https://bugs.llvm.org/show_bug.cgi?id=44770

If so, can you mention it in the commit message and update the bug report too?

In term of testing, I think that the code in bug report could be used to show that vectorization happens with the new pass manager too?

Kind regards,

Francesco

pjeeva01 added a comment.Feb 28 2020, 8:46 AM

Since the InjectTLIMappings pass impacts mappings of scalar math functions to their vector counterparts in all three of the currently supported vector libraries, I would like to suggest adding a test case for each one of them. In other words, testing in the presence of -fveclib=Accelerate, -fveclib=SVML, and -fveclib=MASSV libraries.

fpetrogalli added a comment.Feb 28 2020, 8:58 AM
In D75354#1898245, @pjeeva01 wrote:

Since the InjectTLIMappings pass impacts mappings of scalar math functions to their vector counterparts in all three of the currently supported vector libraries, I would like to suggest adding a test case for each one of them. In other words, testing in the presence of -fveclib=Accelerate, -fveclib=SVML, and -fveclib=MASSV libraries.

Isn't the follwing test already doing that via the -passes=inject-tli-mappings option?

rapet01@man-08:~/projects/upstream-clang/llvm-project (master)$ cat llvm/test/Transforms/Util/add-TLI-mappings.ll
; RUN: opt -vector-library=SVML       -inject-tli-mappings        -S < %s | FileCheck %s  --check-prefixes=COMMON,SVML
; RUN: opt -vector-library=SVML       -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,SVML
; RUN: opt -vector-library=MASSV      -inject-tli-mappings        -S < %s | FileCheck %s  --check-prefixes=COMMON,MASSV
; RUN: opt -vector-library=MASSV      -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,MASSV
; RUN: opt -vector-library=Accelerate -inject-tli-mappings        -S < %s | FileCheck %s  --check-prefixes=COMMON,ACCELERATE
; RUN: opt -vector-library=Accelerate -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,ACCELERATE

...

I guess all we need to do then is to make sure that the pass is executed in the new pass manager pipelines, which is what this patch is doing.

fpetrogalli added a comment.Feb 28 2020, 9:00 AM

My bad, vectorization is what needs to be tested, not the generation of the attribute.

Although, what should really be tested, is the fact that calls decorated with the attribute are in fact vectorized, which is unrelated to the fact that the attribute is attached to a MASSV or Accelerate function call.

masoud.ataei added a comment.Feb 28 2020, 9:54 AM

@Whitney @fpetrogalli @pjeeva01:
Thank you guys for those comments. About the test, as @fpetrogalli said test for attribute generation is available llvm/test/Transforms/Util/add-TLI-mappings.ll for new pass manager. And I added the test to see if it is invoked in pipeline here in this patch. As @pjeeva01 said, I need to add a test to see if the vectorized function is called in presence of the vector library option.

@fpetrogalli:
About that bug report, yes this fixes that bug.

$ cat veclib.c 
void foo(float *a) {
  for (int i = 0; i != 1024; ++i)
    a[i] = __builtin_expf(a[i]);
}

$ clang -O3 -S -o - veclib.c -fveclib=Accelerate -fexperimental-new-pass-manager|grep vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
	bl vexpf
fpetrogalli accepted this revision.Feb 28 2020, 11:05 AM

As @pjeeva01 said, I need to add a test to see if the vectorized function is called in presence of the vector library option.

These tests are already there, those are the tests that use the -vector-library option:

LoopVectorize/X86/svml-calls.ll:; RUN: opt -vector-library=SVML -loop-vectorize -force-vector-width=4 -force-vector-interleave=1 -mattr=avx -S < %s | FileCheck %s
LoopVectorize/X86/veclib-calls.ll:; RUN: opt < %s -vector-library=Accelerate -loop-vectorize -S | FileCheck %s
LoopVectorize/X86/TLI-to-vfabi-attribute.ll~:; RUN: opt -vector-library=SVML -loop-vectorize -force-vector-width=4 -force-vector-interleave=1 -mattr=avx -S < %s | FileCheck %s
LoopVectorize/X86/svml-calls-finite.ll:; RUN: opt -vector-library=SVML -loop-vectorize -S < %s | FileCheck %s
LoopVectorize/PowerPC/widened-massv-call.ll:; RUN: opt < %s -vector-library=MASSV -force-vector-interleave=1 \
LoopVectorize/PowerPC/massv-nobuiltin.ll:; RUN: opt -vector-library=MASSV -loop-vectorize -force-vector-interleave=1 -S < %s | FileCheck %s
LoopVectorize/PowerPC/massv-unsupported.ll:; RUN: opt -vector-library=MASSV -loop-vectorize -force-vector-interleave=1 -S < %s | FileCheck %s
LoopVectorize/PowerPC/massv-altivec.ll:; RUN: opt -vector-library=MASSV -loop-vectorize -force-vector-interleave=1 -mattr=-altivec -S < %s | FileCheck %s
LoopVectorize/PowerPC/massv-calls.ll:; RUN: opt -vector-library=MASSV -loop-vectorize -force-vector-interleave=1 -S < %s | FileCheck %s

As the tests are showing, we already know that the library calls are inserted when vectorizing: InjectTLIMappings is adding the attribute vector-function-abi-variant to the call sites, and the LoopVectorizer is using it for vectorization.

What is missing in this picture (LoopVectorize with vector-function-abi-variant attribute) are the test that make sure vectorization happens when -vector-library is not used and only the attribute is present. That deserves a separate patch that is on my todo list. Of course, anyone is _very_ welcome to come up with a patch, which I happily review! :)

Thank you, this LGTM now.

This revision is now accepted and ready to land.Feb 28 2020, 11:05 AM
jsji set the repository for this revision to rG LLVM Github Monorepo.Mar 2 2020, 8:27 AM
jsji updated this revision to Diff 255338.Apr 6 2020, 7:41 AM
jsji retitled this revision from Add InjectTLIMappings pass to new pass manager to Add InjectTLIMappings pass to new pass manager.
jsji added a subscriber: jsji.

Rebased.

Herald added a subscriber: • wuzish. · View Herald TranscriptApr 6 2020, 7:41 AM
Harbormaster completed remote builds in B51946: Diff 255338.Apr 6 2020, 9:12 AM
Closed by commit rG9ed0612ccaf0: Add InjectTLIMappings pass to new pass manager (authored by masoud.ataei, committed by kamaub). · Explain WhyApr 6 2020, 11:27 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 255424

llvm/lib/Passes/PassBuilder.cpp

Show First 20 Lines • Show All 966 Lines • ▼ Show 20 Lines OptimizePM.addPass(createFunctionToLoopPassAdaptor(
LoopRotatePass(), EnableMSSALoopDependency, DebugLogging)); LoopRotatePass(), EnableMSSALoopDependency, DebugLogging));
// Distribute loops to allow partial vectorization. I.e. isolate dependences // Distribute loops to allow partial vectorization. I.e. isolate dependences
// into separate loop that would otherwise inhibit vectorization. This is // into separate loop that would otherwise inhibit vectorization. This is
// currently only performed for loops marked with the metadata // currently only performed for loops marked with the metadata
// llvm.loop.distribute=true or when -enable-loop-distribute is specified. // llvm.loop.distribute=true or when -enable-loop-distribute is specified.
OptimizePM.addPass(LoopDistributePass()); OptimizePM.addPass(LoopDistributePass());
// Populates the VFABI attribute with the scalar-to-vector mappings
// from the TargetLibraryInfo.
OptimizePM.addPass(InjectTLIMappings());
// Now run the core loop vectorizer. // Now run the core loop vectorizer.
OptimizePM.addPass(LoopVectorizePass( OptimizePM.addPass(LoopVectorizePass(
LoopVectorizeOptions(!PTO.LoopInterleaving, !PTO.LoopVectorization))); LoopVectorizeOptions(!PTO.LoopInterleaving, !PTO.LoopVectorization)));
// Enhance/cleanup vector code. // Enhance/cleanup vector code.
OptimizePM.addPass(VectorCombinePass()); OptimizePM.addPass(VectorCombinePass());
OptimizePM.addPass(EarlyCSEPass()); OptimizePM.addPass(EarlyCSEPass());
▲ Show 20 Lines • Show All 1,521 Lines • Show Last 20 Lines

llvm/test/Other/new-pm-defaults.ll

Show First 20 Lines • Show All 239 Lines • ▼ Show 20 Lines
; CHECK-EP-VECTORIZER-START-NEXT: Running pass: NoOpFunctionPass ; CHECK-EP-VECTORIZER-START-NEXT: Running pass: NoOpFunctionPass
; CHECK-EXT: Running pass: {{.*}}::Bye on foo ; CHECK-EXT: Running pass: {{.*}}::Bye on foo
; CHECK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass ; CHECK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass
; CHECK-O-NEXT: Starting llvm::Function pass manager run. ; CHECK-O-NEXT: Starting llvm::Function pass manager run.
; CHECK-O-NEXT: Running pass: LoopSimplifyPass ; CHECK-O-NEXT: Running pass: LoopSimplifyPass
; CHECK-O-NEXT: Running pass: LCSSAPass ; CHECK-O-NEXT: Running pass: LCSSAPass
; CHECK-O-NEXT: Finished llvm::Function pass manager run. ; CHECK-O-NEXT: Finished llvm::Function pass manager run.
; CHECK-O-NEXT: Running pass: LoopDistributePass ; CHECK-O-NEXT: Running pass: LoopDistributePass
; CHECK-O-NEXT: Running pass: InjectTLIMappings
; CHECK-O-NEXT: Running pass: LoopVectorizePass ; CHECK-O-NEXT: Running pass: LoopVectorizePass
; CHECK-O-NEXT: Running analysis: BlockFrequencyAnalysis ; CHECK-O-NEXT: Running analysis: BlockFrequencyAnalysis
; CHECK-O-NEXT: Running analysis: BranchProbabilityAnalysis ; CHECK-O-NEXT: Running analysis: BranchProbabilityAnalysis
; CHECK-O-NEXT: Running pass: VectorCombinePass ; CHECK-O-NEXT: Running pass: VectorCombinePass
; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running pass: EarlyCSEPass
; CHECK-O-NEXT: Running pass: LoopLoadEliminationPass ; CHECK-O-NEXT: Running pass: LoopLoadEliminationPass
; CHECK-O-NEXT: Running analysis: LoopAccessAnalysis ; CHECK-O-NEXT: Running analysis: LoopAccessAnalysis
; CHECK-O-NEXT: Running pass: InstCombinePass ; CHECK-O-NEXT: Running pass: InstCombinePass
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines

llvm/test/Other/new-pm-thinlto-defaults.ll

Show First 20 Lines • Show All 209 Lines • ▼ Show 20 Lines
; CHECK-POSTLINK-O-NEXT: Running pass: LowerConstantIntrinsicsPass ; CHECK-POSTLINK-O-NEXT: Running pass: LowerConstantIntrinsicsPass
; CHECK-EXT: Running pass: {{.*}}::Bye ; CHECK-EXT: Running pass: {{.*}}::Bye
; CHECK-POSTLINK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass ; CHECK-POSTLINK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass
; CHECK-POSTLINK-O-NEXT: Starting llvm::Function pass manager run ; CHECK-POSTLINK-O-NEXT: Starting llvm::Function pass manager run
; CHECK-POSTLINK-O-NEXT: Running pass: LoopSimplifyPass ; CHECK-POSTLINK-O-NEXT: Running pass: LoopSimplifyPass
; CHECK-POSTLINK-O-NEXT: Running pass: LCSSAPass ; CHECK-POSTLINK-O-NEXT: Running pass: LCSSAPass
; CHECK-POSTLINK-O-NEXT: Finished llvm::Function pass manager run ; CHECK-POSTLINK-O-NEXT: Finished llvm::Function pass manager run
; CHECK-POSTLINK-O-NEXT: Running pass: LoopDistributePass ; CHECK-POSTLINK-O-NEXT: Running pass: LoopDistributePass
; CHECK-POSTLINK-O-NEXT: Running pass: InjectTLIMappings
; CHECK-POSTLINK-O-NEXT: Running pass: LoopVectorizePass ; CHECK-POSTLINK-O-NEXT: Running pass: LoopVectorizePass
; CHECK-POSTLINK-O-NEXT: Running analysis: BlockFrequencyAnalysis ; CHECK-POSTLINK-O-NEXT: Running analysis: BlockFrequencyAnalysis
; CHECK-POSTLINK-O-NEXT: Running analysis: BranchProbabilityAnalysis ; CHECK-POSTLINK-O-NEXT: Running analysis: BranchProbabilityAnalysis
; CHECK-POSTLINK-O-NEXT: Running pass: VectorCombinePass ; CHECK-POSTLINK-O-NEXT: Running pass: VectorCombinePass
; CHECK-POSTLINK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-POSTLINK-O-NEXT: Running pass: EarlyCSEPass
; CHECK-POSTLINK-O-NEXT: Running pass: LoopLoadEliminationPass ; CHECK-POSTLINK-O-NEXT: Running pass: LoopLoadEliminationPass
; CHECK-POSTLINK-O-NEXT: Running analysis: LoopAccessAnalysis ; CHECK-POSTLINK-O-NEXT: Running analysis: LoopAccessAnalysis
; CHECK-POSTLINK-O-NEXT: Running pass: InstCombinePass ; CHECK-POSTLINK-O-NEXT: Running pass: InstCombinePass
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines

llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll

Show First 20 Lines • Show All 179 Lines • ▼ Show 20 Lines
; CHECK-O-NEXT: Running pass: LowerConstantIntrinsicsPass ; CHECK-O-NEXT: Running pass: LowerConstantIntrinsicsPass
; CHECK-EXT: Running pass: {{.*}}::Bye ; CHECK-EXT: Running pass: {{.*}}::Bye
; CHECK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass ; CHECK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass
; CHECK-O-NEXT: Starting {{.*}}Function pass manager run ; CHECK-O-NEXT: Starting {{.*}}Function pass manager run
; CHECK-O-NEXT: Running pass: LoopSimplifyPass ; CHECK-O-NEXT: Running pass: LoopSimplifyPass
; CHECK-O-NEXT: Running pass: LCSSAPass ; CHECK-O-NEXT: Running pass: LCSSAPass
; CHECK-O-NEXT: Finished {{.*}}Function pass manager run ; CHECK-O-NEXT: Finished {{.*}}Function pass manager run
; CHECK-O-NEXT: Running pass: LoopDistributePass ; CHECK-O-NEXT: Running pass: LoopDistributePass
; CHECK-O-NEXT: Running pass: InjectTLIMappings
; CHECK-O-NEXT: Running pass: LoopVectorizePass ; CHECK-O-NEXT: Running pass: LoopVectorizePass
; CHECK-O-NEXT: Running pass: VectorCombinePass ; CHECK-O-NEXT: Running pass: VectorCombinePass
; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running pass: EarlyCSEPass
; CHECK-O-NEXT: Running pass: LoopLoadEliminationPass ; CHECK-O-NEXT: Running pass: LoopLoadEliminationPass
; CHECK-O-NEXT: Running analysis: LoopAccessAnalysis ; CHECK-O-NEXT: Running analysis: LoopAccessAnalysis
; CHECK-O-NEXT: Running pass: InstCombinePass ; CHECK-O-NEXT: Running pass: InstCombinePass
; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
; CHECK-O2-NEXT: Running pass: SLPVectorizerPass ; CHECK-O2-NEXT: Running pass: SLPVectorizerPass
▲ Show 20 Lines • Show All 88 Lines • Show Last 20 Lines

llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll

Show First 20 Lines • Show All 190 Lines • ▼ Show 20 Lines
; CHECK-O-NEXT: Running pass: LowerConstantIntrinsicsPass ; CHECK-O-NEXT: Running pass: LowerConstantIntrinsicsPass
; CHECK-EXT: Running pass: {{.*}}::Bye ; CHECK-EXT: Running pass: {{.*}}::Bye
; CHECK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass ; CHECK-O-NEXT: Running pass: FunctionToLoopPassAdaptor<{{.*}}LoopRotatePass
; CHECK-O-NEXT: Starting {{.*}}Function pass manager run ; CHECK-O-NEXT: Starting {{.*}}Function pass manager run
; CHECK-O-NEXT: Running pass: LoopSimplifyPass ; CHECK-O-NEXT: Running pass: LoopSimplifyPass
; CHECK-O-NEXT: Running pass: LCSSAPass ; CHECK-O-NEXT: Running pass: LCSSAPass
; CHECK-O-NEXT: Finished {{.*}}Function pass manager run ; CHECK-O-NEXT: Finished {{.*}}Function pass manager run
; CHECK-O-NEXT: Running pass: LoopDistributePass ; CHECK-O-NEXT: Running pass: LoopDistributePass
; CHECK-O-NEXT: Running pass: InjectTLIMappings
; CHECK-O-NEXT: Running pass: LoopVectorizePass ; CHECK-O-NEXT: Running pass: LoopVectorizePass
; CHECK-O-NEXT: Running pass: VectorCombinePass ; CHECK-O-NEXT: Running pass: VectorCombinePass
; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running pass: EarlyCSEPass
; CHECK-O-NEXT: Running pass: LoopLoadEliminationPass ; CHECK-O-NEXT: Running pass: LoopLoadEliminationPass
; CHECK-O-NEXT: Running analysis: LoopAccessAnalysis ; CHECK-O-NEXT: Running analysis: LoopAccessAnalysis
; CHECK-O-NEXT: Running pass: InstCombinePass ; CHECK-O-NEXT: Running pass: InstCombinePass
; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
; CHECK-O2-NEXT: Running pass: SLPVectorizerPass ; CHECK-O2-NEXT: Running pass: SLPVectorizerPass
▲ Show 20 Lines • Show All 58 Lines • Show Last 20 Lines