svenvh added inline comments.
================
Comment at: clang/lib/Sema/OpenCLBuiltins.td:54
+// Extension associated to a type.
+class TypeExtension<string _Ext> : AbstractExtension<_Ext>;
+
----------------
Anastasia wrote:
> I am trying to understand why would we need a special abstraction for the
> type? Would it not be easier if we just guard the BIFs by the extensions that
> allow the use of the type?
>
> We would need to separate the definitions of course but this can be more
> helpful in order to understand what overloads are available conditionally?
Yes, it is possible to achieve the same with the current `FunctionExtension`
class.
However, that would require a lot of duplication in the Builtin descriptions,
as for example many math builtins have overloads for `half` and `double` that
will have to be conditionalized. The purpose of `TypeExtension` is precisely
to avoid separating and duplicating the definitions. For example, `acos` is
currently more or less defined as:
```
def FGenTypeN : GenericType<"FGenTypeN", TypeList<[Float, Double, Half]>,
VecAndScalar>;
def : Builtin<"acos", [FGenTypeN, FGenTypeN], Attr.Const>;
```
with `double` and `half` conditionalization conveniently handled in a single
place through a `TypeExtension`.
If we would only use `FunctionExtension`s, the definition would become more
like the following:
```
def FGenTypeN : GenericType<"FGenTypeN", TypeList<[Float]>, VecAndScalar>;
def : Builtin<"acos", [FGenTypeN, FGenTypeN], Attr.Const>;
let Extension = Fp64 in {
def DGenTypeN : GenericType<"DGenTypeN", TypeList<[Double]>, VecAndScalar>;
def : Builtin<"acos", [DGenTypeN, DGenTypeN], Attr.Const>;
}
let Extension = Fp16 in {
def HGenTypeN : GenericType<"HGenTypeN", TypeList<[Half]>, VecAndScalar>;
def : Builtin<"acos", [HGenTypeN, HGenTypeN], Attr.Const>;
}
```
I personally don't think there is value in adding these explicit guards for
every conditional builtin, as the duplication makes the definitions harder to
maintain. In addition, I expect it would also increase the size of the
generated tables, as the `GenericType`s have to be split up (though I have not
measured this).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100209/new/
https://reviews.llvm.org/D100209
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
