[PATCH] D75201: [ThinLTO/LowerTypeTests] Handle unpromoted local type ids

Mon Mar 2 13:19:19 PST 2020

tejohnson added a comment.

Thanks for the reproducer!

In D75201#1901357 <https://reviews.llvm.org/D75201#1901357>, @evgeny777 wrote:

> @tejohnson
>
> > This should happen if the type ID has no use on type metadata, but then I think it should be removed by WPD.
>
> It can't be removed or otherwise handled by WPD, because WPD doesn't even see this type.test (it doesn't exist in the index). The problem is
>  in the analysis phase of thin LTO compilation. I suggest you take a look at `findDevirtualizableCallsForTypeTest` function and then at 
>  `findLoadCallsAtConstantOffset`. The latter can process three kind of vptr users: bitcast, load and gep. It can't process store with constant 
>  expression. So far we don't reflect this type.test in summary index and WPD doesn't see it at all.

Ah ok - I misunderstood what reference you were talking about. Even if findDevirtualizableCallsForTypeTest and its descendants could process the reference on the store, it wouldn't automatically help since we are ultimately only looking for uses on virtual calls for devirtualization. And in this case there is no call. As a result WPD does not create a WPDResolution (which defaults to Unknown resolution), and therefore when we import the type id in LTT it appears Unsat.

I think if I expanded findLoadCallsAtConstantOffset to mark all other uses as HasNonCallUses, and when true added the type test to the list of summarized type tests (similar to the way findDevirtualizableCallsForTypeCheckedLoad and its caller in ModuleSummaryAnalysis.cpp behaves for non-call uses), then that might be enough to provoke the correct behavior from LTT.

Hmm, nope that isn't enough to fix it. I tested this change, rebuilt the index by disassembling v2.bc, manually stripping the index, and rebuilding it via "opt -module-summary". Disassembled again and confirmed that the enclosing function's summary now has a type test summary for it. Re-did the link but still see the unreachable in the function after optimization.

This needs more thought and investigation, so at this point I am going to revert the first patch and this follow on one so I can unblock your builds. I may have to table this work for a couple weeks in any case.

> Here is an object file F11467617: v2.bc <https://reviews.llvm.org/F11467617>
> 
> I'm using the following command to compile
> 
>   lld-link  /dll v2.bc /out:a.dll /force:unresolved /nodefaultlib /lldsavetemps
> 
> 
> Check `??_GRealFile@?A0x1AE555D7@@UEAAPEAXI at Z` function in temps.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75201/new/

https://reviews.llvm.org/D75201