[clang] [Clang][TableGen] Support specifying address space in clang builtin prototypes (PR #108497)

Vikram Hegde via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 20 04:04:33 PDT 2024


vikramRH wrote:

> > > > Gentle ping @AaronBallman , @philnik777 , @fpetrogalli :)
> > > 
> > > 
> > > Ah, sorry -- because the PR is marked as a Draft, I figured it wasn't ready for review yet.
> > > I think I'd rather this was expressed differently; we already don't put attribute information in the prototype anyway (`noexcept` as an example), so I'd prefer to continue down that road and put the address space information into the `Attributes` field. e.g.,
> > > ```
> > > def BuiltinCPUIs : Builtin {
> > >   let Spellings = ["__builtin_cpu_is"];
> > >   let Attributes = [NoThrow, Const, AddressSpace<2>];
> > >   let Prototype = "bool(char const*)";
> > > }
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > I think that makes it more clean in terms of specifying the attribute, and it also means we can name the address spaces in `BuiltinsBase.td` if we would like, which is even easier for folks to understand when reading `Builtins.td`
> > > WDYT?
> > 
> > 
> > Thanks for the reply @AaronBallman . The reason this is still a draft is that I wanted it to be an initial proposal to get some inputs and a consensus on the final design. and about it being part of the "Attributes" field, one major issue is that the address space information should be per argument including the return type. "Attributes" field currently expresses attributes to the function. If attribute in the prototype is not desired, probably a new field that lets us specify per argument attributes makes sense ?
> 
> Oh! I hadn't realized this was needed on a per-parameter basis. Oof that makes this more awkward. I'd still love to avoid writing this as part of the signature; I think we could use the existing `IndexedAttribute` to specify which argument the attribute applies to. e.g.,
> 
> ```
> class AddressSpace<int Idx, int AddrSpaceNum> : IndexedAttribute<"something", Idx> {
>   int SpaceNum = AddrSpaceNum;
> }
> ```

Makes sense, I will give this a try and update the PR

https://github.com/llvm/llvm-project/pull/108497


More information about the cfe-commits mailing list