While this makes some case better and some case worse - so it's unclear if it is a worthy combine just by itself - this is a useful canonicalisation.
As per discussion in D32756 .
Paths
| Differential D32916
[DAGCombine] (addcarry 0, 0, X) -> (ext/trunc X) ClosedPublic Authored by deadalnix on May 5 2017, 10:15 AM.
Details Summary While this makes some case better and some case worse - so it's unclear if it is a worthy combine just by itself - this is a useful canonicalisation. As per discussion in D32756 .
Diff Detail
Event Timeline
deadalnix added a child revision: D32925: [DAGCombine] (add/uaddo X, Carry) -> (addcarry X, 0, Carry).May 5 2017, 3:18 PM
Comment Actions See inline for a few nits, but I think this makes sense now. If I'm seeing the diffs correctly, there was no codegen difference for x86 after adding the 'and' mask, so we must be recognizing and optimizing that pattern. @RKSimon - do you see any other problems?
Comment Actions
This looks right to me now - thanks for reviewing the getBoolExtOrTrunc usage. Other than what's to be done with the mul-i1024 this looks good to go.
Comment Actions I suggest that we just delete mul-i1024.ll before accepting this patch - any objections? Comment Actions The test file(s) were added with D24478 / rL281403. cc'ing @chfast to see if it's necessary. PR30354 ( https://bugs.llvm.org//show_bug.cgi?id=30354 ) specified i1024, but do we actually need such a large value to trigger the bug? Comment Actions I agree that we shouldn't gate this patch waiting for a reply to the i1024 question. If you're confident that the code in those super-wide cases is still correct, then LGTM. This revision is now accepted and ready to land.May 19 2017, 10:41 AM Comment Actions I looked at the codegen for the mul and it looks correct. There are very little actual changes, but, they do cause registers to be allocated differently, so the diff ends up being huge. Closed by commit rL303441: [DAGCombine] (addcarry 0, 0, X) -> (ext/trunc X) (authored by deadalnix). · Explain WhyMay 19 2017, 11:34 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 98230 lib/CodeGen/SelectionDAG/DAGCombiner.cpp
test/CodeGen/X86/addcarry.ll
test/CodeGen/X86/mul-i1024.ll
test/CodeGen/X86/mul-i256.ll
test/CodeGen/X86/mul-i512.ll
test/CodeGen/X86/overflow.ll
|
Please split these changes before committing the patch.