MeteoSwiss-APN / dawn

Compiler toolchain to enable generation of high-level DSLs for geophysical fluid dynamics models
MIT License
28 stars 30 forks source link

temporary scope of field version #192

Open cosunae opened 5 years ago

cosunae commented 5 years ago

On the following example:

stencil copy_stencil {
  storage in;
  var oo;
  Do {
    vertical_region(k_start, k_end) {
      oo = in;
      in = oo[i+1];
    }
  }
};

the generated code creates a field version

      m_stencil = gridtools::make_computation<backend_t>(
          grid_, (p_in() = in), (p_in_0() = in_0),
          gridtools::make_multistage(
              gridtools::enumtype::execute<gridtools::enumtype::forward /*parallel*/>(),
              gridtools::define_caches(
                  gridtools::cache<gridtools::K, gridtools::cache_io_policy::flush>(p_in_0()),
                  gridtools::cache<gridtools::IJ, gridtools::cache_io_policy::local>(p_oo()),
                  gridtools::cache<gridtools::K, gridtools::cache_io_policy::fill_and_flush>(p_in())),
              gridtools::make_stage_with_extent<stage_0_0, extent<0, 1, 0, 0>>(p_in(), p_in_0()),
              gridtools::make_stage_with_extent<stage_0_1, extent<0, 1, 0, 0>>(p_oo(), p_in_0()),
              gridtools::make_stage_with_extent<stage_0_2, extent<0, 0, 0, 0>>(p_in(), p_oo())));

This is fine, but the problem is that p_in_0 is not properly scoped as a temporary. This is defeating the purpose of the field versioning. Being p_in_0 defined as a normal storage we still have a data hazard there

cosunae commented 5 years ago

Another issue with field versioning happen with the following example:

      oo = in;
      in = oo;

where in is versioned. We should check for SCC with solid edges (only)

cosunae commented 5 years ago

Another example that breaks field versioning:

        storage kk;
  var tmp;
  var oo;
  Do {
    vertical_region(k_start, k_end) {
      tmp = kk;
      oo = tmp;
      tmp = oo[i+1];
    }
    struct stage_0_0 {
      using kk = gridtools::accessor<0, gridtools::enumtype::in, gridtools::extent<0, 0, 0, 0, 0, 0>>;
      using oo = gridtools::accessor<1, gridtools::enumtype::inout, gridtools::extent<0, 0, 0, 0, 0, 0>>;
      using tmp_0 = gridtools::accessor<2, gridtools::enumtype::inout, gridtools::extent<0, 0, 0, 0, 0, 0>>;
      using arg_list = boost::mpl::vector<kk, oo, tmp_0>;

      template <typename Evaluation>
      GT_FUNCTION static void Do(Evaluation& eval, interval_start__end_) {
        eval(tmp_0(0, 0, 0)) = eval(kk(0, 0, 0));
        eval(oo(0, 0, 0)) = eval(tmp_0(0, 0, 0));
      }
    };

    struct stage_0_1 {
      using oo = gridtools::accessor<0, gridtools::enumtype::in, gridtools::extent<1, 1, 0, 0, 0, 0>>;
      using arg_list = boost::mpl::vector<oo>;

      template <typename Evaluation>
      GT_FUNCTION static void Do(Evaluation& eval, interval_start__end_) {
        gridtools::clang::float_type __local_tmp_18 = eval(oo(1, 0, 0));
      }
    };

tmp should not be versioned. The SCC should not be applied to temporaries, unless it happens in the same statement

cosunae commented 4 years ago

This issue is about field versioning. Since there has been work from @twicki and @jdahm I would pass this to them to comment on the status, and probably cleanup all the issues related to field versioning.