dzhidzhoev wrote: > > > How does this work for LTO? (we'd want all inlined instances to refer to > > > a singular abstract definition) Yeah, looks like that's broken (the > > > abstract origins are not shared - they're duplicated in every translation > > > unit). > > > > > > But the notion of "abstract" subprogram pops up only during DWARF > > generation, doesn’t it? On the level of LLVM IR, we don’t have a way to > > distinguish between abstract and concrete DISubprogram, if I get it right. > > I think it might be useful to think of it that we only have abstract > subprograms - concrete and inlined subprograms exist as a consequence of the > instructions/locations in the llvm functions that define them. > > (long time ago we had subprograms that referred to functions (which became > problematic in LTO - with multiple subprograms referring to the same > deduplicated function, or optimized out functions still having lingering > subprograms), so we inverted it - functions refer to their subprogram - at > which point I think we sort of created the lazily-referenced "abstract" > subprogram by construction, it can be referred to by both/either the concrete > function, or the inlined instructions - it seems to me it is an > abstract/shared description across both of those cases) > > > > We should fix that. We have mangled names on functions like we do for > > > types, so we could use the same approach to deduplicate them via mangled > > > name as a key? (for types this mangled name field was introduced > > > specifically as a deduplication key (I think initially for type units, > > > then for LTO merging, but maybe it was the other way around) - currently > > > the linkage name for functions isn't a load bearing key, but it could > > > become one, or perhaps needs to be a distinct key compared to the mangled > > > name - in the case of internal functions/types they might have a mangled > > > name, but not allow deduplication (for internal linkage types, we omit > > > the mangled name/deduplication key)) > > > > > > It seems something similar is done for declarations of composite type > > member subprograms, according to > > https://llvm.org/docs/LangRef.html#disubprogramdeclaration : > > > When spFlags: DISPFlagDefinition is not present, subprograms describe a > > > declaration in the type tree as opposed to a definition of a function. In > > > this case, the declaration field must be empty. If the scope is a > > > composite type with an ODR identifier: and that does not set flags: > > > DIFwdDecl, then the subprogram declaration is uniqued based only on its > > > linkageName: and scope:. > > Oh, I think that was added for call-site declarations which are deduplicated > correctly/not emitted as unique, etc.... so they're probably uniqued with > each other, but not with the unique definitions from other translation units. > > So maybe the solution is as simple as removing the "unique" from DISubprogram > definitions, and the existing infrastructure will handle it? (maybe have to > expand it to allow it to work for things with DISPFlagDefinition... ) > > (I'm a bit confused about some of that spec wording though - DIFwdDecl is > /not/ set but also DISPFlagDefinition is not present... (a: why do we have > both of those, they seem redundant with each other, b: both being not-present > seems contradictory))
If I understand the idea behind https://github.com/llvm/llvm-project/pull/119001 right, the problem that caused the crashes after https://github.com/llvm/llvm-project/pull/75385 was the case of two distinct DISubporgrams having a local type that gets uniqued during LTO, and these parent subprograms both retain reference to the same local type (It was described here https://github.com/llvm/llvm-project/pull/75385#issuecomment-2386684121). So I was thinking in the opposite direction: should we force unique distinct DISubporgrams containing ODR-uniqued types, as well as it's done for declaration subprograms contained by ODR-uniqued types? https://github.com/llvm/llvm-project/pull/142166 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
