Page MenuHomePhabricator

LTO: Add option to initialize with opaque/non-opaque pointer types
ClosedPublic

Authored by MatzeB on May 17 2022, 8:39 PM.

Details

Summary

LTO code may end up mixing bitcode files from various sources varying in their use of opaque pointer types. The current strategy to decide between opaque / typed pointers upon the first bitcode file loaded does not work here, since we could be loading a non-opaque bitcode file first and would then be unable to load any files with opaque pointer types later.

This enables/disables opaque pointers immediately when creating an LLVMContext for LTO. Opaque pointers are enabled by default.

  • Adds an lto::Config::OpaquePointer option and enforces an upfront decision between the two modes.
  • Adds -opaque-pointers/-no-opaque-pointers options to the gold plugin and lld; enabled by default.
  • Adds an -lto-opaque-pointers/-lto-opaque-pointers=0 option to the llvm-lto2 tool.
  • Changes the clang driver to pass -plugin-opt=no-opaque-pointers to the linker in LTO modes when clang was configured with opaque pointers disabled.

This fixes https://github.com/llvm/llvm-project/issues/55377

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
MatzeB edited the summary of this revision. (Show Details)May 18 2022, 10:14 AM
MatzeB updated this revision to Diff 430492.May 18 2022, 1:49 PM

update tests

+1 to adding something to lto::Config.

Do we really need this for a temporary switch? Will this force us to keep supporting the new switch into the future for backwards compatibility in the commandline interface even when we dropped all references to typed pointers from LLVM?

MatzeB updated this revision to Diff 430511.May 18 2022, 2:48 PM
MatzeB updated this revision to Diff 430515.May 18 2022, 3:17 PM

+1 to adding something to lto::Config.

Do we really need this for a temporary switch? Will this force us to keep supporting the new switch into the future for backwards compatibility in the commandline interface even when we dropped all references to typed pointers from LLVM?

You mentioned that -mllvm -opaque-pointers=0 is a temporary workaround with this patch, but -mllvm -opaque-pointers=1 works just fine for what this patch is trying to do. I think respecting -Xclang -[no-]opaque-pointers when performing LTO is desirable, and for that you'd need the lto::Config change.

We've removed a bunch of legacy/new pass manager switches, I don't see why we'd keep this switch around.

MaskRay added inline comments.May 19 2022, 12:21 AM
llvm/include/llvm/LTO/Config.h
300

delete braces

While trying to implement things I noticed that -Xclang options are only meant for the LLVM codegeneration part (aka CC1) of clang, they are not meant for the linker/LTO (which would be -Xlinker or -Wl) and consequently there is no easy way in the code to query -Xclang options and pass them on to the linker (nor would it be a good idea I think).

The usual approach seems to be that codegen options are recorded in the IR as much as possible so we can reconstruct the settings from that during LTO. This is however awkward in this case, as we have to make the decision between opaque/typed pointer types before reading the first bitcode file and cannot really go back and undo it...

I think what I'm gonna do is:

  • Add an LTOConfig option for opaque pointers, disabled by default
  • Add -opaque-pointer, -no-opaque-pointer options to gold-plugin and lld/ELF, disabled by default
  • Have clang pass a -opaque-pointer option to lld if CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL was enabled during the build and the user didn't pass any other -opaque-pointer/-no-opaque-pointer option via -Wl / -Xlinker.
MatzeB updated this revision to Diff 430825.May 19 2022, 3:21 PM
MatzeB retitled this revision from LTO: Enable opaque pointers by default. to LTO: Decide upfront whether to use opaque/non-opaque pointer types.
MatzeB edited the summary of this revision. (Show Details)

Changed the approach a bit; summary (and tests) are updated.

MatzeB updated this revision to Diff 430837.May 19 2022, 4:01 PM
MatzeB updated this revision to Diff 430838.May 19 2022, 4:01 PM
Herald added a project: Restricted Project. · View Herald Transcript
nikic added inline comments.May 20 2022, 1:01 AM
llvm/include/llvm/LTO/Config.h
182

The default should be true here and for the LLD/LLVMgold flags, to match the Clang default and upcoming LLVM default. It's also the conservative option, because OpaquePointers=true will work fine with typed bitcode, but not the other way around. As this change disables the auto-detection, leaving the default to off would be a big footgun for anyone invoking LTO through the linker rather than the clang driver.

Is there any issues currently if we just always use opaque pointer in LTO?

I'm still looking into a whole program devirtualization bug that only repros with opaque pointers, and there are still potential performance issues to look into

