[GSoC] 'Replace Runtime Hooks with Templates' progress and update thread

[Dan Printzell]

On Tuesday, 28 May 2019 at 10:47:03 UTC, Dan Printzell wrote:
> [...]

Sorry, I've been busy with getting everything into a workable to 
state
so I've been late to with posting these updates. So this time it 
is for
the last three weeks. There is also some planning for the final
submission at the bottom.


Week 31
=======

This week
~~~~~~~~~
Last week I've been trying a lot to use the `__ctfe` variable to 
detect
when to call the rewritten hook, verses when to call the original
expression. As discussed, the old CTFE interception turned into a 
big
edge-case friendly place where it needs to be able to detect all 
the
different ways the AST could look when executing a hook.

Sadly lowering `this.data ~= val` to ` __ctfe ? this.data ~= val :
_d_arrayappendT(this.data, val)` didn't work as it threw the 
following
assertion in the backend. `dmd: dmd/backend/symbol.d:1170: 
Assertion
'(*s).Ssymnum == -1' failed.`.

I have not really made any progress on the next hooks or the 
previous
hooks as I wanted to get the DMD part done first. It is in my 
opinion
pretty useless to add translated hooks into druntime if their dmd 
part
won't work. As adding unused code is still adding maintenance 
cost to
druntime.


