[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:
> [...]

I forgot to post week 5 & 6, so here is week 5-7

Week 5
======

This week
~~~~~~~~~
- Translate _d_arraysetlengthT to template
- _d_arraysetlengthT druntime patch got merged
   - [https://github.com/dlang/druntime/pull/2656]
- Got _d_arraycatnTX to pass all the testcases.
- Updated all the PRs based on feedback.

Current PRs: _d_arraysetlengthT:
- [https://github.com/dlang/dmd/pull/10106]

_d_array{,set}ctor:
- [https://github.com/dlang/druntime/pull/2655]
- [https://github.com/dlang/dmd/pull/10102]

_d_arraycatnTX:
- [https://github.com/dlang/druntime/pull/2648]
- [https://github.com/dlang/dmd/pull/10064]

_d_arrayappend{T,cTx}:
- [https://github.com/dlang/druntime/pull/2632]
- [https://github.com/dlang/dmd/pull/9982]


Next week
~~~~~~~~~
- Work on the feedback I get on the dmd patches, when the 
druntime patch
   have been merged.
- Start working on the _d_arrayliteralTX hook, which was planned 
for
   next week.


Blockers
~~~~~~~~
Catching `Throwable` and then rethrowing makes the function not 
be able
to be deduced as `nothrow`.

Realized that there is no way to easily check if something is 
`nothrow`
using only druntime. Except for `__traits(getFunctionAttributes, 
x)`,
but that requires a lot of code just to check for nothrow.


Week 6
======

This week
~~~~~~~~~
Made two fix PR for `_d_arraysetlengthT*` to disallow the hooks 
to be
inlined, and to force it to generate a *Trace instant for each 
regular
hook. Both of these PRs have been merged.

Updated `_d_array{,set}ctor` based on feedback from thewilsonator.
[https://github.com/dlang/druntime/pull/2655#issuecomment-507923713]

`_d_arraycatnTX` is updated with the sane fixes as 
`_d_arraysetlengthT`.
[https://github.com/dlang/druntime/pull/2648]

Next week
~~~~~~~~~
- Porting over on `_d_arrayliteralTX`.
- Apply fixes to `_d_arrayappendcTX`.
- Work on getting the previous PRs merged.

Blockers
~~~~~~~~
Learned that I need `pragma(inline, false);` for the hooks to not 
break
my CTFE interception, and the bigger thing I had to fight with was
getting the compiler to instantiate both `_d_arraysetlengthT` and
`_d_arraysetlengthTTrace`. Here I tested multiple thing, like
referencing the trace function from the regular hook, but this 
did not
work and only caused template instantiating problems. What I used 
in the
end, which was also the most simple and clean solution, was to 
wrap both
hooks inside of a `template _d_arraysetlengthTImpl` block. This 
caused
both functions to be instantiated, even if only one hook was 
referenced.

All of this was needed because now if you, for example, use
`std.array.Appender!(char[])` which is a template struct, in a 
project
when you compile with the `-profile=gc`. Means that the template 
gets
instantiated but because that template instantiating already exist
inside of phobos, dmd chooses to use that one instance. But 
because that
phobos instance didn't use `_d_arraysetlengthTTrace`, it cause a
`unknown reference to`.  I'm not sure if I explain the problem 
here
correctly, but it was weird and hard to understand enough to be 
able to
make a fix for it.

_d_arraysetlength's dmd PR is currently failing the CI during the
`profilegc`, but I cannot reproduce it locally.


Week 7
======

This week
~~~~~~~~~
Continued to work on my PR to make them into a mergeable state.

'Cleanup and fix implementation of 
_d_array{setlengthT,catnTX}Trace'
- [https://github.com/dlang/druntime/pull/2673]
Fixes some problem with the Trace entrypoint for hooks not calling
`rt.profilegc.accumulate`, and a refactoring to templateize the 
Trace
function to make sure that code isn't duplicated over all the 
Trace
entrypoints.

Finished the dmd PR for _d_arraysetlengthT using all the fixes 
I've been
working on the last week.  It currently waits for someone to do 
the last
review and hopefully merge it.
[https://github.com/dlang/dmd/pull/10106]

Next week
~~~~~~~~~
Pretty much same as last week:
- Finish port of `_d_arrayliteralTX`
- Work on getting the previous PRs merged.
   - The pr for _d_arrayappend* needs to be rebased and integrated 
with
     the new way to implement Trace
     function. [https://github.com/dlang/druntime/pull/2632] Its 
dmd
     patches also need to be turned into a PR.
   - [https://github.com/dlang/dmd/pull/10102] - 
_d_array{,set}ctor and
     [https://github.com/dlang/dmd/pull/10064] - _d_arraycatnTX 
needs to
     be updated to follow how _d_arraysetlengthT was implemented.

Blockers
~~~~~~~~
The previous week I've learned more about every edge-case that 
I've
encountered, and how to fix them.  Like the how I fixed and 
cleaned up
the *Trace implementations, here I had to figured out how to do a 
`pure`
bypass, while still not allowing the compiler to remove calls 
that are
requested, but the optimizer thinks are useless. I solved this by 
`if
(...) assert(0);`, with a condition that I know will never pass, 
and the
optimizer will never remove a `assert(0)`, which forced it to 
allow the
call to still be done.

During the creation I found a way inside of the dmd's source-code 
on how
to bypass pure, without using any `cast()`s. Sadly this was 
classified
by github user aG0aep6G as a bug. Personally I'm a bit sad that 
this
bypass method was removed in 
[https://github.com/dlang/dmd/pull/10172],
as I really though it looked cleaner than using casts, but as 
this is a
bug I cannot argue against removing it.

On the note about bypassing pure I've mentioned that I will 
probably
introduce `HookCallExp` or a bool inside of `CallExp` to allow 
purity to
be ignore, but only for hook calls. If/when I do this I also want 
to add
support to be able to detect hooks that have been inlined, maybe 
by
adding a `HookCompoundStatement` and/or `HookCommaExp`. But 
currently I
want to spend my time to get all the hooks into dmd and druntime, 
and
probably after GSoC start introducing these improvements.

Another problem I encountered after the `HookTraceImpl` PR was 
merged
was that I broke the CTFE intercept code I had for *Trace hooks. 
This
happened because the following code does not introduce a new 
symbol into
the context and it is just alias:
,----
| alias _d_arraysetlengthTTrace = HookTraceImpl!(Tarr, 
_d_arraysetlengthT, errorMessage);
`----
This is something I overlooked when doing the PR, but in the end 
this
was not hard to big and I would say actually lead to better code. 
As
now I have a `removeHookTraceImpl` function inside dinterpret.d:
[https://github.com/dlang/dmd/pull/10106/files#diff-264b29261e215213d3026cd44003c2ebR7452]
,----
| //This function will replace:
| HookTraceImpl!(T, Hook, errMsg).HookTraceImpl(..., parameters)
| // with
| Hook(parameters)
`----
It does this by getting getting the `FuncDeclaration` which in 
this case
is `HookTraceImpl` function, goes to its parent the 
`HookTraceImpl`
template, takes the second template parameters, and uses to 
rewrite the
`CallExp` and replace the `FuncDeclaration` that is used to do 
the CTFE
interception.