This is an archive of the discontinued LLVM Phabricator instance.

[NewGVN] Add unary FNeg support to NewGVN pass
ClosedPublic

Authored by cameron.mcinally on Jun 28 2019, 7:48 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2019, 7:48 AM
fhahn accepted this revision.Jun 28 2019, 8:17 AM
fhahn added a subscriber: fhahn.

LGTM, thanks.

llvm/test/Transforms/NewGVN/fpmath.ll
46 ↗(On Diff #207065)

Can you update the test to check !1 points to the expected metadata?

This revision is now accepted and ready to land.Jun 28 2019, 8:17 AM
This revision was automatically updated to reflect the committed changes.
cameron.mcinally marked an inline comment as done.Jun 28 2019, 1:14 PM
cameron.mcinally added inline comments.
llvm/test/Transforms/NewGVN/fpmath.ll
46 ↗(On Diff #207065)

Sorry, missed this comment. Will submit a follow-up patch ASAP.

cameron.mcinally marked an inline comment as done.Jun 28 2019, 1:45 PM
cameron.mcinally added inline comments.
llvm/test/Transforms/NewGVN/fpmath.ll
46 ↗(On Diff #207065)

Actually, I'm not sure how to do that well. Would you point me to an example, please?

The metadata string is at the end of the file. I could check for it at the very end of all the tests. Is that what you're suggesting?

If I check the metadata in this particular test, that would prevent new tests from being added after this test, since it would throw off the CHECK line order.

fhahn added inline comments.Jun 28 2019, 2:23 PM
llvm/test/Transforms/NewGVN/fpmath.ll
46 ↗(On Diff #207065)

I think the most robust thing is to check it at the end of the file.

Ideally you would use a regex to match the ID in the CHECK lines for the operations and then check the value of the metadata at the bottom.

cameron.mcinally marked 3 inline comments as done.Jun 28 2019, 2:40 PM
cameron.mcinally added inline comments.
llvm/test/Transforms/NewGVN/fpmath.ll
46 ↗(On Diff #207065)

Submitted:

Author: mcinally
Date: Fri Jun 28 14:39:08 2019
New Revision: 364685

Just give me a heads up if you had something different in mind...