I'm still looking into a whole program devirtualization bug that only repros with opaque pointers, and there are still potential performance issues to look into

Thanks. Maybe here is another way to look at this problem, instead of giving people a temporary switch to use opaque_pointer if their first bitcode is no_opaque_pointer, we opt as aggressive as possible into opaque_pointer and give people a temporary switch to go back to old behavior just like how clang is configured.

MaskRay added 1 blocking reviewer(s): MaskRay.May 21 2022, 11:10 AM
MaskRay added inline comments.
lld/ELF/Options.td
629

The convention is =opaque-pointers

MatzeB updated this revision to Diff 431853.May 24 2022, 6:12 PM
MatzeB marked an inline comment as done.

Enable LTO by default and fix a whole bunch of LTO tests because of it.

MatzeB marked 2 inline comments as done.May 24 2022, 6:13 PM
nikic added a comment.May 25 2022, 6:29 AM

Looks reasonable to me, but probably @MaskRay should review.

We could add a -fno-opaque-pointers driver option that covers both the compiler backend and LTO, but we can probably get away without it as well.

lld/ELF/Options.td
311

(default) here is outdated.

Possibly this option should be lto_opaque_pointers? That seems to be the convention for other related options.

llvm/docs/OpaquePointers.rst
71

-plugin-opt=opaque-pointers. Though it probably makes more sense now to mention this at the end of the document, as an option to disable.

MatzeB updated this revision to Diff 432347.May 26 2022, 12:03 PM
MatzeB marked an inline comment as done.

Address review feedback.

MatzeB marked an inline comment as done.May 26 2022, 12:03 PM
MatzeB added inline comments.
lld/ELF/Options.td
311

👍

MatzeB updated this revision to Diff 432350.May 26 2022, 12:04 PM
MatzeB marked an inline comment as done.
nikic accepted this revision.Tue, May 31, 3:06 AM

LGTM

clang/test/CodeGen/thinlto-inline-asm2.c
6

I don't think this is needed for cc1 invocations (they should always default to opaque pointers).

Adds -opaque-pointers/-no-opaque-pointers options to the gold plugin; disabled by default.

-plugin-opt=[no-]opaque-pointers

--opaque-pointers/--no-opaque-pointers options with -plugin-opt=-opaque-pointers/-plugin-opt=-no-opaque-pointers aliases to lld; disabled by default.

For such temporary options, don't add regular options. Just add -plugin-opt=[no-]opaque-pointers.

Changes the clang driver to pass -plugin-opt=-opaque-pointers to the linker in LTO modes when clang was configured with opaque pointers enabled by default.

-plugin-opt=no-opaque-pointers

MaskRay added inline comments.Tue, May 31, 7:33 PM
clang/test/Driver/lto-no-opaque-pointers.c
3

-target is a legacy option. Use --target= for new tests.

No need to use check fd 2 (2> %t). There is sufficient coverage testing that the options are in fd 2.

Just use 2>&1 | FileCheck %s.

clang/test/Driver/lto-opaque-pointers.c
3

ditto

MatzeB updated this revision to Diff 433577.Wed, Jun 1, 3:41 PM

address review feedback. I assume this is accepted and good to push when the buildkit builds look reasonable.

MatzeB marked 3 inline comments as done.Wed, Jun 1, 3:41 PM
MatzeB edited the summary of this revision. (Show Details)
MatzeB edited the summary of this revision. (Show Details)Wed, Jun 1, 3:44 PM
MaskRay accepted this revision.Wed, Jun 1, 4:42 PM

LTO: Decide upfront whether to use opaque/non-opaque pointer types

This may need to mention "add options" to decide <...>

This revision is now accepted and ready to land.Wed, Jun 1, 4:42 PM
MaskRay added inline comments.Wed, Jun 1, 4:43 PM
llvm/tools/gold/gold-plugin.cpp
966

Conf?

MatzeB added inline comments.Wed, Jun 1, 4:45 PM
llvm/tools/gold/gold-plugin.cpp
966

Ouch, good catch. Guess we have no form of direct testing of the gold plugin in LLVM...

address review feedback. I assume this is accepted and good to push when the buildkit builds look reasonable.

