Meinersbur requested changes to this revision.
Meinersbur added inline comments.
This revision now requires changes to proceed.
================
Comment at: llvm/test/Feature/load_extension.ll:1
+; RUN: not test -f %llvmshlibdir/libBye%shlibext || opt %s
-load=%llvmshlibdir/libBye%shlibext -goodbye -wave-goodbye \
+; RUN: -disable-output 2>&1 | grep Bye
----------------
It would be preferable to have a "REQUIRES" line that checks that the Bye pass
has been linked-in dynamically.
Alternatively, use the lit.cfg to expend to the required `-load=` argument if
required, so this test checks with and without LLVM_BYE_LOAD_INTO_TOOLS
================
Comment at: llvm/test/Feature/load_extension.ll:2
+; RUN: not test -f %llvmshlibdir/libBye%shlibext || opt %s
-load=%llvmshlibdir/libBye%shlibext -goodbye -wave-goodbye \
+; RUN: -disable-output 2>&1 | grep Bye
+; REQUIRES: plugins
----------------
[serious] Use FileCheck
================
Comment at: llvm/test/lit.cfg.py:193-196
+if config.linked_bye_extension:
+ llvm_config.with_environment('LLVM_CHECK_EXT', 'CHECK-EXT')
+else:
+ llvm_config.with_environment('LLVM_CHECK_EXT', 'CHECK-NOEXT')
----------------
[serious] This kind of shell expansion does not work on Windows:
```
$ "c:\users\meinersbur\build\llvm\release\bin\filecheck.exe"
"C:\Users\meinersbur\src\llvm\test\Other\new-pm-thinlto-defaults.ll"
"--check-prefixes=CHECK-O,CHECK-O1,CHECK-POSTLINK-O,${LLVM_CHECK_EXT},CHECK-POSTLINK-O1"
# command stderr:
Supplied check-prefix is invalid! Prefixes must be unique and start with a
letter and contain only alphanumeric characters, hyphens and underscores
error: command failed with exit status: 2
```
Polly 'solves' this by adding itself to the pass manager only when another
command line flag is added (`-polly`) which would be less intrusive.
================
Comment at: llvm/test/lit.site.cfg.py.in:49
config.has_plugins = @LLVM_ENABLE_PLUGINS@
+config.linked_bye_extension = "@LLVM_BYE_LINK_INTO_TOOLS@".lower() in ("on", 1)
----------------
See https://cmake.org/cmake/help/latest/command/if.html for which values cmake
considers true-ish.
================
Comment at: polly/lib/Support/RegisterPasses.cpp:726
+
+#ifndef LLVM_POLLY_LINK_INTO_TOOLS
+extern "C" LLVM_ATTRIBUTE_WEAK ::llvm::PassPluginLibraryInfo
----------------
[serious] `LLVM_POLLY_LINK_INTO_TOOLS` is a cmake configuration parameter, but
not a preprocessor symbol. Hence, `LLVM_POLLY_LINK_INTO_TOOLS` is never defined.
Error on Windows:
```
Bye.lib(Bye.cpp.obj) : error LNK2005: llvmGetPassPluginInfo already defined in
Polly.lib(RegisterPasses.cpp.obj)
bin\opt.exe : fatal error LNK1169: one or more multiply defined symbols found
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61446/new/
https://reviews.llvm.org/D61446
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
