Page MenuHomePhabricator

use an integer constant for FABS / FNEG (x86)
ClosedPublic

Authored by spatel on Aug 25 2014, 12:22 PM.

Details

Summary

This patch is just a bit of cleanup to ease refactoring LowerFABS() and LowerFNEG() since they have a lot of overlap.

I've removed the creation of a floating point constant from an integer because it's going to be used for a bitwise integer op anyway.

I don't intend for this to make any difference to codegen, and AFAICT there isn't. There's no difference in test-suite either.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel updated this revision to Diff 12907.Aug 25 2014, 12:22 PM
spatel retitled this revision from to use an integer constant for FABS / FNEG (x86).
spatel updated this object.
spatel edited the test plan for this revision. (Show Details)
spatel added a subscriber: Unknown Object (MLST).
spatel updated this revision to Diff 12916.Aug 25 2014, 1:20 PM
spatel added a reviewer: chandlerc.

Updated to use APInt::getSignBit() - no need to mess around with bit shifting here.

majnemer added inline comments.
lib/Target/X86/X86ISelLowering.cpp
12124 ↗(On Diff #12916)

I think you can replace ~APInt::getSignBit(EltBits) with APInt::getSignedMaxValue(EltBits)

spatel updated this revision to Diff 12918.Aug 25 2014, 1:46 PM

Updated to use APInt::getSignedMaxValue() - thanks, David!

Ok to commit?

Ping.

I think it would be ok to commit this small change, but the dev policy page (code review rule #5) says that I need an explicit LGTM because I posted this as a patch for review. Please let me know if I'm misunderstanding the rules.

spatel closed this revision.Sep 1 2014, 12:11 PM
spatel updated this revision to Diff 13143.

Closed by commit rL216889 (authored by @spatel).

Thanks, Andrea! Committed at r216889.