ConstantRange::sub

Project Infrastructure LLVM Dev List Archives
#1

Hi,

I have a question about ConstantRange::sub(const ConstantRange &Other) at lib/Support/ConstantRange.cpp:524. The code computes the new bounds as follows.

APInt NewLower = getLower() - Other.getLower();
APInt NewUpper = getUpper() - Other.getUpper() + 1;

Could someone explain this to me? I was expecting something like

APInt NewLower = getLower() - Other.getUpper() + 1;
APInt NewUpper = getUpper() - Other.getLower();

Thanks.

- xi

#2

Thanks, I think you’ve found a serious bug!

Would you be willing to fix it? Please add a test to unittests/Support/ConstantRangeTest.cpp and then mail llvm-commits with the patch to fix it and add the test.

#3

Sure. I will submit a patch.

BTW, what's the difference between the bounds I was expecting

APInt NewLower = getLower() - Other.getUpper() + 1;
APInt NewUpper = getUpper() - Other.getLower();

and the two you mentioned

NewLower = Lower - (Upper-1)
NewUpper = (Upper-1) - Lower + 1

They look equivalent to me. Did I miss anything? Thanks.

- xi

#4

Sure. I will submit a patch.

BTW, what’s the difference between the bounds I was expecting

APInt NewLower = getLower() - Other.getUpper() + 1;
APInt NewUpper = getUpper() - Other.getLower();

and the two you mentioned

NewLower = Lower - (Upper-1)
NewUpper = (Upper-1) - Lower + 1

They look equivalent to me. Did I miss anything? Thanks.

… they are. Sorry, I was just surprised to see the +1 on the NewLower (it always goes on the NewUpper instead, right), but I didn’t actually simplify the expressions.

It sounds to me like you’ve got this one handled. :slight_smile:

Nick

#5

Here goes the patch along with a test for ConstantRange::sub. Thanks.

crsub.patch (1.26 KB)