This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] (add X, (adde Y, 0, Carry)) -> (adde X, Y, Carry)
ClosedPublic

Authored by deadalnix on Feb 5 2017, 8:37 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

deadalnix created this revision.Feb 5 2017, 8:37 AM
RKSimon accepted this revision.Feb 5 2017, 3:41 PM

LGTM with one style minor

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
1840 ↗(On Diff #87149)

clang-format this

This revision is now accepted and ready to land.Feb 5 2017, 3:41 PM
zvi added inline comments.Feb 5 2017, 9:13 PM
test/CodeGen/X86/adde-carry.ll
89 ↗(On Diff #87149)

If you intend to improve this test case in future patches, i think you should add a dedicated minimal test to ensure the combine introduced this patch will be covered. Other than that LGTM

deadalnix added inline comments.Feb 6 2017, 6:18 AM
test/CodeGen/X86/adde-carry.ll
89 ↗(On Diff #87149)

I don't think that's a problem. The add case is always the bottom turtle.

deadalnix updated this revision to Diff 87219.Feb 6 2017, 6:39 AM

clang-format

This revision was automatically updated to reflect the committed changes.