Add ptr-eq short-cut to `compareByteArrays#` primitive
ClosedPublic

Authored by hvr on Jan 17 2018, 5:37 AM.

Details

Summary

This is an obvious optimisation whose overhead is neglectable but
which significantly simplifies the common uses of compareByteArrays#
which would otherwise require to make *careful* use of
reallyUnsafePtrEquality# or (equally fragile) byteArrayContents#
which can result in less optimal assembler code being generated.

Test Plan

carefully examined generated cmm/asm code; validate via phab

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.
hvr created this revision.Jan 17 2018, 5:37 AM
alexbiehl added inline comments.Jan 17 2018, 12:15 PM
compiler/codeGen/StgCmmPrim.hs
1786

You could provide a likeliness annotation (Just False) to inform the codegen about your reasoning here. I heard we are slowly making progress on this front..

bgamari requested changes to this revision.Jan 17 2018, 6:15 PM

@alexbiehl raises a good point; perhaps you could add this? We should also presumably add a note to ghc-prim's changelog.

This revision now requires changes to proceed.Jan 17 2018, 6:15 PM
hvr added inline comments.Jan 18 2018, 6:10 AM
compiler/codeGen/StgCmmPrim.hs
1786

I deliberately didn't do that, because I believe that https://github.com/ghc/ghc/blob/57372a7cc958ebfa4ac64fc800e00baacfc3cf5c/compiler/codeGen/StgCmmMonad.hs#L814 would then do the wrong thing here. :-)

hvr added a comment.Jan 18 2018, 6:19 AM

We should also presumably add a note to ghc-prim's changelog.

As this is actually intended for GHC 8.4.1 there's no need for a changelog entry; I had it originally in the outofline cmm implementation, but didn't get to port it over to the inline primop implementation.

hvr added inline comments.Jan 18 2018, 6:25 AM
compiler/codeGen/StgCmmPrim.hs
1786

@alexbiehl pointed out that mkCmmIfThenElse isn't involved here; nevertheless its weird inversion confused me, cause originally I had been thinking in terms of mkCmmIfThenElse, and if I had written Just False there, the jump would have been rearranged in the wrong way.

Moreover, I based my implementation on

https://software.intel.com/en-us/articles/branch-and-loop-reorganization-to-prevent-mispredicts

which discouraged the use of branch hints:

The Pentium® 4 Processor introduced new instructions for adding static hints to branches. It is not recommended that a programmer use these instructions, as they add slightly to the size of the code and are static hints only. It is best to use a conditional branch in the manner that the static predictor expects, rather than adding these branch hints.

However, I've also been told this information may be outdated by now... *sigh* :-)

hvr updated this revision to Diff 15155.Jan 19 2018, 3:48 AM
  • Add likely=False hint
hvr marked an inline comment as done.Jan 19 2018, 3:50 AM
hvr added inline comments.
compiler/codeGen/StgCmmPrim.hs
1786

I've just added a Just False hint, as there's likely little harm in doing so.

hvr edited the summary of this revision. (Show Details)Jan 21 2018, 3:06 AM
simonmar accepted this revision.Jan 22 2018, 3:30 AM
simonmar added inline comments.
compiler/codeGen/StgCmmPrim.hs
1785

Use zeroExpr here

hvr updated this revision to Diff 15191.Jan 22 2018, 12:03 PM
  • Make use of zeroExpr
hvr marked an inline comment as done.Jan 22 2018, 12:04 PM
hvr added inline comments.
compiler/codeGen/StgCmmPrim.hs
1785

Good catch, thanks! (while at it, I noticed a couple other places which could be refactored to use zeroExpr; I'll likely submit a Diff soon).

bgamari accepted this revision.Jan 22 2018, 12:06 PM
This revision is now accepted and ready to land.Jan 22 2018, 12:06 PM
This revision was automatically updated to reflect the committed changes.