Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

[WIP] Lower newExp #2508

Closed
wants to merge 2 commits into from
Closed

[WIP] Lower newExp #2508

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33b1ea3
Lower newExp
edi33416 Mar 8, 2019
4cabd7c
Use T to check if isZeroInitialized
edi33416 Mar 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions src/object.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,68 @@ alias dstring = immutable(dchar)[];

version (D_ObjectiveC) public import core.attribute : selector;

// Size used to store the TypeInfo at the end of an allocation for structs that
// have a destructor
private size_t structTypeInfoSize(T)() pure nothrow @nogc
{
// can't use because it's in rt.lifetime
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about this? Any solutions?

Copy link
Contributor

Choose a reason for hiding this comment

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

The simplest would be to make callStructDtorsDuringGC package and import rt.lifetime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't import rt.lifetime. You can't import anything from rt/* since it's not in mak/COPY's $(IMPDIR).

Maybe I'm missing something, as the GC is able to import from rt/util/

Copy link
Contributor

Choose a reason for hiding this comment

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

Then you can do a hack, declare an extern variable and use pragma(mangle) to get the same mangling as for callStructDtorsDuringGC declared in rt.lifetime.

Copy link
Member

Choose a reason for hiding this comment

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

We can do even better. Here's an example from the existing code:

import core.internal.traits : externDFunc;
alias rt_tlsgc_init = externDFunc!("rt.tlsgc.init", void* function() nothrow @nogc);

Copy link
Contributor

@jacob-carlborg jacob-carlborg Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Does that work with non-function types? That won't work for non-function types. The template explicitly checks for a function type.

Copy link
Contributor

@JinShil JinShil Apr 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

You could add an extern (C) function in lifetime.d to return callStructDtorsDuringGC and then just declare that prototype in object.d. You can already find precedent for that technique in object.d (e.g.

druntime/src/object.d

Lines 3909 to 3910 in 4ad638f

extern (C) void _d_arrayshrinkfit(const TypeInfo ti, void[] arr) nothrow;
extern (C) size_t _d_arraysetcapacity(const TypeInfo ti, size_t newcapacity, void[]* arrptr) pure nothrow;
)

Though, this isn't much different than @jacob-carlborg's suggestion.

//if (!callStructDtorsDuringGC)
//return 0;

static if (is(T == struct) && is(typeof({ T t; t.__xdtor; })))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the same as TypeInfo_Struct.xdtor()?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid I don't know at this time. I'm sorry.

return size_t.sizeof;
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will complain about unreachable statement if the above static if is true.

}

/*
* Allocate an uninitialized non-array item.
Copy link
Contributor

Choose a reason for hiding this comment

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

What's an item? It's not very descriptive.

* This is an optimization to avoid things needed for arrays like the __arrayPad(size).
*/
private T* ___d_newitemU(T)()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these names, ___d_newitemU and ___d_newitemT, are very descriptive. It's difficult to know what's the difference just by looking at their names. Perhaps ___d_newitemU instead should contain the word "allocate".

{
import core.memory;
import core.internal.traits : Unqual;

auto ti = typeid(Unqual!T);
auto flags = !(ti.flags & 1) ? GC.BlkAttr.NO_SCAN : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the whole point was to avoid typeid? Surely you should be able to use a compiler trait for ti.flags & 1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Otherwise one can be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't found a compiler trait. Maybe I missed it. Anyone know about one?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, not that I can see. But add one. I think it's currently handled here: https://github.com/dlang/dmd/blob/bf26fb4ffc173f016eae9c241bcfd5a88da92957/src/dmd/toobj.d#L1291-L1327.

Copy link
Contributor

@JinShil JinShil Apr 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't know what (ti.flags & 1) meant, so I had to go spelunking in DMD source code to see what it means. Assuming I'm reading the DMD source code correctly, it's isCOMclass. At a minimum maybe you could do something like enum COMClass = 1 to save the next poor soul the trouble.

Second, according to https://dlang.org/spec/interface.html#com-interfaces does that flag mean that it inherits from core.stdc.windows.com.IUnknown? Is there an existing trait that can check for that? Or can you use __traits(compiles,...) to achieve the desired result?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's really bad:

druntime/src/object.d

Lines 1313 to 1315 in 4ad638f

/** Get flags for type: 1 means GC should scan for pointers,
2 means arg of this type is passed in XMM register */
@property uint flags() nothrow pure const @safe @nogc { return 0; }

Especially as there is this easily confusing beauty (used for TypeInfo_Class.m_flags) you referred to:

druntime/src/object.d

Lines 1969 to 1980 in 4ad638f

enum ClassFlags : uint
{
isCOMclass = 0x1,
noPointers = 0x2,
hasOffTi = 0x4,
hasCtor = 0x8,
hasGetMembers = 0x10,
hasTypeInfo = 0x20,
isAbstract = 0x40,
isCPPclass = 0x80,
hasDtor = 0x100,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

druntime/src/object.d

Line 2194 in 4ad638f

override @property uint flags() nothrow pure const { return m_flags; }

Now I'm thoroughly confused.

Copy link
Member

Choose a reason for hiding this comment

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

This one is for structs, not classes. TypeInfo_Class.flags() always returns 1 as TypeInfo_Class tries to describe both the reference and the instance of a class at the same time (which is causing trouble elsewhere, too, e.g. when trying to specify the type of memory for the precise GC).

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an existing trait that can check for that? Or can you use __traits(compiles,...) to achieve the desired result?

I guess is(T : IUnknown) can be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using magic constants like this 1.

enum tiSize = structTypeInfoSize!T();
enum size = T.sizeof + tiSize;

static if (is(T == struct))
{
if (tiSize)
flags |= GC.BlkAttr.STRUCTFINAL | GC.BlkAttr.FINALIZE;
}
auto blkInf = GC.qalloc(size, flags, ti);
auto p = blkInf.base;

static if (is(T == struct))
{
if (tiSize)
*cast(TypeInfo*)(p + blkInf.size - tiSize) = cast() ti;
}
return cast(T*) p;
}

// Same as above, for item with non-zero initializer.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a public symbol, should it has a Ddoc comment?

T* ___d_newitemT(T)()
{
import core.stdc.string;
auto p = ___d_newitemU!T();
memset(p, 0, T.sizeof);
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment says "for item with non-zero initializer". But doesn't this initialize to zero?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a problem of using T.init when it's zero? Why can't the function below be used for all types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe memset is faster than memcpy? Not sure; I'll do some benchmarking

Copy link
Member

@dnadlinger dnadlinger Mar 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Zero initialisers didn't actually get emitted as such; the pointer then is zero. This only applies to TypeInfo, of course.

return p;
}

// Same as above, for item with non-zero initializer.
T* ___d_newitemiT(T)()
{
import core.stdc.string;
auto p = ___d_newitemU!T();
auto init = T.init;
Copy link
Contributor

Choose a reason for hiding this comment

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

const.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe static const even?

memcpy(p, &init, T.sizeof);
return p;
}

int __cmp(T)(scope const T[] lhs, scope const T[] rhs) @trusted
if (__traits(isScalar, T))
{
Expand Down