[NFC] Fixing build warning with llvm-mca

[plotfi]

Summary

Warning is generated because the range based for-loop is always copying by value but uses a ref &:

[4083/4581] Building CXX object tools/llvm-mca/CMakeFiles/llvm-mca.dir/Views/InstructionInfoView.cpp.o
/path/to/Projects/llvm-project/llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:96:20: warning: loop variable 'I' is always a copy because the range of type 'detail::zippy<detail::zip_shortest, ArrayRef<MCInst>, MutableArrayRef<InstructionInfoViewData> &>' does not return a reference [-Wrange-loop-analysis]
  for (const auto &I : zip(getSource(), IIVD)) {
                   ^
/path/to/Projects/llvm-project/llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:96:8: note: use non-reference type 'std::__1::tuple<const llvm::MCInst &, llvm::mca::InstructionInfoView::InstructionInfoViewData &>'
  for (const auto &I : zip(getSource(), IIVD)) {
       ^~~~~~~~~~~~~~~
1 warning generated.

[plotfi]

@wolfgangp I get this warning on macOS. I noticed it wasn't fixed in https://github.com/llvm/llvm-project/commit/c6e8f81410a2942b5abd112aa6e468268e01d946#diff-67f7be4aff998787c9c4efcb95e5d0dc225e1dcd37b9b005a3db5493f02efdc3 so I posted this small diff.

[andreadb]

LGTM.

For the record, that warning was firstly introduced by another commit which was meant to be an NFC refactoring
in preparation for the most recent work that added support for the generation of JSON files.

commit cf6adecd6a8718ee2737ca55e4cd938364b984cc
[llvm-mca][NFC] Refactor views to separate data collection from printing.

That change was reviewed by @lebedev.ri.
Using zip may be sub-optimal because it introduces copies of every MCInst in source.
While it is true that our typical source size is often less than 16, there may be some
rare cases where the source is much larger.
I am not a big fan of using zip in this particular case. However the cost of using zip is probably negligible.

[lebedev.ri]

In D95321#2519352, @andreadb wrote:

LGTM.

For the record, that warning was firstly introduced by another commit which was meant to be an NFC refactoring
in preparation for the most recent work that added support for the generation of JSON files.

commit cf6adecd6a8718ee2737ca55e4cd938364b984cc
[llvm-mca][NFC] Refactor views to separate data collection from printing.

That change was reviewed by @lebedev.ri.

Using zip may be sub-optimal because it introduces copies of every MCInst in source.

Citation needed.
Even the warning says otherwise.

While it is true that our typical source size is often less than 16, there may be some
rare cases where the source is much larger.
I am not a big fan of using zip in this particular case. However the cost of using zip is probably negligible.

[lebedev.ri]

BTW, https://reviews.llvm.org/D86644?id=318269 likely added a few more of these bugs.

[andreadb]

In D95321#2519375, @lebedev.ri wrote:

BTW, https://reviews.llvm.org/D86644?id=318269 likely added a few more of these bugs.

Oh I see. I completely missed those changes from auto I to const auto &I.
That's why I thought it was from a previous commit.
Apologies for that.

[andreadb]

In D95321#2519369, @lebedev.ri wrote:

In D95321#2519352, @andreadb wrote:

LGTM.

For the record, that warning was firstly introduced by another commit which was meant to be an NFC refactoring
in preparation for the most recent work that added support for the generation of JSON files.

commit cf6adecd6a8718ee2737ca55e4cd938364b984cc

Citation needed.
Even the warning says otherwise.

Right. I completely misunderstood that warning.
I wrongly thought that somehow llvm::zip was introducing copies under the hood. But as you wrote, that is not what the warning is saying and zip doesn't work that way.
Sorry about the confusion. You are absolutely right and the patch still looks good to me.

[andreadb]

Ping.

@plotfi could you please commit this?

Thanks

[RKSimon]

This was committed in D95997 / rG91e7a17133324ac4beaf6ed45c170436c2d91c98

llvm/tools/llvm-mca/Views/InstructionInfoView.cpp

void InstructionInfoView::printView(raw_ostream &OS) const {
   TempStream.flush();
   OS << Buffer;
 }
 
 void InstructionInfoView::collectData(
     MutableArrayRef<InstructionInfoViewData> IIVD) const {
   const llvm::MCSubtargetInfo &STI = getSubTargetInfo();
   const MCSchedModel &SM = STI.getSchedModel();
-  for (const auto &I : zip(getSource(), IIVD)) {
+  for (const auto I : zip(getSource(), IIVD)) {
     const MCInst &Inst = std::get<0>(I);
     InstructionInfoViewData &IIVDEntry = std::get<1>(I);
     const MCInstrDesc &MCDesc = MCII.get(Inst.getOpcode());
 
     // Obtain the scheduling class information from the instruction.
     unsigned SchedClassID = MCDesc.getSchedClass();
     unsigned CPUID = SM.getProcessorID();