Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-betterC #2365

Merged
merged 7 commits into from Oct 31, 2017
Merged

-betterC #2365

merged 7 commits into from Oct 31, 2017

Conversation

kinke
Copy link
Member

@kinke kinke commented Oct 13, 2017

No description provided.

gen/runtime.cpp Outdated
global.params.targetTriple->isOSDarwin()
? llvm::ArrayRef<Type *>({voidPtrTy, voidPtrTy, uintTy, voidPtrTy})
: llvm::ArrayRef<Type *>({voidPtrTy, voidPtrTy, uintTy}),
{}, Attr_Cold_NoReturn);
Copy link
Member Author

@kinke kinke Oct 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess adding nounwind would be appropriate?

Edit: added.

@kinke
Copy link
Member Author

kinke commented Oct 13, 2017

Wrt. the TypeInfos to be omitted for -betterC, I stumbled upon DtoTypeInfoOf() again (recently for this issue as well), which fails horribly if the TypeInfo isn't emitted (LDC assertion or segfault; happens for DCompute, if the type is deemed speculative or if we would simply skip its emission for -betterC there).
So to handle the many code parts relying on DtoTypeInfoOf() gracefully, I think we should forward-declare the IR global at least, so that linker errors will result if the TypeInfo is needed but unavailable.

While thinking about this, I seemed to recall reading on the forum that GDC handles things differently, only emitting the TypeInfos 'on-demand', so that people don't necessarily need to resort to -betterC for less bloated binaries. AFAICT, we currently emit a struct's TypeInfo incl. its helper functions [exclusively] in its (instantiating) module(s), which may account for bloat (pulling in that object with all of its dependencies during linking [and wrt. the template-culling issue, this may be an arbitrary object where the struct has been instantiated - or none at all :D] and/or bloating the objects with unneeded TypeInfos and functions).
I like the idea of emitting a struct's TypeInfo and its helper functions [only] into each module which references it. Edit: This would also allow normal D code to make use of TypeInfos for types defined in -betterC libraries.
Edit2: A quick test reveals that that's apparently how we handle TypeInfos for non-templated structs already. ;) Edit3: I previously tested with a dirty version of mine... - currently, the struct TypeInfo seems to be emitted in the owning module even if unused, and additionally in the module referencing it. The first one seems bogus.

I haven't looked at the ClassInfos yet.

@kinke
Copy link
Member Author

kinke commented Oct 14, 2017

I'm nowhere near the end of this rabbit hole, but I gave up on skipping the TypeInfo emission in the declaring module for now, as redundant TypeInfos in other objects still depend on the struct's init symbol, which is only emitted in the declaring module etc. That's the case for -betterC as well btw; DMD emits it too.

I guess there was a historic reason for building extra 'typeid(...)' LL types for the TypeInfos, similar as the custom types for the init symbols which I got rid of not too long ago, but I found none that justifies them any longer, only complicating stuff and uglifying the IR due to more overall types and pointer bitcasts.

Issue #2357 is fixed by the 5th commit - at least the module can be compiled without issues; not sure if that ClassInfo is really defined in another object so that linking will succeed with Debian's separate-compilation-setup.

@kinke
Copy link
Member Author

kinke commented Oct 14, 2017

Oh please don't tell me the reason for the 'typeid(...)' types were opaque structs. I hoped to get away by skipping their TypeInfo definition, but runnable/testtypeid then fails. LDC 1.4 emits a weird thing, basically a TypeInfo (vtable + monitor only), but with TypeInfo_Struct's vtable (but none of its ~13 extra fields). DMD defines it like this too.

Edit: Extended to a full dummy TypeInfo_Struct.

Instead of druntime's _d_assert[_msg], _d_arraybounds and
_d_switch_error.

Tested by dmd-testsuite's runnable/cassert and compilable/betterCarray.
There's a functional change here: previously, the direct calls to
`TypeInfo...Declaration_codegen()` bypassed the check whether the
TypeInfo can be omitted due to the described type being speculative,
which is performed for normal `Declaration_codegen()` calls only.
The latter is used by `DtoTypeInfoOf()`.
Use the real LL type representing the TypeInfo (sub)class directly and
from the beginning, i.e., already for the declaration, instead of
building an extra LL struct type (firstly opaque, then finalized when
defining the TypeInfo via RTTIBuilder).

To get this to work in all cases, the dummy TypeInfo for opaque structs
is now a complete TypeInfo_Struct, with all 11/13 fields zero-
initialized. Previously, it was a special TypeInfo (no extra
TypeInfo_Struct fields) with TypeInfo_Struct vtable, something which DMD
also emits.

Also refactor the RTTIBuilder finalize() interface - e.g., don't set the
linkage there (had to be reverted for ModuleInfos) and only take the
global variable to be defined. Cast the field initializers if required,
e.g., null pointers to appropriately typed function pointers for
TypeInfo_Struct etc.
@kinke kinke changed the base branch from merge-2.076 to master October 30, 2017 19:51
@kinke kinke changed the title [2.076] -betterC -betterC Oct 30, 2017
@kinke kinke added this to the 1.6.0 milestone Oct 30, 2017
@kinke kinke merged commit c1f921e into ldc-developers:master Oct 31, 2017
@kinke kinke deleted the betterC branch October 31, 2017 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant