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

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: llvm/lib/CodeGen/JMCInstrumenter.cpp:72
   SmallString<256> FilePath(SP.getDirectory());
   sys::path::append(FilePath, SP.getFilename());
   sys::path::native(FilePath);
----------------
These sys::path calls introduce host-dependence, and that should be eliminated


================
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:
> > 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119910