[Idea] Introduce PrecomputeLoopExpressionsPass to replace expensive array index computation with precomputed array lookup

Hi,

In downstream LLVM compiler, we have this optimization (developed back in 2012) that if we have expensive array index computation that only depends on the loop induction variables. At the same time the ranges and increments of these induction variables are known at compile time. Then we can precompute the array indexes at compile time and place them in a static array, and then look up at run time.

I have posted this optimization in PR 90263. Introducing a PrecomputeLoopExpressionsPass in IR optimization pipeline.

To demonstrate this idea, use test.c below. This is a reduced test from customer mp3decoder codebase.

extern int      cos_table[4 * 36];

void foo(int out[36]) {
  int p, m, N, sum;
  N = 36;
  for (p=0; p<N; p++){
    sum = 0;
    for (m=0; m < N/2; m++)
      sum += cos_table[((2*p+1+N/2)*(2*m+1))%(4*36)];
    out[p] = sum;
  }
}

Sub-expression “…(2m+1))%144” is expensive and cannot be hoisted outside.
PrecomputeLoopExpressionsPass will create a static array Temp[36][18], use expression “((2p+1+N/2)(2m+1))%144" to initialize Temp. Then transform original array index computation "cos_table[((2p+1+N/2)(2m+1))%144]” into a lookup from Temp “cos_table[[Temp[p][m]]”.

Apply PR90263, run below command to see the IR and assembly difference.
clang --target=aarch64 -mcpu=cortex-x4 -march=armv8.7-a -S -O3 test.c -fno-unroll-loops -o pcle.ll -emit-llvm
append “-mllvm -disable-pcle” to disable PrecomputeLoopExpresions to get baseline output.

Then you will see PCLE cuts 3 instructions from innermost loop in the generated assembly.

.LBB0_2:                                // baseline
                                        //   Parent Loop BB0_1 Depth=1
                                        // =>  This Inner Loop Header: Depth=2
        umulh   x1, x17, x12
        subs    w16, w16, #1
        add     x17, x17, x11
        lsr     x1, x1, #7
        mul     x1, x1, x13
        ldr     w1, [x18, x1]
        add     x18, x18, x9
        add     w15, w1, w15
        b.ne    .LBB0_2
.LBB0_2:                                // with PCLE
                                        //   Parent Loop BB0_1 Depth=1
                                        // =>  This Inner Loop Header: Depth=2
        ldr     w13, [x9, x12]
        add     x12, x12, #4
        cmp     x12, #72
        ldr     w13, [x10, x13, lsl #2]
        add     w11, w13, w11
        b.ne    .LBB0_2

In IR generated, you will see that
sequence

  %mul6 = shl nuw nsw i32 %m.023, 1
  %add7 = or disjoint i32 %mul6, 1
  %mul8 = mul nuw nsw i32 %add7, %1
  %rem = urem i32 %mul8, 144

is replaced with

  %txgep = getelementptr [36 x [18 x i32]], ptr @tx0, i64 0, i64 %indvars.iv27, i64 %indvars.iv
  %txld = load i32, ptr %txgep, align 4

Although PR90263 optimization is able to speedup that customer mp3decoder benchmark by 50%. I don’t see other internal benchmarks benefit from such optimization.
There are two major limitations:

  1. The ranges and increments of loop induction variables need to be compile-time known. And array index computation depends only on these induction variables, with no other runtime variables.
  2. The current implementation use type extension from i32 (computation based on induction variable) to i64 (GEP offset) to detect candidate expressions. But not general enough to apply to cases like function calls.
    There are similar patterns in ffmpeg https://ffmpeg.org/
    tests/checkasm/vp9dsp.c
static void fdct_1d(double *out, const double *in, int sz)
{
    int k, n;
    for (k = 0; k < sz; k++) {
        out[k] = 0.0;
        for (n = 0; n < sz; n++)
            out[k] += in[n] * cos(M_PI * (2 * n + 1) * k / (sz * 2.0));
    }
    out[0] *= M_SQRT1_2;
}

If ‘sz’ is compile-time known, “cos(M_PI * (2 * n + 1) * k / (sz * 2.0))” should be optimized in a similar way. But we need to figure out better ways to detect candidate expressions.

I want to share this optimization to get more feedbacks.
And see if there are other benchmarks that can benefit from such optimization.
Also want to see if there are enough interest to bring such optimization into main.

Thanks, -Huihui

1 Like

A new pass would need to show its usefulness for more than just one anecdotal function for an undisclosed software. If it is just this one it would be simpler to just apply that optimization in that source. Speedups on llvm-test-suite can be used to show general usefulness. Also, the compile-time increase needs to be considered.

This optimization specifically trades memory for execution time. It will be more beneficial on targets with fast memory and slow compute but the memory wall would suggest it usually is the other way around. PR90263 has a limit of up to 2 Megabytes which is far too much for embedded systems. PrecomputeLoopExpressions::computeExpressionCost uses hardcoded instruction cost, but should rather use TargetTransformInfo and be in relation to a LoadInst and additional memory pressure/caches. Probably also for MaxSizeThreshold/MaxTotalSizeThreshold.

PR90263 is an independent pass, applied on top of upstream main sha id 300340f6. There shouldn’t be an issue to apply PR90263 to latest git repo.

I have run on llvm-test-suite, PR90263 only made 4 successful transformation. But none of them are truly desirable.
MicroBenchmarks/ImageProcessing/Blur/gaussianBlurKernel.c gaussianBlurKernel
MultiSource/Applications/oggenc/oggenc.c mapping0_forward
MultiSource/Benchmarks/DOE-ProxyApps-C/miniGMG/mg.c create_domain
MultiSource/Benchmarks/MiBench/consumer-lame/layer3.c init_layer3

The transformed functions follow a pattern below (reduced test from layer3.c init_layer3)

unsigned int i_slen2[256]; /* MPEG 2.0 slen for intensity stereo */
void foo() {
  for(int i=0;i<5;i++) {
    for(int j=0;j<6;j++) {
      for(int k=0;k<6;k++) {
        int n = k + j * 6 + i * 36;
        i_slen2[n] = i|(j<<3)|(k<<6)|(3<<12);
      }}}}

Array index expression “n = k + j * 6 + i * 36” is detected as candidate. But only sub-expression “k+…” needs to be done in the innermost loop. We want to detect and replace expensive expression that cannot be hoisted outside.
The current implementation of PR90263 doesn’t exclude cases where most part of the expensive expression can be hoisted outside. (refer to computeExpressionCost() and collectCandidateExpressions(), the implementation will need some improvements.)

Regarding

We might want to think differently. For applications that repeatedly calling over small trip count loops, that have considerable amounts of computation relying on innermost induction variable. (refer to the reduced test.c from mp3decoder routine).
The additional memory pressure can be negligible, 2.5KB for that mp3decoder benchmark. Then we are really looking at the throughput of a single load vs four arithmetic operations (including mul/div).

Another open question: For benchmarks with patterns like below

 for (p=0; p<SmallConstantTC1; p++){
    sum = 0;
    for (m=0; m<SmallConstantTC2; m++)
      sum += cos_table[...*(2*m+1))%(144)];
    out[p] = sum;
  }

PR90263 is one way to speedup. What other optimizations could we do to speedup such case?

Seems to me like there are induction sub-expressions that can be hoisted, and strength reduction applied.

unsigned int i_slen2[256];
void foo() {
  for (int i=0; i< 5; i++) {
    int i0 = i * 36; // i0 = (i<<5) + (i<<2);
    for (int j=0; j<6, j++) {
      int j0 = j * 6; // j0 = (j<<2) + (j<<1);
      int j1 = j<<3;
      int n0 = j0 + i0;
      for (int k=0; k<6; k++) {
        i_slen2[n0+k] = i | j1 | (k<<6) | (3<<12);
      }}}}

I’m sure there are other opportunities as well. Are there reasons why these invariant sub-expressions aren’t hoisted?

Hey Paul,

Thanks for looking into this! I think I may have misled you a little bit in my previous description.

Let’s look at t.c

unsigned int i_slen2[256];
void foo() {
  for(int i=0;i<5;i++) {
    for(int j=0;j<6;j++) {
      for(int k=0;k<6;k++) {
        int n = k + j * 6 + i * 36;
        i_slen2[n] = i|(j<<3)|(k<<6)|(3<<12);
      }}}}

Array index expression “n = k + j * 6 + i * 36” is detected as candidate. Sub-expression “j * 6” and “i * 36” are both hoisted outside by LICM before PrecomputeLoopExpressionsPass.
When we compute the expression cost, we should only consider operations that are done at the same loop level. That said, the cost for array index expression “n” at loop depth level 3 should be a single add “k+ValueComputedOutside”.
PR90263 commit 62037c7 was not getting the expression cost right, it was reporting a cost of two adds and two multiplies.
I just pushed commit 3c71e91 to PR90263 to fix the expression cost computation. Refer to Exclude sub-expressions that are already hoisted outside from expression cost computation

The updated PR90263 now only made one transformation when compiling the entire llvm test-suite.
File:MultiSource/Applications/oggenc/oggenc.c
Function:mapping0_forward

Look for loop and function call below (test case is a bit hard to reduce)

mapping0_forward(...) {
...
        for(k=PACKETBLOBS/2+1;k<PACKETBLOBS-1;k++)
          floor_posts[i][k]=
            floor1_interpolate_fit(vb,b->flr[info->floorsubmap[submap]],
                                   floor_posts[i][PACKETBLOBS/2],
                                   floor_posts[i][PACKETBLOBS-1],
                                   (k-PACKETBLOBS/2)*65536/(PACKETBLOBS/2));
...
}
int *floor1_interpolate_fit(vorbis_block *vb,vorbis_look_floor1 *look, int *A,int *B, int del){
  long i;
  long posts=look->posts;
  int *output=NULL;

  if(A && B){
    output=_vorbis_block_alloc(vb,sizeof(*output)*posts);
    for(i=0;i<posts;i++){
      output[i]=((65536-del)*(A[i]&0x7fff)+del*(B[i]&0x7fff)+32768)>>16;
      if(A[i]&0x8000 && B[i]&0x8000)output[i]|=0x8000;
    }
  }
  return(output);
}

PR90263 detects subtraction “65536-del” as candidate expression. But the transformed IR didn’t show any instruction cut, because there is a reuse for “del” at “del*(B[i]&0x7fff)”.
PR90263 meant to replace expensive expression with a load from precomputed table, but current implementation failed to consider the case where sub-expression has another use inside loop body, and cannot be removed.
PR90263 inserts a load to replace use of detected candidate expression, and rely on InstCombine to get rid of dead instructions.
Current PR90263 implementation failed to consider cases where there is reuse of sub-expression from a detected candidate. And the transformed IR may not have the expected instruction cut.

Just an update, I have pushed commit Reject expression where its sub-expression is used in higher order instructions to reject case like MultiSource/Applications/oggenc/oggenc.c .

Replacing expensive expression with sub-expression used in higher order instructions doesn’t yield instruction cut, since that sub-expression cannot be eliminated.

You are discussing a single case where we both agree this is useful. But my point that it must be useful for more than one code, otherwise just apply the optimization to that code. You seem to not have found another use case in all of the llvm-test-suite (including SPEC benchmarks in Externals/)?

Furthermore, #90263 adds the pass to the default optimization pipeline, therefore its impact must be analyzed:

  • How must is the compile-time overhead?
  • What are the chances that makes code slower and by how much (regressions)?

You are arguing about an increase of 2.5KB of memory, but the pass allows up to 2MB to be used. For the discussion of potential regressions the best-case scenario not relevant, but worst-case scenarios. For instance, for the sake of the argument let’s say I have a code with 1000 files (so MaxTotalSizeThreshold applies to each file separately) containing (auto-generated) loops referencing a 250.000 entry lookup table filled at runtime. In a typical run, the loops have only a single-digit number of iterations determined at runtime. Assuming the lookup table index expressions are sufficiently complex, would your pass generate another 1000 * 250000 * 8 = 2GB of integer lookup tables into the executable?

Hey Michael,

Please see my response below.

Source code rewrite would be the last thing to consider. Other than the proposed approach #90263, I am hoping to see if there are other compiler optimizations we can do to improve codegen for that mp3decoder loop.

No, I did not find other cases to benefit from PR90263 from llvm test-suite. PR90263 is updated to reject the four functions noticed earlier from llvm test-suite, since the transformed IR is not desirable.

Spec2006/Povray and Spec2017/Povray have loop with pattern below

    for (addx = -2; addx <= 2; addx++)  
      for (addy = -2; addy <= 2; addy++)      
          for (addz = -2; addz <= 2; addz++)              
              cvc = 25*(2+addx)+5*(2+addy)+2+addz;
              array[cvc][...] = ...;

‘cvc’ isn’t complex regarding to loop depth level 3, rejected by #90263 commit#2

ffmpeg has similar code pattern to mp3decode, but some of them don’t have compile-time known trip count.

There seems to be a lot of noise when measuring compile-time difference form llvm test-suite.
With PR90263, compile-time diff range from +14.3% to -13.5%. But when I run baseline twice, and compare compile-time difference from two baselines, I got compile-time diff range from +10.1% to -13.3%.
So that’s not a good indicator saying PR90263 introduce much compile-time overhead.

This will require more coverage to see bad IR transformation, and improve PR90263 to reject such cases.
I am not seeing other successful transformation other than the good one (mp3decoder) and bad ones already rejected (the four function mentioned earlier from llvm test-suite).

This will need to be considered when bringing PR90263 to community mainline. Yes, we need to come up with a better mechanism to control the potential overall codesize increase to a reasonable range.

I also agree that I would prefer having the compiler do such optimizations automatically, also understand that including an optimization adds a maintenance burden onto the community, and onto users (compiler slowdown, risk of bugs and regressions). I don’t think it is fair to put this onto the community to safe a single party from changing their code.

Isn’t that exactly what we are discussing here? What the tradeoffs are?