This is an archive of the discontinued LLVM Phabricator instance.

Add FNeg support to InstructionSimplify
ClosedPublic

Authored by cameron.mcinally on May 5 2019, 12:00 PM.

Details

Summary

This is the last chunk separated from the larger D61419...

Add FNeg support to InstructionSimplify. There is only one xform included in this patch:

fneg (fneg X) -> X

I took a liberty with the added test and will comment inline so that it's clear for review purposes.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2019, 12:00 PM
cameron.mcinally marked an inline comment as done.May 5 2019, 12:03 PM
cameron.mcinally added inline comments.
llvm/test/Analysis/ConstantFolding/fneg.ll
2 ↗(On Diff #198186)

I'm thinking that changing the pass to -instsimplify will save creating another file for the new xform. InstSimplify will exercise the ConstantExpr folding as well, so coverage will remain the same.

Does anyone feel strongly against this?

Fix a typo in a comment.

Fix bad copy-and-paste. Misunderstood the purpose of SimplifyUnOp(...).

cameron.mcinally edited the summary of this revision. (Show Details)May 5 2019, 12:25 PM
spatel added inline comments.May 6 2019, 6:44 AM
llvm/test/Analysis/ConstantFolding/fneg.ll
2 ↗(On Diff #198186)

I don't like having a test for a pass out of its named directory. That makes it hard to find.

This test can be added into:
test/Transforms/InstSimplify/floating-point-arithmetic.ll

cameron.mcinally marked an inline comment as done.May 6 2019, 8:09 AM
cameron.mcinally added inline comments.
llvm/test/Analysis/ConstantFolding/fneg.ll
2 ↗(On Diff #198186)

Good idea. Thanks.

I originally was looking at cast-vector.ll in test/Analysis/ConstantFolding, which is an -instsimplify test, so that's probably misplaced too. Just a heads up...

Move tests as @spatel suggested.

spatel accepted this revision.May 6 2019, 8:29 AM

LGTM

llvm/lib/Analysis/InstructionSimplify.cpp
54–56 ↗(On Diff #198287)

Nit: we will never converge if we favor old code formatting over the current formatting spec ('camelCase'), so I'd rather have the right formatting in this patch and then update everything that's wrong with an NFC change or vice-versa. At least for the static functions, that shouldn't be too disruptive?

This revision is now accepted and ready to land.May 6 2019, 8:29 AM

Use camelCase for static functions, as @spatel suggested.

This revision was automatically updated to reflect the committed changes.