This is an archive of the discontinued LLVM Phabricator instance.

[llvm-mca] Rename Backend to Pipeline. NFC.
ClosedPublic

Authored by mattd on Jun 22 2018, 10:56 AM.

Details

Summary

This change renames the Backend and BackendPrinter to Pipeline and PipelinePrinter respectively.
Variables and comments have also been updated to reflect this change.

The reason for this rename, is to be slightly more correct about what MCA is modeling. MCA models a Pipeline, which implies some logical sequence of stages.

Diff Detail

Repository
rL LLVM

Event Timeline

mattd created this revision.Jun 22 2018, 10:56 AM
courbet accepted this revision.Jun 24 2018, 11:06 PM
This revision is now accepted and ready to land.Jun 24 2018, 11:06 PM
andreadb accepted this revision.Jun 25 2018, 3:20 AM

Hi Matt,

The patch looks good modulo the minor nits reported below.

Thanks!
-Andrea

tools/llvm-mca/HWEventListener.h
34 ↗(On Diff #152513)

Please change Scheduler to ExecuteState. Also change components into 'stages'.

tools/llvm-mca/Instruction.h
49 ↗(On Diff #152513)

"backend" was correct here. Please revert this change.

267 ↗(On Diff #152513)

This comment is no longer valid and should be rewritten. We shouldn't just replace backend with pipeline in this sentence.

tools/llvm-mca/Pipeline.cpp
11 ↗(On Diff #152513)

Same.

tools/llvm-mca/Pipeline.h
11 ↗(On Diff #152513)

Remove the OoO part. I don't think it makes much sense when applied to pipeline.

28 ↗(On Diff #152513)

Same. This sentence should probably be rewritten.

30 ↗(On Diff #152513)

The default pipeline currently emulates an OoO execution.

tools/llvm-mca/README.txt
10 ↗(On Diff #152513)

Please revert this line. "Backend" was correct.

21 ↗(On Diff #152513)

Please revert this part too.

347 ↗(On Diff #152513)

I think this should still say 'out-of-order backend'.

tools/llvm-mca/RetireControlUnit.h
29 ↗(On Diff #152513)

s/pipeline/backend

courbet added inline comments.Jun 25 2018, 3:54 AM
tools/llvm-mca/Pipeline.h
11 ↗(On Diff #152513)

I would keep it, just to make it clear to the reader that it's not strictly a linear DAG.

andreadb added inline comments.Jun 25 2018, 4:21 AM
tools/llvm-mca/Pipeline.h
11 ↗(On Diff #152513)

The Pipeline (as a container of stages) is not out-of-order. Instead, the hardware backend is OoO. I don't think it is semantically correct to say that the Pipeline is out-of-order.
We could rephrase this as "a Stage pipeline for an OoO processor". Or something similar.

courbet added inline comments.Jun 25 2018, 4:28 AM
tools/llvm-mca/Pipeline.h
11 ↗(On Diff #152513)

SG

mattd marked 10 inline comments as done.Jun 25 2018, 9:48 AM

Thanks for the feedback. I'll make the changes and land the patch.

tools/llvm-mca/Pipeline.h
11 ↗(On Diff #152513)

That's a very interesting distinction I never really considered, but do agree with. It sounds like we should probably define the terms (backend,pipeline,component) formally somewhere, perhaps as a separate commit to the mythical documentation that describes the stage and pipeline designs :)

This revision was automatically updated to reflect the committed changes.
llvm/trunk/tools/llvm-mca/BackendPrinter.h