[all-commits] [llvm/llvm-project] 149079: [Support] Fix what I think is an off by 1 bug in U...

Craig Topper via All-commits all-commits at lists.llvm.org
Thu Dec 29 09:43:07 PST 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 1490796dd28c80e987a0a9306104141c52022982
      https://github.com/llvm/llvm-project/commit/1490796dd28c80e987a0a9306104141c52022982
  Author: Craig Topper <craig.topper at sifive.com>
  Date:   2022-12-29 (Thu, 29 Dec 2022)

  Changed paths:
    M llvm/lib/Support/DivisionByConstantInfo.cpp

  Log Message:
  -----------
  [Support] Fix what I think is an off by 1 bug in UnsignedDivisionByConstantInfo.

The code in Hacker's Delight says
`nc = -1 - (-d)%d;`

But we have
`NC = AllOnes - (AllOnes-D)%D`

The Hacker's Delight code is written for the LeadingZeros==0 case.
`AllOnes - D` is not the same as `-d` from Hacker's Delight.

This patch changes the code to
`NC = AllOnes - (AllOnes+1-D)%D`

This will increment AllOnes to 0 in the LeadingZeros==0 case. This
will make it equivalent to -D. I believe this is also correct for
LeadingZeros>0.

At least for i8, i16, and i32 the only divisor that changes is
((1 << (BitWidth-1)) | 1). Or 127 for i8, 32769 for i16, and 2147483649
for i32. These are all large enough that the quotient is 0 or 1 so
InstCombine replaces them with an icmp and zext before SelectionDAG.

Reviewed By: lebedev.ri

Differential Revision: https://reviews.llvm.org/D140636




More information about the All-commits mailing list