Generate correct X86 and llvm instructions for abs
AbandonedPublic

Authored by idontgetoutmuch on Feb 6 2017, 5:41 AM.

Details

Summary

Previously abs was implemented in Haskell and consequently produced poor assembly.

idontgetoutmuch created this revision.Feb 6 2017, 5:41 AM
idontgetoutmuch added a comment.EditedFeb 6 2017, 5:51 AM

So

	xorps %xmm1,%xmm1
	ucomiss %xmm1,%xmm0
	jp _c41v
	je _c41w
_c41v:
	xorps %xmm1,%xmm1
	ucomiss %xmm1,%xmm0
	ja _c41t
_c41q:
	leaq GHC.Types.F#_con_info(%rip),%rax
	movq %rax,-8(%r12)
	movss _n41H(%rip),%xmm1
	xorps %xmm1,%xmm0
	movss %xmm0,(%r12)
	leaq -7(%r12),%rbx
	addq $-16,%rbp
	jmp *(%rbp)
_c41w:
	addq $-16,%r12
	leaq GHC.Float.rationalToFloat4_closure(%rip),%rbx
	addq $-16,%rbp
	jmp *(%rbx)
_c41t:

is replaced with

	movss _n40P(%rip),%xmm1
	andps %xmm1,%xmm0

llvm actually has llvm.fabs.f32 and llvm.fabs.f64

but what should I do about SPARC and PPC? I'd like to keep the existing Haskell

abs x    | x == 0    = 0 -- handles (-0.0)
         | x >  0    = x
         | otherwise = negateFloat x

for them but I don't think there is a way of saying if isX86 or isLlvm then.... Do I have to generate the existing asm for these? That seems a bit painful.

michalt added a subscriber: michalt.Feb 6 2017, 1:49 PM

[...]

but what should I do about SPARC and PPC? I'd like to keep the existing Haskell

abs x    | x == 0    = 0 -- handles (-0.0)
         | x >  0    = x
         | otherwise = negateFloat x

for them but I don't think there is a way of saying if isX86 or isLlvm then.... Do I have to generate the existing asm for these? That seems a bit painful.

I don't think there's any way to do that in pure Haskell (maybe with some CPP magic?)
But this reminds me of what callishPrimOpSupported from StgCmmPrim.hs is doing. (depending on whether we're compiling to LLVM, X86, etc. it's going to use either MO_* CallishMachOps or just some generic cmm)

bgamari retitled this revision from Generate correct X86 and llvm instructions. to Generate correct X86 and llvm instructions for abs.Feb 6 2017, 6:21 PM
bgamari edited the summary of this revision. (Show Details)
bgamari requested changes to this revision.Feb 6 2017, 6:33 PM

[...]

but what should I do about SPARC and PPC? I'd like to keep the existing Haskell

abs x    | x == 0    = 0 -- handles (-0.0)
         | x >  0    = x
         | otherwise = negateFloat x

for them but I don't think there is a way of saying if isX86 or isLlvm then.... Do I have to generate the existing asm for these? That seems a bit painful.

I don't think there's any way to do that in pure Haskell (maybe with some CPP magic?)

Indeed, let's toss out the Haskell. We should at least try to ensure primops are supported uniformly across platforms. @michalt is exactly right with respect to callishPrimOpSupported. All you need to do is produce a bit of C-- (it looks worse than it is).

This revision now requires changes to proceed.Feb 6 2017, 6:33 PM
carter added a subscriber: carter.Feb 6 2017, 6:42 PM

We shoudln't have fabs primops be marked as foreignCalls when they're not always.... we can do CPP in the primops.pp.txt file right?

compiler/prelude/primops.txt.pp
560

This isn't true on x86/x86_64, perhaps this part of the attribute should be CPP'd?

689

This isn't true on x86/x86_64, perhaps this part of the attribute should be CPP'd?

carter requested changes to this revision.Feb 6 2017, 6:43 PM

i think that we need to discuss what foreignPrimops mean, and or have that metadata be accurate per target architecture

In D3091#90670, @carter wrote:

We shoudln't have fabs primops be marked as foreignCalls when they're not always.... we can do CPP in the primops.pp.txt file right?

I honestly don't know how much fixing this will affect compilation but in principle this is true.

compiler/prelude/primops.txt.pp
560

This file actually does run through the preprocessor so I suppose it wouldn't be so hard to fix this. However, I think we might want to remain consistent with the surroundings here and fix the code sizes in a new Diff.

If we are going to start reporting sizes correctly then someone should just do a comprehensive review of all of the existing primops as well. It would be nice if it were easier to ensure this file is consistent with the primop implementations.

I understand and will now undertake attempt 3. My notes should make an interesting blog. Thanks for the help @carter @bgamari @michalt.

[...]

but what should I do about SPARC and PPC? I'd like to keep the existing Haskell

abs x    | x == 0    = 0 -- handles (-0.0)
         | x >  0    = x
         | otherwise = negateFloat x

for them but I don't think there is a way of saying if isX86 or isLlvm then.... Do I have to generate the existing asm for these? That seems a bit painful.

I don't think there's any way to do that in pure Haskell (maybe with some CPP magic?)

Indeed, let's toss out the Haskell. We should at least try to ensure primops are supported uniformly across platforms. @michalt is exactly right with respect to callishPrimOpSupported. All you need to do is produce a bit of C-- (it looks worse than it is).