Let the simplifier know that seq# forces
ClosedPublic

Authored by dfeuer on Jun 5 2018, 11:48 AM.

Details

Summary

Add a special case in simplAlt to record that the result of
seq# is in WHNF.

dfeuer created this revision.Jun 5 2018, 11:48 AM
dfeuer updated this revision to Diff 16726.Jun 5 2018, 12:41 PM

Improve comment

dfeuer updated this revision to Diff 16727.Jun 5 2018, 12:48 PM

Remove likely-unnecessary bang pattern

Ok by me, but maybe @simonpj knows a cleaner way to do it.

Fine, modulo some refactoring

compiler/simplCore/Simplify.hs
2605–2606

I think this is the Right place to do this, but

  • Could you put the whole add_evals thing in a separate top-level function, passing scrut, con and vs
  • Add proper Note to explain
  • Use collectArgs to get to the function rather than matching on Apps
dfeuer updated this revision to Diff 16734.Jun 5 2018, 9:02 PM
  • Refactor
dfeuer marked an inline comment as done.Jun 5 2018, 9:05 PM

@simonpj, I didn't really want to use collectArgsTicks because it seems a bit expensive for the job. So for the moment I've written a function to just drop the arguments and ticks I need to make the comparison. Your other refactor request was definitely a major improvement. Thanks.

dfeuer updated this revision to Diff 16735.Jun 5 2018, 9:07 PM
  • Refactor
simonpj accepted this revision.Jun 6 2018, 3:03 AM

Much nicer!

compiler/coreSyn/CoreSyn.hs
2050

Or, even simpler, getAppHead, which fiinds the non-App head of a chain of apps. We don't really need to have the right number of args in this case. But it's not a big deal

compiler/simplCore/Simplify.hs
2678

Make this into a top-level function and call it from the isUnboxedTupleCon case.

This revision is now accepted and ready to land.Jun 6 2018, 3:03 AM
dfeuer added inline comments.Jun 6 2018, 9:13 AM
compiler/coreSyn/CoreSyn.hs
2050

Even getAppHead seems slightly more expensive than necessary. If there are more than four arguments, we can immediately conclude that we don't have seq#.

compiler/simplCore/Simplify.hs
2678

I certainly should call it for the unboxed tuple case. If you want it top-level I won't complain too much, but the function name will end up a substantial fraction of the size of the function body.

dfeuer updated this revision to Diff 16738.Jun 6 2018, 9:37 AM
  • Pull out zapper; improve panic message
dfeuer updated this revision to Diff 16743.Jun 6 2018, 1:32 PM
dfeuer marked an inline comment as done.
  • Add test
This revision was automatically updated to reflect the committed changes.