Closed compor closed 1 year ago
@xjtuwxg this is not urgent, but a look into it would be appreciated :)
Thanks for the pull request. Seems like you want to upgrade the building script to a later Ubuntu version. Do you also have an update on the Dockerfile? Here is a link how to use it. Also, should we choose an LTS Ubuntu version like 20.04 or 22.04 as the base docker image?
I'm having a quick test using your script on a Ubuntu 20.04 base docker image, there seem a lot of warnings during the building process of LLVM/Clang9 (where 18.04 has a perfect building process w/o warnings). But I understand we cannot use the 18.04 docker image forever.
Hey @xjtuwxg, thanks for the comments. All your suggestions make sense, so here is the actionable breakdown:
llvm::ArrayRef
, e.g.:
warning: initializing ‘llvm::ArrayRef<const llvm::SCEV*>::Data’ from ‘std::initializer_list<const llvm::SCEV*>::begin’ does not extend the lifetime of the underlying array [-Winit-list-lifetime]
warning: redundant move in return statement [-Wredundant-move]
warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
Regarding the warnings, I think this is due to a newer GCC version which probably has improved checks for the code quality. I cannot guarantee a timeline for this, since I'm a bit busy this period, so I'd suggest leaving this PR open and I'll ping you to have another look once I address the above points.
Hi again @xjtuwxg,
As per above, here's a more refined breakdown:
for(auto e: mylist)
instead of for(const auto &e: mylist)
.
These are issues with the LLVM codebase which have been fixed in newer releases like this.
Hence, this warning is left to be emitted.Let me know what you think. Maybe the extra CMake CXX flags in e8bbb2bebcb5fd2f2a85505f7489013c06133489 for the aforementioned warnings could be guarded by a version check, e.g., to revisit them if the LLVM is greater than 9.1. Not sure if that is something desirable in this project.
This PR is based on issues that came upon a "clean" Ubuntu 21.10 Docker image:
Fix use of base path (
--base-path
commandline option) used for:This allows installation of the Popcorn compiler without having to use the repo's top dir as the CWD.
Ideally, the
--base-path
value should be autodetected, but I had no time to make this happen.#include <limits>
). This is part of the benchmarking code of the project, thus inconsequential for the core Popcorn compiler.