[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

Cool!

I'd suggesting adding a note to llvm/docs/ReleaseNotes.rst at the same time :-)



================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:142
 
+CODEGENOPT(HotPatch, 1, 0) ///< Supports the Microsoft /HOTPATCH flag and would
+                           ///< generate a 'patchable-function' attribute.
----------------
nit: I'd s/would generate/generages/


================
Comment at: clang/include/clang/Driver/Options.td:2493
   MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryCount">>;
+def fhotpatch : Flag<["-"], "fhotpatch">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Ensure that all functions can be hotpatched at runtime">,
----------------
Is this flag also exposed as a driver flag, or is it cc1 only?

I wonder if we should go for a less generic name here, perhaps -fms-hotpatch? For example, the flag above is also about hotpatching, but a different mechanism.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:648
   Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug;
+  Options.Hotpatch = CodeGenOpts.HotPatch;
 
----------------
Since it's currently only supported on x86_64, we should probably diagnose trying to use it for other target somewhere.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5998
+  Args.AddLastArg(CmdArgs, options::OPT_fhotpatch);
 
   if (TC.SupportsProfiling()) {
----------------
Can we also pass /functionpadmin when clang-cl invokes the linker, like cl does?


================
Comment at: clang/test/Driver/cl-options.c:489
 // RUN:     /homeparams \
 // RUN:     /hotpatch \
 // RUN:     /JMC \
----------------
We should drop this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116511