[LLVMdev] review request for patch
gohman at apple.com
Fri Jul 10 17:35:42 PDT 2009
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
> 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.
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
More information about the llvm-dev