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

Vitaly Buka via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 19 12:17:32 PDT 2023


vitalybuka added inline comments.


================
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
----------------
mstorsjo wrote:
> mstorsjo wrote:
> > mstorsjo wrote:
> > > 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.
> > Actually, the difference is even more subtle… In most tests, we execute e.g. `%t/binary`, but here we try to execute `./binary`. The Python executor seems to handle adding an implicit extra `.exe` if needed for finding the executable with an absolute path from `%t`, but not with `./`.
> So I guess writing `%run %t/binary` here would work too (it doesn’t seem to remove anything of what this test tries to test); would you prefer that? That would be more alike most often tests.
Yes, I would prefer to avoid .exe on platforms where it's not needed.
Same applies to merge4


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