cmm/CBE: Fix comparison between blocks of different lengths
ClosedPublic

Authored by bgamari on Nov 4 2017, 5:36 PM.

Details

Summary

Previously CBE computed equality by taking the lists of middle nodes of
the blocks being compared and zipping them together. It would then map
over this list with the equality relation, and accumulate the result.

However, this is completely wrong: Consider what will happen when we
compare a block with no middle nodes with one with one or more. The
result of zip will be empty and consequently the pass may conclude
that the two are indeed equivalent (if their last nodes also match).
This is very bad and the cause of Trac #14361.

The solution I chose was just to write out an explicit recursion, like I distinctly
recall considering doing when I first wrote this code. Unfortunately I was feeling
clever at the time.

Unfortunately this case was just rare enough not to be triggered by the
testsuite. I still need to find a testcase that doesn't have external
dependencies.

Test Plan

Need to find a more minimal testcase

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bgamari created this revision.Nov 4 2017, 5:36 PM
bgamari edited the summary of this revision. (Show Details)Nov 4 2017, 5:37 PM
hvr awarded a token.Nov 4 2017, 5:48 PM
michalt accepted this revision.Nov 5 2017, 5:28 AM
michalt added a subscriber: michalt.

Oh, this is unexpected - we discussed this problem on one of the previous diffs: D3973#inline-30997 (and it fixed the problem). Unfortunately it seems D3999 re-introduced it. (I guess bad rebase?)

Anyway, looks good to me. :)

This revision is now accepted and ready to land.Nov 5 2017, 5:28 AM
This revision was automatically updated to reflect the committed changes.