# Changeset View

# Standalone View

# lib/Transforms/InstCombine/InstCombineAddSub.cpp

Context not available. | |||||

1008 | return false; | 1008 | return false; | ||
---|---|---|---|---|---|

1009 | } | 1009 | } | ||

1010 | 1010 | | |||

1011 | /// \brief Return true if we can prove that: | ||||

1012 | /// (mul LHS, RHS) === (mul nsw LHS, RHS) | ||||

1013 | bool InstCombiner::WillNotOverflowSignedMul(Value *LHS, Value *RHS, | ||||

1014 | Instruction *CxtI) { | ||||

1015 | if (IntegerType *IT = dyn_cast<IntegerType>(LHS->getType())) { | ||||

1016 | unsigned BitWidth = IT->getBitWidth(); | ||||

gottesmm: I would put the comment below about Hacker's delight up here and add a quick high level… | |||||

1017 | unsigned SignBits = ComputeNumSignBits(LHS, 0, CxtI) + | ||||

reamesUnsubmitted Not Done ReplyBrief comment here would be good. You appear to be using the fact that a N bit number multiplied by a M bit number can yield a maximum of an N+M+1 bit number right? reames: Brief comment here would be good. You appear to be using the fact that a N bit number… | |||||

Not Done ReplyMake sure to mention that underestimating the number of sign bits gives you a more conservative answer so the fact that we are underestimating can not cause us to get the wrong answer. gottesmm: Make sure to mention that underestimating the number of sign bits gives you a more conservative… | |||||

1018 | ComputeNumSignBits(RHS, 0, CxtI); | ||||

1019 | if (SignBits > BitWidth + 1) | ||||

1020 | return true; | ||||

1021 | | ||||

Not Done ReplyYou should add why it is interesting that the result is n+m significant bits. Otherwise the line does not add anything. gottesmm: You should add why it is interesting that the result is n+m significant bits. Otherwise the… | |||||

1022 | if (SignBits == BitWidth + 1) { | ||||

1023 | // The smallest negative numbers in this case still overflow. | ||||

1024 | // E.g. mul i16 with 17 sign bits: 0xff00 * 0xff80 = 0x8000 | ||||

1025 | bool LHSNonNegative, LHSNegative; | ||||

1026 | bool RHSNonNegative, RHSNegative; | ||||

1027 | ComputeSignBit(LHS, LHSNonNegative, LHSNegative, DL, 0, AT, CxtI, DT); | ||||

1028 | ComputeSignBit(RHS, RHSNonNegative, RHSNegative, DL, 0, AT, CxtI, DT); | ||||

1029 | if (LHSNonNegative || RHSNonNegative) | ||||

Not Done ReplyThere are 2x ambiguous cases. I would say what they actually are. Also you should mention that we are not handling the second case. I would also specify that you are only handling a part of the first ambiguous case that can be known easily via the information at hand pi.e. that there is a low likelihood that we will have enough information to check the product, but we *can* check the sign = )]. gottesmm: There are 2x ambiguous cases. I would say what they actually are. Also you should mention that… | |||||

1030 | // No overflow if at least one side is not negative. | ||||

1031 | return true; | ||||

1032 | } | ||||

1033 | } | ||||

1034 | return false; | ||||

1035 | } | ||||

1036 | | ||||

1011 | // Checks if any operand is negative and we can convert add to sub. | 1037 | // Checks if any operand is negative and we can convert add to sub. | ||

1012 | // This function checks for following negative patterns | 1038 | // This function checks for following negative patterns | ||

Not Done ReplyI would remove this if you do what I suggested with the comment above. gottesmm: I would remove this if you do what I suggested with the comment above. | |||||

1013 | // ADD(XOR(OR(Z, NOT(C)), C)), 1) == NEG(AND(Z, C)) | 1039 | // ADD(XOR(OR(Z, NOT(C)), C)), 1) == NEG(AND(Z, C)) | ||

Context not available. |

I would put the comment below about Hacker's delight up here and add a quick high level explanation of what you are doing.