iu-parfunc / gibbon

A compiler for functional programs on serialized data
http://iu-parfunc.github.io/gibbon/
151 stars 13 forks source link

Port cilk to openmp #220

Closed adityagupta1089 closed 5 months ago

adityagupta1089 commented 11 months ago

Trying to port cilk to openmp to reduce dependency on it (part of bigger goal of upgrading dependencies and making all tests work)

Testing

Correctness

gibbon_main = go 20

gives

0 1 1 2 3 5 8 13 21 34 55 89 144 233 377 610 987 1597 2584 4181 6765 '#()

Also the codegen produces correctly:
```c
IntTy fltSpawnE_128_150 = n_33_98_147 - 1;
IntTy x_34_99_151;

#pragma omp task shared(x_34_99_151)
{
    x_34_99_151 = fib_par(cutoff_32_97_146, fltSpawnE_128_150);
}

IntTy fltAppE_129_152 = n_33_98_147 - 2;
IntTy y_35_100_153 =  fib_par(cutoff_32_97_146, fltAppE_129_152);

#pragma omp taskwait

bar :: Int -> Int bar b = let _ = printsym (quote "World") in b

gibbon_main :: () gibbonmain = let a = spawn (foo 2) b = bar 3 = sync _ = printint (a+b) in ()

 produces both:

WorldHello5'#()

 and

HelloWorld5'#()

 Where the codegen produces
 ```c
#pragma omp parallel
#pragma omp single
{
    add_symbol(47, "World");
    add_symbol(48, "Hello");

    IntTy a_13_29_38;

    #pragma omp task shared(a_13_29_38)
    {
        a_13_29_38 = foo(2);
    }

    IntTy b_14_30_39 =  bar(3);

    #pragma omp taskwait

    IntTy fltPrm_37_41 = a_13_29_38 + b_14_30_39;
    unsigned char wildcard__6_16_32_42 = printf("%lld", fltPrm_37_41);

    printf("'#()");
    printf("\n");
    free_symtable();
}
return 0;

Timing

ckoparkar commented 11 months ago

Thanks! This is amazing PR with a plot and everything ❤️

A minor change request: instead of renaming GetCilkWorkerNum to GetOmpWorkerNum, can you name it GetThreadId or GetThreadNum? Just something not tied to a specific implementation. So if we switch from openmp to something else, we don't have to change it every time. I'm also thinking if we should run all the icfp21 benchmarks with this patch, just to know what the difference is... It's probably not worth it since we have to switch anyway because Cilk is not an option anymore.

Another alternative to Cilk was discussed last week, parlaylib. So we should discuss this change as a group once more.

adityagupta1089 commented 10 months ago

Thanks! This is amazing PR with a plot and everything heart

A minor change request: instead of renaming GetCilkWorkerNum to GetOmpWorkerNum, can you name it GetThreadId or GetThreadNum? Just something not tied to a specific implementation. So if we switch from openmp to something else, we don't have to change it every time. I'm also thinking if we should run all the icfp21 benchmarks with this patch, just to know what the difference is... It's probably not worth it since we have to switch anyway because Cilk is not an option anymore.

I was able to rename and have all the tests pass image I will look into running icfp21 benchmarks between cilk and openmp.

Another alternative to Cilk was discussed last week, parlaylib. So we should discuss this change as a group once more.

I will take a look at using parlaylib.

ulysses4ever commented 5 months ago

The PR was closed automatically because the master branch it targeted was not active for months and was removed. Please, feel free to reopen the PR against the main branch (after rebasing).