intel / llvm

Intel staging area for llvm.org contribution. Home for Intel LLVM-based projects.
Other
1.25k stars 738 forks source link

SYCL 2020 discrepancy with Spec on unsampled_image #12807

Closed orion160 closed 8 months ago

orion160 commented 8 months ago

On SYCL 2020 reference get_access<DataT, Mode, Targ>. Mode needs to be read, or read_write. But on Intel 2024.0.2 I get a compilation error by an static assert.

// accessor.hpp
static_assert(AccessMode == access_mode::read || AccessMode == access_mode::write, "Access mode must be either read or write.");

https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#_unsampled_image_interface

Also what is image pitch, and why can I set it on a constructor?

#include <ImfArray.h>
#include <ImfRgbaFile.h>
#include <iostream>

#include <sycl/sycl.hpp>

int main(int argc, char **argv) {
  sycl::queue q{sycl::aspect_selector(sycl::aspect::fp16)};

  Imf::Rgba *pixels = nullptr;
  int width = -1;
  int height = -1;
  Imf::RgbaChannels channels;
  Imf::Compression compression;
  try {
    Imf::RgbaInputFile file("StillLife.exr");
    channels = file.channels();
    compression = file.compression();
    Imath::Box2i dw = file.dataWindow();
    width = dw.max.x - dw.min.x + 1;
    height = dw.max.y - dw.min.y + 1;
    pixels = new Imf::Rgba[width * height];

    file.setFrameBuffer(pixels, 1, width);
    file.readPixels(dw.min.y, dw.max.y);

  } catch (const std::exception &e) {
    std::cerr << "error reading image file StillLife.exr:" << e.what()
              << std::endl;
    return 1;
  }

  std::cout << "Channels: " << channels << '\n';
  std::cout << "Compression: " << compression << '\n';
  std::cout << "Image size: " << width << "x" << height << '\n';

  try {
    Imf::RgbaOutputFile file("out.exr", width, height, Imf::WRITE_RGBA);
    file.setFrameBuffer(pixels, 1, width);
    file.writePixels(height);
  } catch (const std::exception &e) {
    std::cerr << "Error writing image file out.exr:" << e.what() << std::endl;
    return 1;
  }

  {
    sycl::unsampled_image<2> img(pixels,
                                 sycl::image_format::r16b16g16a16_sfloat,
                                 sycl::range<2>(width, height));
    std::cout << "Byte size: " << img.byte_size() << '\n';
    std::cout << "width: " << img.get_range().get(0) << '\n';
    std::cout << "height: " << img.get_range().get(1) << '\n';
    std::cout << "size: " << img.size() << '\n';
    std::cout << "Pitch: " << img.get_pitch().get(0) << std::endl;
    q.submit([&](sycl::handler &h) {
      auto acc = img.get_access<sycl::half4, sycl::access_mode::read_write>(h);
    });
  }

  delete[] pixels;

  return 0;
}

Originally posted by @orion160 in https://github.com/intel/llvm/discussions/12720

KornevNikita commented 8 months ago

Reproduced with the fresh intel/llvm build. I'm not sure if it's intended or just a small bug in assert. I guess @steffenlarsen can provide more info as he implemented this.

steffenlarsen commented 8 months ago

Hi @orion160 ! The assertion you see is in relation to the following statement from the SYCL 2020 specification:

[...] Both unsampled image accessor classes support the access_mode::read and access_mode::write access modes. In addition, the host_unsampled_image_accessor class supports access_mode::read_write.

The accessor you are trying to create is a device accessor, given the default image_target::device. That said, it is a little confusing that the default for non-const DataT is invalid.

Also what is image pitch, and why can I set it on a constructor?

The pitch lets you specify how many bytes wide rows and columns are in the image. This isn't necessarily the same as the corresponding dimension in the image range, as images may have padding at the end of each row and column.

As a side note, all devices in DPCPP will report false for info::device::image_support so using unsampled_image and the corresponding unsamped_image_accessor on them is not supported. You can still use SYCL 1.2.1 images on devices that report support for aspect::ext_intel_legacy_image, though note that they are likely not going to stay around forever. Alternatively, you could have a go at sycl_ext_oneapi_bindless_images.

orion160 commented 8 months ago

Definetively the default ctor triggered the issue.