problems with OpCopyMemory

[bashbaug]

Reverse translation (from SPIR-V to LLVM) does not appear to be working propery for OpCopyMemory. Note that OpCopyMemorySized is working correctly, and there do not appear to be any tests for OpCopyMemory.

I am encountering two problems with the following tester (note: this is an except of a test for the SPIR-V 1.4 feature where OpCopyMemory can accept two optional memory operands):

; SPIR-V
; Version: 1.4
               OpCapability Addresses
               OpCapability Kernel
               OpMemoryModel Physical64 OpenCL
               OpEntryPoint Kernel %kernel "copymemory_test"
       %uint = OpTypeInt 32 0
       %void = OpTypeVoid
   %gptr_int = OpTypePointer CrossWorkgroup %uint
   %pptr_int = OpTypePointer Function %uint
 %kernel_sig = OpTypeFunction %void %gptr_int
    %uint_42 = OpConstant %uint 42
     %kernel = OpFunction %void None %kernel_sig
        %dst = OpFunctionParameter %gptr_int
      %entry = OpLabel
     %pvalue = OpVariable %pptr_int Function %uint_42
               OpCopyMemory %dst %pvalue
               OpReturn
               OpFunctionEnd

1. The asserts are incorrect in SPIRV::SPIRVCopyMemory::validate, because they are checking Id, but this instruction does not set an Id. This one is pretty easy to fix - something like:

   void validate() const override {
       assert(getValueType(Target)->isTypePointer() && "Invalid Target type");
       assert(getValueType(Source)->isTypePointer() && "Invalid Source type");
       assert(!(getValueType(Target)->getPointerElementType()->isTypeVoid()) &&
              "Invalid Target pointer type");
       assert(!(getValueType(Source)->getPointerElementType()->isTypeVoid()) &&
              "Invalid Target pointer type");
       assert(getValueType(Target)->getPointerElementType() ==
                  getValueType(Source)->getPointerElementType() &&
              "Mismatched Target and Source types");
       SPIRVInstruction::validate();
   }

2. There is no special handling for OpCopyMemory (there is for OpCopyMemorySized), so translating OpCopyMemory falls into the default transSPIRVBuiltinFromInst, which does not work properly. I suspect that OpCopyMemory should be lowered into a call to llvm.memcpy, similar to OpCopyMemorySized, but the size will be dependent on the pointed-to types vs. provided explicitly.

Once these problems are fixed we should add some tests for OpCopyMemory also.

[svenvh]

Thanks for the detailed report @bashbaug!

I've gone with a minimal fix for the first issue in https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2770 ; more thorough validation should be spirv-val's job.

For the second issue, I expect that we can treat OpCopyMemory pretty much like OpCopyMemorySized indeed.