ychen 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:
> > > > 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.
> Great point. This is not ideal in principle. For the distributed build, this 
> may break if the remote and host compilation uses different environments like 
> Windows/Linux. I'll create a follow-up patch. 
Filed https://github.com/llvm/llvm-project/issues/54219 to follow up on the bug.


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

Reply via email to