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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
