llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.92k stars 11.52k forks source link

[lto] Distributed thinlto `Error loading imported file` return 0 #75441

Open zcfh opened 9 months ago

zcfh commented 9 months ago

problem

The following script can reproduce the scenario I mentioned. When using -fthinlto-index to optimization, if the dependent file cannot be found, only an error will be output, and the return value will still be 0, making it impossible to determine whether an error occurred.

strip output

+ clang++ -c file1.cc file2.cc -O2 -g -flto=thin
+ clang++ -flto=thin -fuse-ld=lld -Wl,--thinlto-emit-imports-files,--thinlto-index-only -O2 file1.o file2.o
+ rm file2.o
+ clang++ -O2 -o file1.native.o -x ir file1.o -c -fthinlto-index=./file1.o.thinlto.bc
Error loading imported file 'file2.o': No such file or directory
+ echo 'return code 0'
return code 0

Note

$ clang -v
clang version 11.1.0
project=tmp_thinlto_test
mkdir -p $project
cd $project

main_str='int lib_func(int val);
int main(int argc, char **argv) {
  return lib_func(1);
}
'

lib_str='int lib_func(int val) {
  return val + 123;
}
'

echo "$main_str" > file1.cc
echo "$lib_str" > file2.cc

set -x
pwd
clang++ -c file1.cc file2.cc -O2 -g -flto=thin
clang++ -flto=thin -fuse-ld=lld  -Wl,--thinlto-emit-imports-files,--thinlto-index-only -O2 file1.o file2.o

# file1.o dep file2.o
rm file2.o
clang++ -O2 -o file1.native.o -x ir file1.o -c -fthinlto-index=./file1.o.thinlto.bc

echo "return code $?"
EugeneZelenko commented 9 months ago

@teresajohnson

teresajohnson commented 9 months ago

Hmm, not sure why, I looked at the code and afaict it should be checking and reporting errors. If you want to dig into this some more, here's what I am seeing in the code:

The opening of files it will import from is done in FunctionImporter::importFunctions, and should return an error if it fails to load: https://github.com/llvm/llvm-project/blob/66e0159db85fc2ccbf098b7a74c06e208cc94ce4/llvm/lib/Transforms/IPO/FunctionImport.cpp#L1360-L1362

That's called from here which should return if an error is returned: https://github.com/llvm/llvm-project/blob/0d02ecc6bddc5e8c8bacbae3ab72a10467e177b4/llvm/lib/LTO/LTOBackend.cpp#L658

On the line above that call you can see the ModuleLoader being passed in, which is a lambda defined just above there. It should return an error if llvm::MemoryBuffer::getFile returns an error.

In the distributed ThinLTO case it is called from here in clang which should handle any returned error: https://github.com/llvm/llvm-project/blob/732bccb8c1b4ea724919db6ff02b1188e20850e7/clang/lib/CodeGen/BackendUtil.cpp#L1268

zcfh commented 9 months ago

Thank you for your answer. Does this mean that an error should be returned, but it is not? Or is there something wrong with the clang binary I built, causing it not to be returned?

teresajohnson commented 9 months ago

It looks to me like it should be reporting an error, so unfortunately I'm not sure why it isn't. Are you able to dig a bit more into it? If not I can take a look but it won't be this week.

zcfh commented 8 months ago

https://github.com/llvm/llvm-project/blob/1fdec59bffc11ae37eb51a1b9869f0696bfd5312/clang/lib/CodeGen/BackendUtil.cpp#L1525-L1530

I checked the 11.x code and found that the location where the error log is printed is just return after outputting the error. Is this expected?

teresajohnson commented 8 months ago

Ah, didn't notice before that you are using a quite old version of clang. That code no longer exists and has been improved. See the links in my earlier comment to where it should now be detecting and handling the error.

zcfh commented 8 months ago

Thanks for your answer, but upgrading the compiler is relatively expensive for me. I'll try to find other ways to get around it.