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

[Dean Michael Berris via llvm-commits]

dberris added inline comments.

================
Comment at: lib/Target/X86/X86MCInstLower.cpp:837-838
@@ -836,1 +836,4 @@
+    (void) OnlyOneNop;
+    assert((!OnlyOneNop || NumBytes == 0) &&
+           "Allowed only one nop instruction!");
   } // while (NumBytes)
----------------
I'm not sure this assertion makes sense here. I would have thought this assert should have been done in the calling code, that it doesn't ask for a single nop in the first place?

================
Comment at: lib/Target/X86/X86MCInstLower.cpp:948-949
@@ +947,4 @@
+    } else {
+      EmitNops(*OutStreamer, MinSize, Subtarget->is64Bit(), getSubtargetInfo(),
+               /* OnlyOneNop = */ true);
+    }
----------------
So in this branch, MinSize != 2 or Opcode != X86::PUSH64r.

Question: If you check instead whether the function where MI is included in had the correct type of patchable-function attribute, and see that the nops being added is less than 2, you can assert and say this is actually a bug in a higher implementation detail? i.e. this would be a bug in the insertion of this instruction.

This way you wouldn't need to touch EmitNops.


http://reviews.llvm.org/D19046