This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Push truncate through adde when the carry isn't used.
ClosedPublic

Authored by deadalnix on Feb 3 2017, 6:26 PM.

Event Timeline

deadalnix created this revision.Feb 3 2017, 6:26 PM
arsenm added a subscriber: arsenm.Feb 3 2017, 6:32 PM
arsenm added inline comments.
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
7784

Extra whitespace change

7874–7876

Needs a legality check for the resulting adde?

deadalnix updated this revision to Diff 87092.Feb 4 2017, 5:31 AM

Check for adde's validity before combining.

arsenm added inline comments.Feb 4 2017, 10:21 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
7877

Usually this is done as !LegalizeDAG || TLI.isOperationLegal..

7878

SDLoc SL(N)

test/CodeGen/X86/adde-carry.ll
165–166

Can you add tests with more types?

deadalnix marked an inline comment as done.Feb 4 2017, 1:24 PM
deadalnix added inline comments.
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
7877

There is no wrapper for this ?

deadalnix updated this revision to Diff 87119.Feb 4 2017, 3:22 PM

Add test cases for various sizes.
Use SDLoc SL(N) .

RKSimon added inline comments.Feb 6 2017, 6:10 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
7875

clang-format

deadalnix updated this revision to Diff 87232.Feb 6 2017, 7:53 AM

Rebase, clang-format

RKSimon added inline comments.Feb 6 2017, 10:55 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
7876

This should probably be (!LegalOperations || TLI.isOperationLegal(ISD::ADDE, VT)) - otherwise you might get into trouble if custom lowering results in the truncate being regenerated.

7879

Typically we don't use auto for these cases - just use SDvalue

deadalnix updated this revision to Diff 87289.Feb 6 2017, 1:11 PM

Use (!LegalOperations || TLI.isOperationLegal(ISD::ADDE, VT))

arsenm accepted this revision.Feb 7 2017, 3:18 PM

LGTM

This revision is now accepted and ready to land.Feb 7 2017, 3:18 PM
This revision was automatically updated to reflect the committed changes.