This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Do various combine on usubo.
ClosedPublic

Authored by deadalnix on Feb 27 2017, 6:06 PM.

Event Timeline

deadalnix created this revision.Feb 27 2017, 6:06 PM
RKSimon added inline comments.Mar 5 2017, 8:15 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2115

Can vectors ever use this? In which case we need to be careful of generating constants after legalization (see tryFoldToZero).

2119

Is it possible to add an explicit test for this?

zvi added inline comments.Mar 5 2017, 10:52 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2130

If i am not mistaken, we don't have a similar LLVM IR transform in InstCombine or InstSimplify.

deadalnix added inline comments.Mar 5 2017, 3:12 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2115

I don't think so.

2119

Both codegen result in a sub, the overflow flag goes into the flag register no matter what. I'm not sure what can be done. Add a usubo with a transform that work on a sub and nothing that use the flag and see the transform kicks in ?

deadalnix updated this revision to Diff 91176.Mar 9 2017, 8:34 AM

Rebase and fix merge conflict. Also, ping ?

RKSimon accepted this revision.Mar 9 2017, 9:43 AM

LGTM with one minor to check for vector types.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2115

Please can you add an early out for vectors then, just in case.

This revision is now accepted and ready to land.Mar 9 2017, 9:43 AM
deadalnix updated this revision to Diff 91196.Mar 9 2017, 11:24 AM

Bail in case of vector type.

LGTM - thanks.

This revision was automatically updated to reflect the committed changes.