[PATCH] D148169: [compiler-rt] [test] [profile] Add an .exe suffix to some temp executables

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 19 11:34:57 PDT 2023


mstorsjo added inline comments.


================
Comment at: compiler-rt/test/profile/instrprof-basic.c:41
+// RUN: %run %t.dir4/merge4.exe
+// RUN: rm -f %t.dir4/merge4.exe
 // RUN: llvm-profdata merge -o %t.m4.profdata ./
----------------
vitalybuka wrote:
> mstorsjo wrote:
> > alvinhochun wrote:
> > > Do you know is there any reason why this test case needs to remove the exe but the previous test cases don't? Can we instead just remove this rm command?
> > In this test, the profile data files are written in the same directory as the executable, and `llvm-profdata` reads all files the directory. If the executable is left there, it fails to read that file.
> can you keep everything as is and just do
> ```
> rm -f %t.dir4/merge4*
> ```
Yeah, that should work too 


================
Comment at: compiler-rt/test/profile/instrprof-tmpdir.c:7
 // Check that a dir separator is appended after %t is subsituted.
-// RUN: env TMPDIR="%t" LLVM_PROFILE_FILE="%%traw1.profraw" %run ./binary
+// RUN: env TMPDIR="%t" LLVM_PROFILE_FILE="%%traw1.profraw" %run ./binary.exe
 // RUN: llvm-profdata show ./raw1.profraw | FileCheck %s -check-prefix TMPDIR
----------------
vitalybuka wrote:
> why do you need exe here?
> isn't windows will implicitly add .exe when you run  ./binary ?
Yes, e.g. `cmd.exe` does that, but here, we have Python executing the commands.

In most cases, the executable has got some suffix (e.g. `%t` expanding to something with `.tmp` as suffix); in those cases there’s no extra suffix added - but for a plain name like `binary` here, we’ve got this mismatch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148169



More information about the llvm-commits mailing list