llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.08k stars 11.99k forks source link

[SelectionDAG] Generalise SelectionDAG::computeOverflowKind to support other opcodes #37109

Closed RKSimon closed 8 months ago

RKSimon commented 6 years ago
Bugzilla Link 37761
Version trunk
OS Windows NT
CC @deadalnix,@LebedevRI,@rotateright

Extended Description

SelectionDAG::computeOverflowKind is currently only to be used for additions; it'd be useful for it to also handle signed/unsigned ISD::SUB and ISD::MUL (to cover all the SADDO/UADD, SSUBO/USUBO and SMULO/UMULO nodes).

RKSimon commented 1 year ago

This has now been split into:

computeOverflowForSignedAdd
computeOverflowForUnsignedAdd
computeOverflowForSignedSub
computeOverflowForUnsignedSub

These more closely match the InstCombine methods, so we're just missing:

computeOverflowForSignedMul
computeOverflowForUnsignedMul

It might be useful to add similar helpers to detect exact shifts.

llvmbot commented 1 year ago

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1) Assign the issue to you. 2) Fix the issue locally. 3) Run the test suite locally. 3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests. 4) Create a git commit 5) Run git clang-format HEAD~1 to format your changes. 6) Submit the patch to Phabricator. 6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue

elhewaty commented 1 year ago

I will work on this

elhewaty commented 1 year ago

@RKSimon Candidate Patch: https://reviews.llvm.org/D159406

RKSimon commented 1 year ago

We also need to extend the computeOverflowForSignedSub/computeOverflowForUnsignedSub implementations with ConstantRange - I think this will mainly involve adding test coverage.

elhewaty commented 1 year ago

OK, I am working on it now

RKSimon commented 1 year ago

computeOverflowForSignedMul computeOverflowForUnsignedMul Additional Tests: e086e0aeef6be3b1b3b403e54fbe2669c649973d Fixed: 741c1278175b9354442cd2143e1452714dc020a2

elhewaty commented 1 year ago

@RKSimon Here is the new pull : https://github.com/llvm/llvm-project/pull/67890

RKSimon commented 8 months ago

Closing this - the computeOverflow implementations are in place, we can create smaller issues for any specifics we're still missing.