[patch] fix comment on findGCD

[Preston Briggs]

The patch looks ok to me,
but it also looks more complex than simply fixing the comment.
Rather than negating the conditional each time we call findGCD,
let's leave the code alone and make the comment say something like:

*findGCD returns true if dependence has been disproven;*
*that is, if gcd does not divide Delta.*


Preston



On Thu, Jan 2, 2014 at 10:31 PM, Mingjie Xing <mingjie.xing at gmail.com>wrote:

> Hi,
>
> While reading source code again, I think it is not a simple comment
> typo. The function
> findGCD always finds the GCD. So the boolean return value depends on the
> meaning
> what we want it to be. We say the Diophantine equation has a solution
> iff GCD divides
> Delta. And the dependence is disproved if there is no solution.
>
> So if we want findGCD return true when GCD divides Delta, souce code
> should be fixed
> as the following patch,
>
> Index: lib/Analysis/DependenceAnalysis.cpp
> ===================================================================
> --- lib/Analysis/DependenceAnalysis.cpp (revision 198188)
> +++ lib/Analysis/DependenceAnalysis.cpp (working copy)
> @@ -1275,7 +1275,7 @@
>  //
>  // Program 2.1, page 29.
>  // Computes the GCD of AM and BM.
> -// Also finds a solution to the equation ax - by = gdc(a, b).
> +// Also finds a solution to the equation ax - by = gcd(a, b).
>  // Returns true iff the gcd divides Delta.
>  static
>  bool findGCD(unsigned Bits, APInt AM, APInt BM, APInt Delta,
> @@ -1301,11 +1301,11 @@
>    // make sure gcd divides Delta
>    R = Delta.srem(G);
>    if (R != 0)
> -    return true; // gcd doesn't divide Delta, no dependence
> +    return false; // gcd doesn't divide Delta, no dependence
>    Q = Delta.sdiv(G);
>    X *= Q;
>    Y *= Q;
> -  return false;
> +  return true;
>  }
>
>
> @@ -1398,7 +1398,7 @@
>    APInt AM = ConstSrcCoeff->getValue()->getValue();
>    APInt BM = ConstDstCoeff->getValue()->getValue();
>    unsigned Bits = AM.getBitWidth();
> -  if (findGCD(Bits, AM, BM, ConstDelta->getValue()->getValue(), G, X, Y))
> {
> +  if (!findGCD(Bits, AM, BM, ConstDelta->getValue()->getValue(), G, X,
> Y)) {
>      // gcd doesn't divide Delta, no dependence
>      ++ExactSIVindependence;
>      ++ExactSIVsuccesses;
> @@ -1802,7 +1802,7 @@
>    APInt AM = ConstSrcCoeff->getValue()->getValue();
>    APInt BM = ConstDstCoeff->getValue()->getValue();
>    unsigned Bits = AM.getBitWidth();
> -  if (findGCD(Bits, AM, BM, ConstDelta->getValue()->getValue(), G, X, Y))
> {
> +  if (!findGCD(Bits, AM, BM, ConstDelta->getValue()->getValue(), G, X,
> Y)) {
>      // gcd doesn't divide Delta, no dependence
>      ++ExactRDIVindependence;
>      return true;
>
> Best Regards,
> Mingjie
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140103/554b629d/attachment.html>