tejohnson added inline comments.
================ Comment at: llvm/lib/LTO/LTOBackend.cpp:151 + TargetMachine::initTargetOptions(M, Conf.Options); + ---------------- This is going to be problematic. The Conf is a reference to the Config object saved on the LTO class instance shared by all backend invocations (the regular LTO module if one exists and any ThinLTO modules). They will end up clobbering each other's values here - although from the assert in initTargetOptions I see they are required to all have the same value anyway. Still, it is not good as the assert may actually be missed with unlucky interference between the threads. The Config object here should really be marked const, let me see if I can change that. You could make a copy of the Config here, but that essentially misses the assertion completely. A better way to do this would be in LTO::addModule, which is invoked serially to add each Module to the LTO object. However - this misses other places that invoke createTargetMachine - should other places be looking at this new module flag as well? One example I can think of is the old LTO API (*LTOCodeGenerator.cpp files), used by linkers such as ld64 and some other proprietary linkers and the llvm-lto testing tool. But I have no idea about other invocations of createTargetMachine. Note that changes to LTO.cpp/LTOBackend.cpp (the new LTO API) needs some kind of llvm-lto2 based test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72245/new/ https://reviews.llvm.org/D72245 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
