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

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 13 11:41:02 PDT 2016


sanjoy marked 2 inline comments as done.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:30642
@@ +30641,3 @@
+  case X86::SUB64ri32:
+    // We know this instruction takes more than two bytes.
+    break;
----------------
rnk wrote:
> I guess we are confident that modifying an instruction during its execution is not problematic.
At this point I'm fairly sure that replacing an instruction with another instruction of the exact same size is okay in practice.  What I'm less sure of is replacing an executing instruction with another one that is smaller (i.e. replace only the prefix of an instruction), which is what this patch does.

Unfortunately, I can't think of a way to determine if the second assertion is correct or not except by running a lot of code compiled with `"patchable-prologue"="hotpatch-compact"` on machines with high core counts (the patch as is passes some basic sanity checks).  If more thorough testing uncovers issues, then we'll deal with them as they come.

Given what I just said, do you think it is a good idea to rename the attribute to `"experimental-hotpatch-compact"`?


http://reviews.llvm.org/D19046





More information about the llvm-commits mailing list