[PATCH] D100331: [Dependence Analysis] Fix ExactSIV producing wrong analysis

[Artem Radzikhovskyy via Phabricator via llvm-commits]

artemrad added a comment.

In D100331#2696167 <https://reviews.llvm.org/D100331#2696167>, @Meinersbur wrote:

> This change reverses almost all directions in the test cases. Was the implementation that terribly wrong?
>
> Not having studies the used algorithms myself, I assumed the DA result to be correct and when using it as analysis assumed that `flow [<]` and `anti [>]` are anti-dependences (WAR).

Hi Michael,

> Was the implementation that terribly wrong?

No, but there is a pretty bad bug.

As for the assumption it is incorrect. In literature, when talking about two statements `src` and  `dst`, [<] indicates `src` occurs before `dst`, and [>] indicates `dst` occurs before `src`. Which further implies the following statements:
	• `flow [<]` is RAW
	• `anti [>]` is RAW
	• `flow [>]` is WAR
	• `anti [<]` is WAR
This is corroborated by https://dl.acm.org/doi/pdf/10.1145/113446.113448 (see section 1.3)

Now this is also not just a case of LLVM differing from the academic community in its definitions.  Let me demonstrate:

1. In the first test of StrongSIV.ll (Note: I did not modify this test)

  	;;  for (int i = 0; i < n; i++) {
  	;;    A[i + 2] = i;
  	;;    *B++ = A[i];

We get the following result from DA:

  	Src:  store i32 %1, i32* %arrayidx, align 4 --> Dst:  %2 = load i32, i32* %arrayidx3, align 4
  	  da analyze - consistent flow [2]!

Which shows that (+) positive distance in LLVM means `src` happens before `dst`

2. From the following lines in DependenceAnalysis.cpp

  	    if (Distance.sgt(0))
  	      Result.DV[Level].Direction &= Dependence::DVEntry::LT;
  	    else if (Distance.slt(0))
  	      Result.DV[Level].Direction &= Dependence::DVEntry::GT;
  	    else
  	      Result.DV[Level].Direction &= Dependence::DVEntry::EQ;

We see that DA establishes the following relationship positive values -> [<], and negative values -> [>]. Combining this with (1), we see that LLVM's definition aligns with the literature's definition.

3. Now let's take a look at one of the lit tests that were modified by my change, say `exact3` from ExactSIV.ll

  	;;  for (long unsigned i = 0; i <= 10; i++) {
  	;;    A[6*i] = i;
  	;;    *B++ = A[i + 60];
  	
  	Src:  store i32 %conv, i32* %arrayidx, align 4 --> Dst:  %0 = load i32, i32* %arrayidx1, align 4
  	  da analyze - flow [>]!

Here we can clearly see that the load in `dst` (i=0) comes before the store in `src`(i=10), hence [>].

Hopefully this shows that there is a bug in Dependence Analysis.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100331/new/

https://reviews.llvm.org/D100331