This is an archive of the discontinued LLVM Phabricator instance.

Refactor LowerFABS and LowerFNEG into one function (x86) (NFC)
ClosedPublic

Authored by spatel on Aug 26 2014, 7:55 AM.

Details

Summary

We duplicate ~30 lines of code to lower FABS and FNEG for x86, so I've combined them into one function. No functional change intended, so no additional test cases. Test-suite behavior is unchanged.

This is a follow-on patch to http://reviews.llvm.org/D5052 (use an integer constant for FABS / FNEG (x86)).

If this patch is OK, we can abandon the other patch because that change is included here.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel updated this revision to Diff 12947.Aug 26 2014, 7:55 AM
spatel retitled this revision from to Refactor LowerFABS and LowerFNEG into one function (x86).
spatel updated this object.
spatel edited the test plan for this revision. (Show Details)
spatel added reviewers: majnemer, chandlerc, nadav.
spatel added a subscriber: Unknown Object (MLST).
spatel updated this revision to Diff 13144.Sep 1 2014, 12:59 PM
spatel retitled this revision from Refactor LowerFABS and LowerFNEG into one function (x86) to Refactor LowerFABS and LowerFNEG into one function (x86) (NFC).
spatel added a reviewer: andreadb.

Updated patch because http://reviews.llvm.org/D5052 was checked in at r216889. Also updated some comments.

andreadb accepted this revision.Sep 2 2014, 3:13 AM
andreadb edited edge metadata.

Hi Sanjay,

patch LGTM, thanks!

lib/Target/X86/X86ISelLowering.cpp
16900–16901 ↗(On Diff #13144)

Nit: I would just fall-through from case ISD::FABS to the next case (i.e. ISD::FNEG).

This revision is now accepted and ready to land.Sep 2 2014, 3:13 AM
spatel closed this revision.Sep 2 2014, 1:34 PM
spatel updated this revision to Diff 13180.

Closed by commit rL216942 (authored by @spatel).

spatel added a comment.Sep 2 2014, 1:35 PM

Thanks, Andrea! Checked in at r216942.