This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] don't try to partially reduce add-with-overflow ops
ClosedPublic

Authored by spatel on Jul 28 2021, 12:17 PM.

Details

Summary

This transform was added with D58874, but there were no tests for overflow ops.
We need to change this one way or another because it can crash as shown in:
https://llvm.org/PR51238

Note that if there are no uses of an overflow op's bool overflow result, we reduce it to a regular math op, so we continue to fold that case either way.
If we have uses of both the math and the overflow bool, then we are likely not saving anything by creating an independent sub instruction as seen in the test diffs here.

This patch would make the behavior in SDAG consistent with what we do in instcombine AFAICT.

Diff Detail

Event Timeline

spatel created this revision.Jul 28 2021, 12:17 PM
spatel requested review of this revision.Jul 28 2021, 12:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 28 2021, 12:17 PM
lebedev.ri accepted this revision.Jul 29 2021, 5:02 AM

I think this is the right direction, at least in the meanwhile.
Agreed we could restrict the transform to avoid the assertion,
but indeed, clearly it's not really a win, at least not an obvious one.

This revision is now accepted and ready to land.Jul 29 2021, 5:02 AM
This revision was landed with ongoing or failed builds.Jul 29 2021, 5:53 AM
This revision was automatically updated to reflect the committed changes.