[OpenCL] Representing kernel attributes by LLVM target-dependent attribute-value pairs instead of metadata

Hi,

When SPIR 1.2/2.0 spec was developed, there was no target-dependent attribute-value pair in LLVM. As such, kernel attributes were represented as metadata.

This caused lots of inconvenience since these metadata are difficult to manipulate/change when kernel functions went through transformations in backends.

Now LLVM supports target-dependent attribute-value pair, I am wondering whether it is time to use that to represent kernel attributes.

e.g. something like (just to give an idea, not exact llvm IR)

kernel void foo(global int*a, sampler_t s); #1
#1 = {
reqd_work_group_size="1 1 1",
kernel_arg_type="int *, sampler_t",
...
}

basically we keep the info conveyed by the original metadata but attach them to the kernel function as attribute/value pair. This will make these attributes much easier to manipulate/change.

Any feedbacks? Thanks.

Sam

It will make our life much easier I would say.

Hi Sam,

Interesting idea, I am just concerned whether target-dependent attribute-value pairs are really correct for this use case. Because kernel attributes aren’t really target dependent.

However, I agree that current approach is painful enough. Do you think switching to function metadata (see llvm@r235783) would help instead? This way we will also have the kernel attributes directly accessible from the function, simplifying querying them (as compared to what we have at the moment).

Thanks,

Anastasia

Hi Anastasia,

Target-dependent attribute-value pair is not limited to one target. It is just a pair of strings attached to a function. It can be used by different targets as far as they accept a commonly agreed attribute name.

The function metadata also requires a commonly agreed attribute name. In this aspect it is similar to target-dependent attribute-value pair. However it may be more difficult to manipulate.

Stas, any comments on this?

Thanks.

Sam

Sam,

I guess metadata attached right to the function should work as well. Nothing would hold us from forming the same list of strings in a single metadata entry. The only problem I see here if someone else would want to use function metadata attachment for some other reason, so we will need to search and/or merge it.

Given that consideration attributes looks better to me, they are already a top-level list.

I feel that both solutions offer similar functionality with slightly different APIs. Just my general feeling is that these are rather properties of an application and have nothing to do with target specific details.

The only problem I see here if someone else would want to use function metadata attachment for some other reason, so we will need to search and/or merge it.

@Stas, would it not be the same if other target-dependent attributes are attached too?

The function metadata also requires a commonly agreed attribute name. In this aspect it is similar to target-dependent attribute-value pair. However it may be more difficult to manipulate.

@Sam, could you provide more details where attributes would be more beneficial to use?

Thanks,

Anastasia

Hi,

When SPIR 1.2/2.0 spec was developed, there was no target-dependent attribute-value pair in LLVM. As such, kernel attributes were represented as metadata.

This caused lots of inconvenience since these metadata are difficult to manipulate/change when kernel functions went through transformations in backends.

Now LLVM supports target-dependent attribute-value pair, I am wondering whether it is time to use that to represent kernel attributes.

e.g. something like (just to give an idea, not exact llvm IR)

kernel void foo(global int*a, sampler_t s); #1
#1 = {
reqd_work_group_size="1 1 1",
kernel_arg_type="int *, sampler_t",
...
}

I don't really think the argument type information should be included in
the target attributes. It's optional information that the backend
doesn't need. It seems like metadata is more appropriate.

-Tom

Actually provided that we can have more than one metadata attachment on a function it should not be different from attributes.

But I have also noticed dropUnknownMetadata() implementation in r235783. I believe we need to be sure this info is not just dropped like it can now happen with metadata attachments on instruction. In this respect attributes seem better, or we will need to make our specific metadata known to llvm.

If the kernel_arg_type is optional, we will not be able to know that a sampler type kernel argument is a sampler type, since it becomes int type in LLVM. Is that OK for the backend and runtime?

Thanks.

Sam

I agree with Tom that the function metadata seems to be more appropriate for this purpose.

If the kernel_arg_type is optional, we will not be able to know that a sampler type kernel argument is a sampler type, since it becomes int type in LLVM. Is that OK for the backend and runtime?

Would representing as metadata help here?

If the kernel_arg_type is optional, we will not be able to know that a sampler type kernel argument is a sampler type, since it becomes int type in LLVM. Is that OK for the backend and runtime?

Would representing as metadata help here?

It is OK as long as it is not dropped by some passes before consumed by the backend.

Sam

I did a comparison between target-dependent function attributes and function metadata.

Here is an example of setting or adding a target-dependent function attribute,

std::string S;
llvm::raw_string_ostream SS(S);
SS << x << " " << y << " " << z;
F.setAttributes(F.getAttributes().addAttribute(Ctx, AttributeSet::FunctionIndex, "work_group_size_hint", SS.str());

To set or add a function metadata,

    llvm::Metadata *MDs[] = {
        llvm::ConstantAsMetadata::get(Builder.getInt32(x)),
        llvm::ConstantAsMetadata::get(Builder.getInt32(y)),
        llvm::ConstantAsMetadata::get(Builder.getInt32(z))};
F.setMetadata("work_group_size_hint", llvm::MDNode::get(Context, MDs));

Sometimes a kernel function is modified to insert a new argument, then the kernel argument metadata needs to be updated. For kernel attribute, the following needs to be done:

* insert info for the new argument into the string
* update the function attribute

For function metadata, since we cannot add insert a new operand into an existing MDNode, the following needs to be done

* the operands of the old MDNode need to be put into a vector of Metadata
* create a new Metadata for the new argument and insert it into the vector
* create a new MDNode with the vector
* set the new MDNode to the function

For readability of LLVM assembly, function attribute is easier to read since the name and the value stay together, e.g.

void f(int, int) #1
#1 = {"work_group_size_hint"="1 1 1", "kernel_arg_type"="int int", ...}

Whereas the operands of function metadata are scattered, e.g

void f(int, int) !work_group_size_hint !1 !kernel_arg_type !2
!1= {!1 !1 !1}
!2={!"int" !"int"}

However this may be less important.

Sam

How about following Tom's suggestion and representing reqd_work_group_size, work_group_size_hint and vector_type_hint as target-dependent function attributes, whereas representing kernel argument metadata as function metadata.

Since the former affects optimization of the code whereas the latter is mostly for reflection.

Thanks.

Sam

I opened a review as a demonstration of these two representation:

http://reviews.llvm.org/D20979

Sam