Too aggressive duplicate code culling

[kinke]

As we know since #2168 at least, there's too much code culling going on. There, it was part of a performance problem due to less inlining opportunities. Weka apparently need to weaken the culling to get around missing symbols. Even non-unittest Phobos needs to be compiled all-at-once to prevent missing symbols since 2.074.

And now it appears to cause a crash for another code base due to a required TypeInfo being wrongly discarded as speculative, once again only when compiling that module separately. It compiles fine with DMD 2.074.0 though.

[dnadlinger]

And now it appears to cause a crash for another code base due to a required TypeInfo being wrongly discarded as speculative, once again only when compiling that module separately. It compiles fine with DMD 2.074.0 though.

I don't think that mechanism can directly lead to crashes.

[kinke]

ldc2: /home/martin/ldc/ir/irvar.cpp:44: IrGlobal* getIrGlobal(VarDeclaration*, bool): Assertion `decl->ir->irGlobal != NULL' failed.
#8 0x00000000006d4d6f (/home/martin/build-ldc/bin/ldc2+0x6d4d6f)
#9 0x00000000006a9d84 DtoTypeInfoOf(Type*, bool) (/home/martin/build-ldc/bin/ldc2+0x6a9d84)
#10 0x00000000006c72fe (anonymous namespace)::DtoArrayEqCmp_impl(Loc&, char const*, DValue*, DValue*, bool) [clone .constprop.146] (/home/martin/build-ldc/bin/ldc2+0x6c72fe)
#11 0x00000000006c8952 DtoArrayEquals(Loc&, TOK, DValue*, DValue*) (/home/martin/build-ldc/bin/ldc2+0x6c8952)
#12 0x0000000000659970 ToElemVisitor::visit(EqualExp*) (/home/martin/build-ldc/bin/ldc2+0x659970)

for DtoTypeInfoOf():

  getTypeInfoType(type, nullptr);
  TypeInfoDeclaration *tidecl = type->vtinfo;
  assert(tidecl);
  Declaration_codegen(tidecl); // no codegen - 'speculative type'
  assert(getIrGlobal(tidecl)->value != NULL); // asserts in getIrGlobal(), as tidecl->ir->irGlobal is null
  LLConstant *c = isaConstant(getIrGlobal(tidecl)->value); // crashes

[dnadlinger]

Obviously it can cause a crash, sorry. What I meant to say is that the bugs in the logic that cause the other problems shouldn't be more than coincidentally related to any crashes – those issues (e.g. as reported on the DMD Bugzilla) are caused by the compiler correctly executing an incorrect decision to discard a symbol.

This, on the other hand, looks like an inconsistency between various parts of the AST – requestTypeInfo should be set on the struct in question (assuming it is a struct).

[kinke]

I added some printfs in the SpeculativeTypeVisitor; class std.parallelism.TaskPool.map!(adjustTagsInRange).map!(Zip!(MapResult!(__lambda3, Zip!(Result, Repeat!ulong)), Repeat!(SamHeaderMerger), Repeat!ulong)).map.Map is instantiated but needsCodegen() returns false. -allinst gets it to work.

[dnadlinger]

From the backtrace you posted, it looks like the TypeInfo is needed for an array comparison. This should either cause the frontend to set requestTypeInfo on the StructDeclaration (which then in turn wouldn't be regarded as speculative), or we shouldn't be using the TypeInfo for codegen.

[kinke]

Just for clarification, the type whose TypeInfo is required is a class, not a struct.

[kinke]

The TypeInfo thingy was a more general issue and has been fixed in the meantime. The rest is too vague and most likely a general front-end issue.