[Beignet] [PATCH] GBE/libocl: Add beignet_get_time_stamp() to get timestamp.

[Zhigang Gong]

On Thu, Sep 18, 2014 at 03:12:01AM +0000, Song, Ruiling wrote:
> My suggestion is to make them consistent to each other.
> I prefer to use __gen_ocl_get_time_stamp().
> >>> well, the naming rule for extension built-in is xxx_function(), you can check the Khronos website. I would change __gen_ocl_simd_any() to beignet_simd_any()
Agreed, we should comply with the spec. But I can't find anything about
extension built-in function naming conventions on the Khronos website.
I checked some existing extension at:
http://www.khronos.org/registry/cl/extensions/intel/cl_intel_motion_estimation.txt
And found the following kernel function:
 _kernel void 
    block_motion_estimate_intel
    (
    accelerator_intel_t accelerator,
    __read_only  image2d_t src_image,
    __read_only  image2d_t ref_image,
    __global short2 * prediction_motion_vector_buffer,
    __global short2 * motion_vector_buffer,
    __global ushort * residuals
    );

It seems that it doesn't follow xxx_function() rule.
Could you share the link at Khronos website?

> 
> 
> These two functions should be pure internal function. We should not put them in the header file. Just put them in the ocl_misc.cl should be good enough.
> >>> I agree with you.
> 
> 
> > +  Instruction READ_ARF(Type type, Register dst, uint32_t regNum, uint32_t subRegNum) {
> > +    return internal::ReadARFInstruction(type, dst, regNum, 
> > + subRegNum).convert();  }
> 
> This is a GEN IR level instructions, but the prototype seems too low level for me.
> It's better to define some abstract ARF types in this layer, and to determine/choose the actual regNum/subRegNum in the instruction selection stage according to the actual target platform.
> 
> For example, we could define some enum values as
> GEN_ARF_TM0
> GEN_ARF_XXX
> at the ocl_misc.h file, and use this enum value in the ocl kernel.
> 
> >>> Yes, I will add some enum value in the Gen IR level, currently I don't want to add a built-in as __gen_ocl_read_arf(uint arf).
> >>> because I have to define both at ocl_misc.h and Gen IR and keep them consistent then.
> >>> if in the future we have that requirement, we can do it that time.
> 
> 
> int __gen_ocl_region(short offset, int data)
> 
> >>> this seems better than current naming. I will use this one.
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet