[PATCH] D19046: Introduce a "patchable-prologue" function attribute

Eric Christopher via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 14 13:53:26 PDT 2016


echristo added inline comments.

================
Comment at: docs/LangRef.rst:1408
@@ -1407,1 +1407,3 @@
     long as they do not significantly impact runtime performance.
+``"patchable-function"``
+    This attribute tells the code generator that the code
----------------
Does this need to be a hard coded attribute? Why not something similar to the floating point ones while we're still working out things? Avoids needing to worry about bitcode reading/writing.

================
Comment at: docs/LangRef.rst:1415
@@ +1414,3 @@
+
+     * ``"prologue-hotpatch-compact"`` - This style of patchable
+       function is intended to support patching a function prologue to
----------------
Perhaps a different name for it? hotpatch-compact isn't particularly enlightening without the description. Is the "compact" because it only handles the small code model? It might be best to talk about the option in an architecture neutral way and then explain the particular implementation in a cpu specific section below for it.

================
Comment at: lib/CodeGen/PatchableFunction.cpp:44-47
@@ +43,6 @@
+  Attribute PatchAttr = MF.getFunction()->getFnAttribute("patchable-function");
+  StringRef PatchType = PatchAttr.getValueAsString();
+
+  assert(PatchType == "prologue-hotpatch-compact" && "Only possibility today!");
+  (void)PatchType;
+
----------------
Can merge all of this.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:2924
@@ +2923,3 @@
+
+  // We need to ensure that FirstMI is at least two bytes long (a short jump can
+  // be encoded in two bytes) and does not span a 16 byte boundary (to be
----------------
Interesting. Is the idea here to avoid too much code growth? I'm assuming the performance of a pile of nops isn't that bad.

Also, are you just using the address of the symbol as the patchable address for the function?


http://reviews.llvm.org/D19046





More information about the llvm-commits mailing list