[PATCH] D78062: Adding Comment Annotations to Outlined Functions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added a comment.

Added some comments.

Also, this needs a testcase which shows that the comments are added.

I think it would make sense to get this going properly for AArch64 first, and ignore X86 for now.



================
Comment at: llvm/include/llvm/CodeGen/MachineOutliner.h:26
 
+enum MachineOutlinerClass {
+  MachineOutlinerDefault,  /// Emit a save, restore, call, and return.
----------------
These are target-specific. I don't think we want to pull these in explicitly here. Different targets aren't guaranteed to implement the same types of outlining.

Also some of these things wouldn't make sense on other targets (e.g. NoLRSave)



================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:709
+    if (OutlinedMetaData)
+      if(MDString *OutlinedMetaString = dyn_cast<MDString>(OutlinedMetaData->getOperand(0)))
+        OutStreamer->GetCommentOS() << ' ' << OutlinedMetaString->getString();
----------------
clang-format


================
Comment at: llvm/lib/CodeGen/MachineOutliner.cpp:78
 #include <functional>
+#include <string>
 #include <tuple>
----------------
Do you have to include `string` here?


================
Comment at: llvm/lib/CodeGen/MachineOutliner.cpp:1151
 
+  std::string OutlineKindString = "; ";
+  if (FirstCand.CallConstructionID == MachineOutlinerTailCall)
----------------
Is it possible to use a `Twine` here?

http://llvm.org/docs/ProgrammersManual.html#the-twine-class


================
Comment at: llvm/lib/CodeGen/MachineOutliner.cpp:1152-1161
+  if (FirstCand.CallConstructionID == MachineOutlinerTailCall)
+    OutlineKindString += "Tail Call";
+  else if (FirstCand.CallConstructionID == MachineOutlinerThunk)
+    OutlineKindString += "Thunk";
+  else if (FirstCand.CallConstructionID == MachineOutlinerNoLRSave)
+    OutlineKindString += "Function with no Save and Restore";
+  else if (FirstCand.CallConstructionID == MachineOutlinerRegSave)
----------------
I think that this should be implemented in the target rather than in the generic algorithm.


================
Comment at: llvm/lib/CodeGen/MachineOutliner.cpp:1152-1161
+  if (FirstCand.CallConstructionID == MachineOutlinerTailCall)
+    OutlineKindString += "Tail Call";
+  else if (FirstCand.CallConstructionID == MachineOutlinerThunk)
+    OutlineKindString += "Thunk";
+  else if (FirstCand.CallConstructionID == MachineOutlinerNoLRSave)
+    OutlineKindString += "Function with no Save and Restore";
+  else if (FirstCand.CallConstructionID == MachineOutlinerRegSave)
----------------
paquette wrote:
> I think that this should be implemented in the target rather than in the generic algorithm.
I think that you want to communicate the FrameID here, not the CallConstructionID?

One outlined function can be called in more than one way. E.g. it's possible to call the same outlined function using the NoLRSave and RegSave modes.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5559-5565
+/*enum MachineOutlinerClass {
   MachineOutlinerDefault,  /// Emit a save, restore, call, and return.
   MachineOutlinerTailCall, /// Only emit a branch.
   MachineOutlinerNoLRSave, /// Emit a call and return.
   MachineOutlinerThunk,    /// Emit a call and tail-call.
   MachineOutlinerRegSave   /// Same as default, but save to a register.
+};*/
----------------
Commented out code in patch?


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:8666-8669
+/*enum MachineOutlinerClass {
   MachineOutlinerDefault,
   MachineOutlinerTailCall
+};*/
----------------
Commented out code in patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78062