[LLVMdev] review request for patch
parseerror at gmail.com
Fri Jul 10 17:44:19 PDT 2009
On Fri, Jul 10, 2009 at 8:35 PM, Dan Gohman<gohman at apple.com> wrote:
> On Jul 10, 2009, at 2:03 PM, Ryan Flynn <parseerror at gmail.com> wrote:
>> I've addressed a "TODO" in ConstantRange and several in its unit test
>> by implementing a stricter "multiply" method (it had been returning a
>> "full" set for anything that wasn't "empty", which broader than
>> and updated the unit test to match, but I'm not completely confident
>> that I understand ConstantRange and APInt and was hoping someone more
>> familiar might take a quick look and give me some feedback.
>> Thanks in advance.
> Thanks for working on this! Here are some comments:
> The overflow check isn't sufficient. Unlike add, multiply can wrap
> around multiple times, so it needs a more involved check. One way to
> do this would be to extend the operands out to twice their original
> width, do the multiply, and then check for overflow.
> For the unit tests, please check for specific values instead of
> checking that One.multiply(Wrap) is equal to Wrap.multply(One) for
> example, so that it won't report a pass if it both sides have the same
> wrong value.
>> P.S. - Also, I'm not totally sure if it's appropriate to ask for this
>> here, but I thought it was more so than to llvm-commit, please let me
>> know otherwise as I am new around here.
> llvm-commits is the usual place for patches, but this works too.
Thanks for the feedback, I will work on it.
More information about the llvm-dev