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

Thu Jan 13 09:03:59 PST 2022

hans added inline comments.

================
Comment at: clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp:13
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH
+// ERR-HOTPATCH: error: unsupported option '/hotpatch' for target
+// HOTPATCH: S_COMPILE3 [size = [[#]]]
----------------
aganea wrote:
> hans wrote:
> > Does MSVC error for ARM/ARM64 too, or does it just ignore the flag?
> It prints a warning:
> ```
> D:\git\llvm-project>cl /c main.cpp /hotpatch
> Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30138 for ARM64
> Copyright (C) Microsoft Corporation.  All rights reserved.
> 
> cl : Command line warning D9002 : ignoring unknown option '/hotpatch'
> main.cpp
> ```
> But in our case, the issue is that `PATCHABLE_OP` isn't supported on ARM backend, so it ends up asserting in `D:/git/llvm-project/release/lib/Target/AArch64/AArch64GenMCCodeEmitter.inc` in `AArch64MCCodeEmitter::getBinaryCodeForInstr`. There's perhaps a (better?) way for shortcutting the use of `/hotpatch` on ARM, instead of erroring-out like today.
> 
> Should we be more clear in the message, saying that hotpatching is supported but we don't the flag? Or just consume and ignore it?
I think the most user-friendly thing would be to consume and ignore it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116511