[llvm] [RISCV] [MachineOutliner] Analyze all candidates (PR #127659)

[Sam Elliott via llvm-commits]

================
@@ -3047,35 +3042,32 @@ RISCVInstrInfo::getOutliningCandidateInfo(
     std::vector<outliner::Candidate> &RepeatedSequenceLocs,
     unsigned MinRepeats) const {
 
-  // Each RepeatedSequenceLoc is identical.
-  outliner::Candidate &Candidate = RepeatedSequenceLocs[0];
-  auto CandidateInfo = analyzeCandidate(Candidate);
-  if (!CandidateInfo)
-    RepeatedSequenceLocs.clear();
+  // Analyze each candidate and erase the ones that are not viable.
+  llvm::erase_if(RepeatedSequenceLocs, analyzeCandidate);
 
   // If the sequence doesn't have enough candidates left, then we're done.
   if (RepeatedSequenceLocs.size() < MinRepeats)
     return std::nullopt;
 
+  // Each RepeatedSequenceLoc is identical.
+  outliner::Candidate &Candidate = RepeatedSequenceLocs[0];
   unsigned InstrSizeCExt =
       Candidate.getMF()->getSubtarget<RISCVSubtarget>().hasStdExtCOrZca() ? 2
                                                                           : 4;
----------------
lenary wrote:

Is this calculation still correct? It's using the context of the candidate rather than the instructions inside the candidate.

Presumably I could have two functions, one of which has Zca/C enabled, one of which doesn't, and the accounting for the overhead of one of these cases would be slightly wrong?

I don't know how we end up constructing a subtarget for the outlined function, based off the candidates. I think in this case, the `InstrSizeCExt` is for a single instruction that appears in the outlined function, rather than in the call sites, so maybe we need to check all the candidate functions have Zca/C enabled before we choose to finalise an sequence with Zca (or calculate the cost based on the expectation we will)?

https://github.com/llvm/llvm-project/pull/127659