[PATCH] D77852: Outline CFI Instructions in Tail Calls

[Andrew Litteken via Phabricator via llvm-commits]

AndrewLitteken marked an inline comment as done.
AndrewLitteken added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5878-5884
+    for (unsigned Loc = C.getStartIdx(); Loc < C.getEndIdx() + 1; Loc++) {
+      if (Mit->isCFIInstruction()) {
+        HasCFI = true;
+        unsigned CFIIndex = Mit->getOperand(0).getCFIIndex();
+        MCCFIInstruction CFI = CFIInstructions[CFIIndex];
+        CFIInstructionsPresent.push_back(CFI);
+      }
----------------
paquette wrote:
> I don't think you have to do this for every candidate.
> 
> The part you have to do for every candidate is checking the number of CFI instructions in each function frame. This should be sufficient, because we know every instruction in every candidate is the same.
> 
> E.g. after collecting the CFI instructions in the first candidate, you can do something like this:
> 
> ```
> if (HasCFI) {
>   for (outliner::Candidate &C : RepeatedSequenceLocs) {
>     if (C.getMF()->getFrameInstructions().size() != CFIInstructionsPresent.size())
>       return outliner::OutlinedFunction();
>   }
> }
> ```
> 
> or even
> 
> ```
> if (HasCFI && any_of(RepeatedSequenceLocs, ...))
>   return outliner::OutlinedFunction()
> ```
> 
> Actually, is it even necessary to //collect// the CFI instructions? Since all we need to know is that the candidates actually contain all of them, it would be sufficient to just count how many are in the candidate versus how many are in the frame of each function, no?
I see now, I thought you reversed what you said in the first review about not having to check everything.  I also agree, just finding the number should be sufficient, we shouldn't even need use a vector.


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

https://reviews.llvm.org/D77852