[PATCH] R600: Add intrinsics for math helper instructions

Matt Arsenault Matthew.Arsenault at amd.com
Mon Jun 23 14:39:05 PDT 2014


On 06/23/2014 02:16 PM, Tom Stellard wrote:
> On Mon, Jun 23, 2014 at 10:59:54AM -0700, Matt Arsenault wrote:
>> On 06/23/2014 09:04 AM, Tom Stellard wrote:
>>> On Sun, Jun 22, 2014 at 04:17:16PM -0700, Matt Arsenault wrote:
>>>> On Jun 17, 2014, at 7:10 AM, Tom Stellard <tom at stellard.net> wrote:
>>>>
>>>>> I think we should replace the r600 in the builtin name with amdgpu, this will
>>>>> prevent some confusion about what hardware is supported on.
>>>> I found a problem with this. getIntrinsicForGCCBuiltin / Clang uses Triple::getArchTypePrefix(), which is “r600” when looking for the GCCBuiltin. It never finds the ones with AMDGPU as the target prefix, so to support both of these prefixes clang would need to specially handle these to support both __builtin prefixes
>>> I normally don't care so much about naming, but in this case I would really
>>> like the builtins to use the amdgpu prefix, because r600 is really confusing.
>>> What other options do we have.  Could we add an amdgpu triple?  Can we
>>> change the getArchTypePrefix() function to return "amdgpu"?
>> The other problem is TargetPrefix also needs to be changed from
>> "AMDGPU" to "amdgpu"
>> I think we could do a combination of these things:
>>
>> 1. Rename the "AMDGPU" target prefixed intrinsics to "amdgpu"
> Is there any particular reason this needs to be lowercase ?
>
getIntrinsicForGCCBuiltin searches for the target prefix, which it gets 
from the triple arch name. These need to be the same for it to find the 
builtins. To be consistent with every other target, the arch name and 
therefore target prefix and builtin name should be all lowercase. I'm 
not sure it really needs to be lowercase other than consistency, but it 
wouldn't surprise me if somewhere assumes triple names are all lowercase 
and breaks somehow
>> 2. Change the triple, and rename all r600 intrinsics to be amdgpu,
>> and move them under the amdgpu target prefix
> Do you know what impact changing the triple will have?  Can we keep the old one too?
We can probably keep both. AArch64 already does this for arm64/aarch64

>
>> 3. Refactor the code in clang handling GCCBuiltin lookups into a
>> function, so in the special case R600 intrinsics function can also
>> lookup GCCBuiltins with the second prefix
> The intrinsics defined in IntrinsicsR600.td are never used as builtins,
> so we can move them into lib/Target/R600 if this would make things easier.
>
They're useful if you want to move the library implementations of the 
OpenCL functions into CL source files instead of directly doing them in 
IR like libclc does now. I also occasionally use them when writing small 
testcases to see if something works rather than doing the work of 
including the kernel header and linking the library. They could be 
renamed easily if nothing is using them now






More information about the llvm-commits mailing list