diff options
author | Matthew Pickering <matthewtpickering@gmail.com> | 2021-07-21 14:03:23 +0100 |
---|---|---|
committer | Matthew Pickering <matthewtpickering@gmail.com> | 2021-08-02 12:23:18 +0100 |
commit | 4d319d06d5c6d50ab14fb558c6f1d0d7fc55af54 (patch) | |
tree | a06e713d11317180596dd8038a4a0536eafa25bf /compiler/cbits | |
parent | 34e352173dd1fc3cd86c49380fda5a4eb5dd7aef (diff) | |
download | haskell-wip/debug-rules.tar.gz |
Remove eager forcing of RuleInfo in substRuleInfowip/debug-rules
substRuleInfo updates the IdInfo for an Id, therefore it is important to not
force said IdInfo whilst updating it, otherwise we end up in an infinite
loop. This is what happened in #20112 where `mkTick` forced the IdInfo being
updated by checking the arity in isSaturatedConApp. The fix is to stop
the expression being forced so early by removing the call to
seqRuleInfo.
The call sequence looked something like:
* `substRecBndrs`
* `substIdBndr`
* `substIdInfo`
* `substRuleInfo`
* `substRule`
* `substExpr`
* `mkTick`
* `isSaturatedConApp`
* Look at `IdInfo` for thing we are currently substituting because the rule is attached to `transpose` and mentions it in the `RHS` of the rule.
Which arose because the `transpose` Id had a rule attached where the RHS
of the rule also mentioned `transpose`.
This call to seqRuleInfo was introduced in 4e7d56fde0f44d38bbb9a6fc72cf9c603264899d
where it was explained
> I think there are now *too many* seqs, and they waste work, but I don't have
> time to find which ones.
We also observe that there is the ominous note on `substRule` about
making sure substExpr is called lazily.
> {- Note [Substitute lazily]
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> The functions that substitute over IdInfo must be pretty lazy, because
> they are knot-tied by substRecBndrs.
>
> One case in point was #10627 in which a rule for a function 'f'
> referred to 'f' (at a different type) on the RHS. But instead of just
> substituting in the rhs of the rule, we were calling simpleOptExpr, which
> looked at the idInfo for 'f'; result <<loop>>.
>
> In any case we don't need to optimise the RHS of rules, or unfoldings,
> because the simplifier will do that.
Before `seqRuleInfo` was removed, this note was pretty much ignored in
the `substSpec` case because the expression was immediately forced after
`substRule` was called.
Unfortunately it's a bit tricky to add a test for this as the failure
only manifested (for an unknown reason) with a dwarf enabled compiler
*AND* compiling with -g3. Fortunatley there is currently a CI
configuration which builds a dwarf compiler to test this.
Also, for good measure, finish off the work started in
840df33685e8c746ade4b9d4d0eb7c764a773e48 which renamed SpecInfo to
RuleInfo but then didn't rename 'substSpec' to 'substRuleInfo'.
Fixes #20112
Diffstat (limited to 'compiler/cbits')
0 files changed, 0 insertions, 0 deletions