I've marked all the current dmd PR as WIP, as they should all 
move to a
better way of detecting CTFE. I've also update
[https://definewild.se/public/Hooks.html] to reflect the current 
state
of all the hooks.

Next week
~~~~~~~~~
- Try and figure how to use CondExp, if it is even possible.
- Maybe rethink my time plan. Should I focus on getting more 
druntime
   hook merged, or should I focus on getting the current merged 
hooks
   working? (The later is probably the better choice.)

Same as last week:
- Finish port of `_d_arrayliteralTX`
- Start working on the `_d_delarray_t*`, `_d_arrayassign*`, and
   `_d_arraysetassign` translations.
- Work on getting the previous PRs merged.
   - The PR for _d_arrayappendcTX need to more logic on how to 
lower the
     appending, and more logic on how to interpret the lowered
     expressions.
   - _d_array{,set}ctor need to implements some fixes to get the 
CTFE to
     work correctly.
   - _d_arraycatnTX need fixes related to the CTFE interpretion. 
Also
     need to fix the code based on the received feedback.

Blockers
~~~~~~~~
The assert problem seem to have something to do with the `val` 
part
getting rewritten. Maybe dmd doesn't support DeclarationExp 
inside of
CondExp?

```
e2ir.d, visit(CondExp de): __ctfe ? this.data ~= 
((InversionList!(GcPolicy) __copytmp4144 = (__copytmp4144 = 
val).__fieldPostblit();) , __copytmp4144) : 
((InversionList!(GcPolicy) __appendtmp4145 = 
(InversionList!(GcPolicy) __copytmp4144 = (__copytmp4144 = 
val).__fieldPostblit();) , __appendtmp4145 = __copytmp4144;) , 
(_d_arrayappendcTX(this.data, 1LU) , this.data[__dollar - 1LU] = 
__appendtmp4145) , this.data)
e2ir.d, visit(CommaExp ce): (InversionList!(GcPolicy) 
__copytmp4106 = (__copytmp4106 = val).__fieldPostblit();) , 
this.data[__dollar - 1LU] = __copytmp4106
e2ir.d, visit(DeclarationExp de): (InversionList!(GcPolicy) 
__copytmp4106 = (__copytmp4106 = val).__fieldPostblit();)
dmd: dmd/backend/symbol.d:1170: Assertion `(*s).Ssymnum == -1' 
failed.
```



Week 32
=======

This week
~~~~~~~~~
*_d_arrayappend{T,cTX}:* Required druntime fixes:
[https://github.com/dlang/druntime/pull/2718] Update dmd patches:
[https://github.com/dlang/dmd/pull/9982] This uses the new 
`__ctfe ?
orgExp : loweredExp` technique instead of intercepting CTFE 
calls. I
also added the fix to the assert
[https://github.com/dlang/dmd/pull/9982/commits/2d81b673424175ae5304f2eee7dcb7bd87d43cc3]
And I made the backend only compile the `!__ctfe` path to machine 
code,
to be able to `assert(0)` the calls to the old hook functions:
[https://github.com/dlang/dmd/pull/9982/commits/1f4d9bf17033cee8e2458c3d1c35a52632caa6f8]

It took some fighting to get the hook working, because I 
triggered some
asserts in the codegen. But I was able to fix / work around all 
of them.

Next week
~~~~~~~~~
- Update `_d_arrayappend{T,cTX}` PRs based on feedback.
- Fix `_d_array{,set}ctor`.
- Fix `_d_arraycatnTX`.

- Figure out what is needed for the final submission.

Blockers
~~~~~~~~
The current problem with `_d_array{,set}ctor` is that it doesn't 
call
all the dtors. It expects to find this in the output `order`:

```
     copy inner #1
     copy inner #1
     copy inner #1
     copy outer
     copy inner #1
     copy inner #2
     destroy inner #1
     destroy outer
     destroy inner #1
     destroy inner #1
     destroy inner #1
```
But only this is found:
```
     copy inner #1
     copy inner #1
     copy inner #1
     copy outer
     copy inner #1
     copy inner #2
     destroy inner #1
```

Unittest:
[https://github.com/dlang/druntime/blob/master/src/object.d#L3647]

I think the problem is that DeclarationExp/VarDeclaration, doesn't
handle `scope (failure)` correctly. Currently I'm only rewriting 
the
AssignExp that is inside the `VarDeclaration`. My current theory 
is that
if I rewrite the DeclarationExp instead the hook may start working
against.


I have not look into `_d_arraycatnTX` issues this week. But using 
the
new `__ctfe ? :` way of restoring the CTFE functionality should
hopefully allow me to remove some hacks I had to implement. Which 
in
turn will allow the PR to not break any existing code.



Week 33
=======

This week
~~~~~~~~~
*_d_arrayappend{T,cTX}:* Remove the debug `message()` calls.

*_d_array{,set}ctor:* Fixed the hook by doing a weird hack I 
don't fully
understand why it is needed. Not sure if it is a dmd bug that 
could
happen in usercode or if it is just because it is called inside 
of a
`DeclarationExp._init`.
[https://github.com/Vild/druntime/blob/FixArrayCtorHook/src/core/internal/array/construction.d#L59]

I have not PR this yet as I'm currently depending on the PR I 
submitted
last week related to _d_arrayappend,
[https://github.com/dlang/druntime/pull/2718]. I may merge these 
changes
into that PR.

*_d_arraycatnTX:* I've figured out the currently problem with why 
cannot
compile this:
[https://github.com/dlang/dmd/blob/b2522da8566783491648bc104a29b42dc2dc569e/src/dmd/mars.d#L2174]

It is because the somewhere the `commited` member of StringExp 
get set
to `1` even if the CTFE was able to run the call at compile and 
get a
single StringExp out of it.

Next week
~~~~~~~~~
- Fix the last changed to `_d_arraycatnTX`.
- Get `_d_arrayappend{T,cTX}`, `_d_array{,set}ctor`, and
   `_d_arraycatnTX`.
- Work on the writeup on what I've learned etc, that I lifted 
last week
   update.

Blockers
~~~~~~~~
*_d_array{,set}ctor:* Here is just some of the differences I 
found in
GHIDRA when I compared compile with and without the hack.  The
differences I could find of the hook is that the working one have 
two
try-blocks ([https://i.definewild.se/3377]) and the loop looks a 
bit
different ([https://i.definewild.se/165e] left is non-working, 
right is
working).

*_d_arraycatnTX:* The biggest blocker right now is how I should 
solve
the `_d_arraycatnTX` problem. But I don't think it will be too 
bad, as
now I've been able to identify where the problem is. Now I just 
need to
compare what code paths the different concatenations goes though, 
and
why they differ.

```
enum fourfivesix = "456";
enum world = () {
     string s;
     s ~= "world!";
     return s;
}();
void test(const(char)* fmt) {}
void main() {
     test("derp" ~ " herp"); // Works
     test("123 " ~ fourfivesix); // Works
     test("hello " ~ world); // Doesn't
     /* Error: function cattesting.test(const(char)* fmt) is not 
callable using argument types (string)
      *      cannot pass argument "hello world!" of type string to 
parameter const(char)* fmt
      */
}
```



Planning for final submission
=============================

I will not be able to finish the hooks all the hooks I listed in 
my
proposal. Instead I should probably just focus on finishing the 
current
WIP hook: `_d_arrayappend{T,cTX}`, `_d_array{,set}ctor`, and
`_d_arraycatnTX`. If I finish these I will have successed with
translating four out of nine hooks (the forth hook is 
_d_arraysetlength
which is already merged).

Furthermore as a final submission it probably should include some
documentation on how to do the translation process, to help aid 
others
to translate hooks. This could include the following 
sections/topics:
- Finding a hook to translate
- Where to add the new code, how to update the buildsystem
- How to manage the CTFE.
- Navigating the codebase: Which dmd files are relevant? Where 
should I
   look for class X?
- Examples of previous translations
   - with notes on how they were updated / fixed
- Debugging technique
- How to implement a -profile=gc entrypoint.

I'm not sure where the best place for this would be as this is 
more for
new compile developers than the average D user. I guess the best 
place
would be the wiki.