The combine added in commit 329525 overlooked the case where
one, but not all, of the divisor elements is -1.
-1 is the only power of two value for which the sdiv expansion recipe
breaks.
Fixes pr37119
Paths
| Differential D45806
DAGcombiner: Handle correctly non-splat power of 2 -1 divisor ClosedPublic Authored by RKSimon on Apr 19 2018, 1:26 AM.
Details Summary The combine added in commit 329525 overlooked the case where Fixes pr37119
Diff Detail
Event Timeline
Comment Actions
@zvi I'm going to commandeer this and get it finished if you don't mind. Comment Actions Refactored the patch to blend the special case values in at the end of the pow2 calculations. I'll add the SignedMin case in a followup but I wanted to keep this patch focussed on the -1 divisor case. Comment Actions Sorry for not being responsive. Won't have time to work on this, so thanks @RKSimon for taking charge.
RKSimon mentioned this in rL335817: [DAGCombiner] Remove unused variable. NFCI..Jun 28 2018, 2:33 AM RKSimon added inline comments. Comment Actions I think you can save a blend by emitting one select for both "1" and "-1"? (SDValue IsOneOrAllOnes = DAG.getNode(ISD::OR, VT, IsAllOnes, IsOne); etc.) This revision is now accepted and ready to land.Jun 29 2018, 1:47 PM Closed by commit rL336048: [DAGCombiner] Handle correctly non-splat power of 2 -1 divisor (PR37119) (authored by RKSimon). · Explain WhyJun 30 2018, 5:27 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 143056 lib/CodeGen/SelectionDAG/DAGCombiner.cpp
test/CodeGen/X86/combine-sdiv.ll
|
Not really relevant to this review, but I think KnownNegatives is dead.