Look through casts when calculating dict arg discount
AbandonedPublic

Authored by mpickering on Dec 29 2017, 4:17 PM.

Details

Reviewers
bgamari
Summary

The dictionary argument discount was not being applied for code such as rec_sel (dict_arg 'cast' cobox).

Rather than modify the desugaring which introduces these casts, it is more robust to just look through coercions when deciding whether we should give the discount.

mpickering created this revision.Dec 29 2017, 4:17 PM
mpickering edited the summary of this revision. (Show Details)Dec 29 2017, 4:34 PM
duog added a subscriber: duog.Jan 1 2018, 4:14 PM
duog added inline comments.
compiler/coreSyn/CoreUnfold.hs
721–728

I suppose nested Casts can't happen? Even so it seems more robust to handle them, maybe:

arg_discount = let go x = case x of
  Var dict | dict `elem` top_args -> unitBag (dict, ufDictDiscount dflags)
  Case e _ -> go e
  _ -> emptyBag
  in go arg1
mpickering updated this revision to Diff 15001.Jan 1 2018, 4:37 PM
mpickering edited the summary of this revision. (Show Details)
  • Doug's refactoring suggestion
mpickering marked an inline comment as done.Jan 1 2018, 4:38 PM
mpickering added inline comments.
compiler/coreSyn/CoreUnfold.hs
721–728

Thanks, good suggestion.

bgamari accepted this revision.Jan 7 2018, 12:11 PM

Looks good to me.

This revision is now accepted and ready to land.Jan 7 2018, 12:11 PM

Strangely I am seeing this fail with,

=====> cast-unfolding(normal) 1 of 1 [0, 0, 0]
cd "./simplCore/should_compile/cast-unfolding.run" && $MAKE -s --no-print-directory cast-unfolding  
Actual stdout output differs from expected:
diff -uw "./simplCore/should_compile/cast-unfolding.run/cast-unfolding.stdout.normalised" "./simplCore/should_compile/cast-unfolding.run/cast-unfolding.run.stdout.normalised"
--- ./simplCore/should_compile/cast-unfolding.run/cast-unfolding.stdout.normalised	2018-01-15 13:47:23.217846563 -0500
+++ ./simplCore/should_compile/cast-unfolding.run/cast-unfolding.run.stdout.normalised	2018-01-15 13:47:23.217846563 -0500
@@ -1,6 +1,2 @@
-[30 30 30 30 30 30 0]
-[0]
-[0]
-[0]
 [0]
 [0]
mpickering marked an inline comment as done.Jan 15 2018, 2:24 PM

Strangely I am seeing this fail with,

=====> cast-unfolding(normal) 1 of 1 [0, 0, 0]
cd "./simplCore/should_compile/cast-unfolding.run" && $MAKE -s --no-print-directory cast-unfolding  
Actual stdout output differs from expected:
diff -uw "./simplCore/should_compile/cast-unfolding.run/cast-unfolding.stdout.normalised" "./simplCore/should_compile/cast-unfolding.run/cast-unfolding.run.stdout.normalised"
--- ./simplCore/should_compile/cast-unfolding.run/cast-unfolding.stdout.normalised	2018-01-15 13:47:23.217846563 -0500
+++ ./simplCore/should_compile/cast-unfolding.run/cast-unfolding.run.stdout.normalised	2018-01-15 13:47:23.217846563 -0500
@@ -1,6 +1,2 @@
-[30 30 30 30 30 30 0]
-[0]
-[0]
-[0]
 [0]
 [0]

Which commit are you building from?

bgamari requested changes to this revision.Jan 26 2018, 12:54 PM

My most recent attempt was based on 40c753f14b314e74723465e6f79316657307f373. Bumping out of the merge queue while @mpickering investigates.

This revision now requires changes to proceed.Jan 26 2018, 12:54 PM