(I changed myself to a blocking review to check the component I mostly care about. I promise I'll perform a quick check.)

MatzeB updated this revision to Diff 433591.Wed, Jun 1, 4:47 PM
MaskRay added inline comments.Wed, Jun 1, 4:48 PM
llvm/tools/gold/gold-plugin.cpp
966

Sanitizers have bots using -DLLVM_BINUTILS_INCDIR=/usr/include, but otherwise using this option is uncommon among build bots.

MatzeB added a comment.Wed, Jun 1, 4:48 PM

address review feedback. I assume this is accepted and good to push when the buildkit builds look reasonable.

(I changed myself to a blocking review to check the component I mostly care about. I promise I'll perform a quick check.)

Of course, I won't push then. I wasn't sure since the accept came after the set to block.

MatzeB marked 2 inline comments as done.Wed, Jun 1, 4:49 PM
MatzeB retitled this revision from LTO: Decide upfront whether to use opaque/non-opaque pointer types to LTO: Add option to initialize with opaque/non-opaque pointer types.
MaskRay accepted this revision.Wed, Jun 1, 4:55 PM
This revision was landed with ongoing or failed builds.Wed, Jun 1, 6:07 PM
This revision was automatically updated to reflect the committed changes.

Your commit message seems to use the original summary.

Hi, this caused arm-float-abi-lto.c to fail on AIX. The failure went away for a few builds, then came back. Could you take a look?

https://lab.llvm.org/buildbot/#/builders/214/builds/1625/steps/6/logs/FAIL__Clang__arm-float-abi-lto_c

MatzeB added a comment.Thu, Jun 2, 2:40 PM

Hi, this caused arm-float-abi-lto.c to fail on AIX. The failure went away for a few builds, then came back. Could you take a look?

https://lab.llvm.org/buildbot/#/builders/214/builds/1625/steps/6/logs/FAIL__Clang__arm-float-abi-lto_c

Uh, this change just enables opaque pointers for this test. And it appears to be crashing nondeterministically somewhere within LLVM. Unfortunately I don't see this happening on my X86 development machine. None of the ASAN enabled buildbots appears to complain either. I don't know how to even start debugging this, do you have at least a stacktrace?

Jake-Egan added a comment.EditedFri, Jun 3, 4:36 PM

Hi, this caused arm-float-abi-lto.c to fail on AIX. The failure went away for a few builds, then came back. Could you take a look?

https://lab.llvm.org/buildbot/#/builders/214/builds/1625/steps/6/logs/FAIL__Clang__arm-float-abi-lto_c

Uh, this change just enables opaque pointers for this test. And it appears to be crashing nondeterministically somewhere within LLVM. Unfortunately I don't see this happening on my X86 development machine. None of the ASAN enabled buildbots appears to complain either. I don't know how to even start debugging this, do you have at least a stacktrace?

Sorry about the delayed response. Marking the test unsupported on AIX for now: c3c75d805c2174388417080f762230961b3433d6 Here is a stack trace:

#0  0x0000000100013e50 in llvm::StringRef::find(llvm::StringRef, unsigned long) const ()
#1  0x0000000101b17fd4 in llvm::ARM_MC::ParseARMTriple(llvm::Triple const&, llvm::StringRef) ()
#2  0x0000000102c16294 in llvm::ARMAsmPrinter::emitAttributes() ()
#3  0x0000000102c16008 in llvm::ARMAsmPrinter::emitStartOfAsmFile(llvm::Module&) ()
#4  0x0000000102a6a980 in llvm::AsmPrinter::doInitialization(llvm::Module&) ()
#5  0x00000001001bdecc in llvm::FPPassManager::doInitialization(llvm::Module&) ()
#6  0x0000000103c9d2a0 in llvm::legacy::PassManagerImpl::run(llvm::Module&) ()
#7  0x0000000103c9ce34 in llvm::legacy::PassManager::run(llvm::Module&) ()
#8  0x0000000103c97014 in codegen(llvm::lto::Config const&, llvm::TargetMachine*, std::__1::function<llvm::Expected<std::__1::unique_ptr<llvm::CachedFileStream, std::__1::default_delete<llvm::CachedFileStream> > > (unsigned int)>, unsigned int, llvm::Module&, llvm::ModuleSummaryIndex const&) ()
#9  0x00000001030105c0 in llvm::lto::thinBackend(llvm::lto::Config const&, unsigned int, std::__1::function<llvm::Expected<std::__1::unique_ptr<llvm::CachedFileStream, std::__1::default_delete<llvm::CachedFileStream> > > (unsigned int)>, llvm::Module&, llvm::ModuleSummaryIndex const&, llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const&, llvm::DenseMap<unsigned long, llvm::GlobalValueSummary*, llvm::DenseMapInfo<unsigned long, void>, llvm::detail::DenseMapPair<unsigned long, llvm::GlobalValueSummary*> > const&, llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > >*, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > const&)::$_3::operator()(llvm::Module&, llvm::TargetMachine*, std::__1::unique_ptr<llvm::ToolOutputFile, std::__1::default_delete<llvm::ToolOutputFile> >) const ()
#10 0x0000000103000b54 in llvm::lto::thinBackend(llvm::lto::Config const&, unsigned int, std::__1::function<llvm::Expected<std::__1::unique_ptr<llvm::CachedFileStream, std::__1::default_delete<llvm::CachedFileStream> > > (unsigned int)>, llvm::Module&, llvm::ModuleSummaryIndex const&, llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const&, llvm::DenseMap<unsigned long, llvm::GlobalValueSummary*, llvm::DenseMapInfo<unsigned long, void>, llvm::detail::DenseMapPair<unsigned long, llvm::GlobalValueSummary*> > const&, llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > >*, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > const&) ()
#11 0x0000000102fff380 in (anonymous namespace)::InProcessThinBackend::runThinLTOBackendThread(std::__1::function<llvm::Expected<std::__1::unique_ptr<llvm::CachedFileStream, std::__1::default_delete<llvm::CachedFileStream> > > (unsigned int)>, std::__1::function<llvm::Expected<std::__1::function<llvm::Expected<std::__1::unique_ptr<llvm::CachedFileStream, std::__1::default_delete<llvm::CachedFileStream> > > (unsigned int)> > (unsigned int, llvm::StringRef)>, unsigned int, llvm::BitcodeModule, llvm::ModuleSummaryIndex&, llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const&, llvm::DenseSet<llvm::ValueInfo, llvm::DenseMapInfo<llvm::ValueInfo, void> > const&, std::__1::map<unsigned long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, llvm::GlobalValue::LinkageTypes> > > const&, llvm::DenseMap<unsigned long, llvm::GlobalValueSummary*, llvm::DenseMapInfo<unsigned long, void>, llvm::detail::DenseMapPair<unsigned long, llvm::GlobalValueSummary*> > const&, llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > >&)::{lambda(std::__1::function<llvm::Expected<std::__1::unique_ptr<llvm::CachedFileStream, std::__1::default_delete<llvm::CachedFileStream> > > (unsigned int)>)#1}::operator()(std::__1::function<llvm::Expected<std::__1::unique_ptr<llvm::CachedFileStream, std::__1::default_delete<llvm::CachedFileStream> > > (unsigned int)>) const ()
#12 0x0000000102ffe54c in std::__1::__function::__func<std::__1::__bind<(anonymous namespace)::InProcessThinBackend::start(unsigned int, llvm::BitcodeModule, llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const&, llvm::DenseSet<llvm::ValueInfo, llvm::DenseMapInfo<llvm::ValueInfo, void> > const&, std::__1::map<unsigned long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, llvm::GlobalValue::LinkageTypes> > > const&, llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > >&)::{lambda(llvm::BitcodeModule, llvm::ModuleSummaryIndex&, llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const&, llvm::DenseSet<llvm::ValueInfo, llvm::DenseMapInfo<llvm::ValueInfo, void> > const&, std::__1::map<unsigned long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, llvm::GlobalValue::LinkageTypes> > > const&, llvm::DenseMap<unsigned long, llvm::GlobalValueSummary*, llvm::DenseMapInfo<unsigned long, void>, llvm::detail::DenseMapPair<unsigned long, llvm::GlobalValueSummary*> > const&, llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > >&)#1}, llvm::BitcodeModule&, std::__1::reference_wrapper<llvm::ModuleSummaryIndex>, std::__1::reference_wrapper<llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const>, std::__1::reference_wrapper<llvm::DenseSet<llvm::ValueInfo, llvm::DenseMapInfo<llvm::ValueInfo, void> > const>, std::__1::reference_wrapper<std::__1::map<unsigned long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, llvm::GlobalValue::LinkageTypes> > > const>, std::__1::reference_wrapper<llvm::DenseMap<unsigned long, llvm::GlobalValueSummary*, llvm::DenseMapInfo<unsigned long, void>, llvm::detail::DenseMapPair<unsigned long, llvm::GlobalValueSummary*> > const>, std::__1::reference_wrapper<llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > > > >, std::__1::allocator<std::__1::__bind<(anonymous namespace)::InProcessThinBackend::start(unsigned int, llvm::BitcodeModule, llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const&, llvm::DenseSet<llvm::ValueInfo, llvm::DenseMapInfo<llvm::ValueInfo, void> > const&, std::__1::map<unsigned long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, llvm::GlobalValue::LinkageTypes> > > const&, llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > >&)::{lambda(llvm::BitcodeModule, llvm::ModuleSummaryIndex&, llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const&, llvm::DenseSet<llvm::ValueInfo, llvm::DenseMapInfo<llvm::ValueInfo, void> > const&, std::__1::map<unsigned long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, llvm::GlobalValue::LinkageTypes> > > const&, llvm::DenseMap<unsigned long, llvm::GlobalValueSummary*, llvm::DenseMapInfo<unsigned long, void>, llvm::detail::DenseMapPair<unsigned long, llvm::GlobalValueSummary*> > const&, llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > >&)#1}, llvm::BitcodeModule&, std::__1::reference_wrapper<llvm::ModuleSummaryIndex>, std::__1::reference_wrapper<llvm::StringMap<std::__1::unordered_set<unsigned long, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<unsigned long> >, llvm::MallocAllocator> const>, std::__1::reference_wrapper<llvm::DenseSet<llvm::ValueInfo, llvm::DenseMapInfo<llvm::ValueInfo, void> > const>, std::__1::reference_wrapper<std::__1::map<unsigned long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, llvm::GlobalValue::LinkageTypes> > > const>, std::__1::reference_wrapper<llvm::DenseMap<unsigned long, llvm::GlobalValueSummary*, llvm::DenseMapInfo<unsigned long, void>, llvm::detail::DenseMapPair<unsigned long, llvm::GlobalValueSummary*> > const>, std::__1::reference_wrapper<llvm::MapVector<llvm::StringRef, llvm::BitcodeModule, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> >, std::__1::vector<std::__1::pair<llvm::StringRef, llvm::BitcodeModule>, std::__1::allocator<std::__1::pair<llvm::StringRef, llvm::BitcodeModule> > > > > > >, void ()>::operator()() ()
#13 0x0000000103caa248 in std::__1::__function::__func<llvm::ThreadPool::createTaskAndFuture(std::__1::function<void ()>)::{lambda()#1}, std::__1::allocator<llvm::ThreadPool::createTaskAndFuture(std::__1::function<void ()>)::{lambda()#1}>, void ()>::operator()() ()
#14 0x0000000103caba5c in llvm::ThreadPool::processTasks(llvm::ThreadPoolTaskGroup*) ()
#15 0x0000000103cab3e4 in void* llvm::thread::ThreadProxy<std::__1::tuple<llvm::ThreadPool::grow(int)::$_0> >(void*) ()
#16 0x09000000005b304c in _pthread_body () from /usr/lib/libpthreads.a(shr_xpg5_64.o)
#17 0x0000000000000000 in ?? ()

I see that clang/test/Driver/arm-float-abi-lto.c now has a // UNSUPPORTED: system-aix workaround but I think someone working on AIX should investigate the issue.
I think the reported issue isn't actionable to those who don't use AIX.

bmahjour added inline comments.
lld/test/ELF/lto/discard-value-names.ll
4

I see --plugin-opt=opaque-pointers is being explicitly specified for some but not all tests. I suppose the reason you explicitly specify it is to make sure the tests pass even for builds where opaque pointers are disabled. If that's the case, why aren't you doing it consistently?

llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
86

why does this test (and ipa-alias.ll above) need to run with opaque pointers off?

llvm/test/LTO/Resolution/X86/alias-alias.ll
3

Why do you pass -lto-opaque-pointers to llvm-lto2 in clang/tests but not here?

bmahjour added a project: Restricted Project.Fri, Jun 10, 8:18 AM
MatzeB added inline comments.Fri, Jun 10, 8:29 AM
lld/test/ELF/lto/discard-value-names.ll
4

It‘s not specified in any of the lld tests except the two tests for the option itself (because in lld lto its enabled by default now).

It is specified for clang (but not clang cc1) in a coupletests because the clang driver drfaults can currently be changed with a cmake flag.

llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
86

I don‘t remember this test specifically but some were tricky to update, feel free to submit a patch.

@JakeEgan I did a quick look for the code in that stacktrace but nothing jumped out on me. It‘s with inlining effects obscuring it. But I think that crash must be investigated on an AIX machine, there‘s no indication that this directly related to my change which works on all other systems.

Your commit message seems to use the original summary.

Yes sorry, I noticed it too after pushing it. I think this can‘t be fixed after the push though…

Thanks for taking a look. I agree someone on AIX should investigate.