aganea marked 2 inline comments as done. aganea 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 = [[#]]] ---------------- 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? ================ Comment at: clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp:19 +/// +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c %s -o %t.obj +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH ---------------- hans wrote: > nit: We still need -- before %s with clang-cl (%s could be /Users/... and get > interpreted as a flag), and so it needs to come last. Same for the line in > debug-info-hotpatch.cpp. Thanks will do! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116511/new/ https://reviews.llvm.org/D116511 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
