[PATCH] D119910: [clang][debug] port clang-cl /JMC flag to ELF

Fri Mar 4 14:49:58 PST 2022

rnk added inline comments.

================
Comment at: llvm/test/Instrumentation/JustMyCode/jmc-instrument-elf.ll:1
-; REQUIRES: system-windows
-; RUN: opt -jmc-instrument -mtriple=x86_64-pc-windows-msvc  -S < %s | FileCheck %s
-; RUN: opt -jmc-instrument -mtriple=aarch64-pc-windows-msvc -S < %s | FileCheck %s
-; RUN: opt -jmc-instrument -mtriple=arm-pc-windows-msvc     -S < %s | FileCheck %s
+; REQUIRES: system-linux
+; RUN: opt -jmc-instrument -mtriple=x86_64-unknown-linux-gnu  -S < %s | FileCheck %s
----------------
ychen wrote:
> rnk wrote:
> > ychen wrote:
> > > rnk wrote:
> > > > Please remove the REQUIRES line, this test should pass on Windows, and the other test should pass on Linux as well. Neither of these tests depend on anything from the system.
> > > Yeah, this is a little tricky. It is about the host rather than the target. The flag symbol name is computed using `sys::path` functions. It is different between Windows and Linux. For testing purposes, I need to anchor them to a specific system to check the result. I could've made this `; REQUIRES: system-windows` and check the result, I thought it is more intuitive to test the ELF on Linux.
> > That seems like a bug. LLVM optimization and code generation should not depend on the host. This is a requirement for distributed build systems, which may migrate inputs to other machines, cross compile, and expect the results to match local compilation.
> The symbol names are host-dependent. It does not depend on the host to work correctly though. It works on both Windows and Linux.
Yes, you say they are host dependent, I see that, but I don't think that's a requirement. `opt` should be a pure function of its inputs. The paths are in the debug info, it shouldn't let the environment leak into the compilation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119910