[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