Generate better fp abs for X86 and llvm.
AbandonedPublic

Authored by idontgetoutmuch on Feb 9 2017, 2:23 PM.

Details

idontgetoutmuch created this revision.Feb 9 2017, 2:23 PM
carter added a subscriber: carter.Feb 9 2017, 3:00 PM

overall looks decent, just some cleaning still needed :)

compiler/cmm/PprC.hs
757

:(

compiler/nativeGen/PPC/CodeGen.hs
1528

:(

compiler/nativeGen/SPARC/CodeGen.hs
613

:(

631

:(

compiler/nativeGen/X86/CodeGen.hs
2071

This seems suspicious ... this could easily be done as a case on the right hand side and then we'd have nice coverage warnings if floating datdamodels get changed in the future

2075

this is duplicated code structure? maybe we should CSE up those patterns of code? @bgamari thoughts?

2602

:(

2619

:(

i think a cleaner approach might be to add Float 32 and Double 64 bit wise operations (and the associated Data.Bits instance) and use the same machinery to define fabs and fabsf

@rwbarton @bgamari thoughts?

I agree to some extent, but are there other uses for those bitwise operations? Implementing them generically and efficiently doesn't sound very easy, while the fallback code for fabs here is reasonable.

bgamari edited edge metadata.Feb 14 2017, 12:11 PM

Bumping out of review queue for now.

compiler/codeGen/StgCmmPrim.hs
1103

These also need to be fixed.

compiler/nativeGen/X86/CodeGen.hs
2075

Where is this repeated? I'm not seeing anything similar in the vicinity.

2082

For the record, this anyReg business appears to be a no-op. I believe these last two lines are equivalent to,

return $ code (getRegisterReg platform True (CmmLocal r))

I may have missed something, however. @simonmar, do you agree that this could be simplified?

bgamari requested changes to this revision.Feb 14 2017, 12:11 PM
This revision now requires changes to proceed.Feb 14 2017, 12:11 PM
idontgetoutmuch marked 10 inline comments as done.Mar 3 2017, 5:10 AM
idontgetoutmuch added inline comments.
compiler/cmm/PprC.hs
757

This is correct as the instruction on anything other than X86 or llvm should *not* be generated.

compiler/codeGen/StgCmmPrim.hs
1103

They are in https://phabricator.haskell.org/D3265 which supersedes this.

compiler/nativeGen/PPC/CodeGen.hs
1528

See my reply for MO_F64_Fabs.

compiler/nativeGen/SPARC/CodeGen.hs
613

See my reply for MO_F64_Fabs.

631

See my previous reply for MO_F64_Fabs.

compiler/nativeGen/X86/CodeGen.hs
2071

Fair point - I will amend the latest PR.

2075

Ignore this as it has changed significantly in https://phabricator.haskell.org/D3265.

2082

Ignore this as it has changed significantly in https://phabricator.haskell.org/D3265.

2602

See my comments above.

2619

See my comments above.

idontgetoutmuch abandoned this revision.Mar 3 2017, 5:10 AM
idontgetoutmuch marked 8 inline comments as done.
idontgetoutmuch added inline comments.Mar 3 2017, 5:28 AM
compiler/nativeGen/X86/CodeGen.hs
2075

Oops - this hasn't changed.

2082

Oops - this hasn't changed. I will replace the last two lines.