Missing TargetPrefix for NVVM intrinsics

Justin Lebar via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 6 10:49:13 PDT 2016


> Subject: [PATCH 1/2] NVPTX: Rename __builtin_ptx_shfl -> __nvvm_shfl

Please run clang-format over the changes to __clang_cuda_intrinsics.h,
if you wouldn't mind.

> Subject: [PATCH 3/4] NVPTX: Remove the legacy ptx intrinsics

Looks like we're losing a bunch of intrinsics here.  Some of them like
reading tid.w I don't care about (does that even work??).  But most of
the others seem to be providing real functionality that afaict isn't
available elsewhere.  Why are we removing them?

On Wed, Jul 6, 2016 at 4:32 AM, Justin Holewinski
<jholewinski at nvidia.com> wrote:
> Yeah, this is probably better handled on llvm-commits. Swapping mailing
> lists. Any particular reason these aren't up on Phabricator?
>
> Overall, the changes look good. A few comments:
>
>
> --- a/lib/Target/NVPTX/NVPTXInstrInfo.cpp
> +++ b/lib/Target/NVPTX/NVPTXInstrInfo.cpp
> @@ -112,8 +112,6 @@ bool NVPTXInstrInfo::isStoreInstr(const MachineInstr
> &MI,
>
>  bool NVPTXInstrInfo::CanTailMerge(const MachineInstr *MI) const {
>    unsigned addrspace = 0;
> -  if (MI->getOpcode() == NVPTX::INT_CUDA_SYNCTHREADS)
> -    return false;
>    if (isLoadInstr(*MI, addrspace))
>      if (addrspace == NVPTX::PTXLdStInstCode::SHARED)
>
> This should be replaced with the bar0 intrinsic. Looks like a bug that bar0
> is missing anyway.
>
> --- a/include/clang/Basic/BuiltinsNVPTX.def
> +++ b/include/clang/Basic/BuiltinsNVPTX.def
> @@ -15,50 +15,21 @@
>  // The format of this database matches clang/Basic/Builtins.def.
>
>  // Builtins retained from previous PTX back-end
>
> I would just remove the comment about being retained from the previous
> back-end. The intent is to remove all legacy compatibility.
>
> --- a/test/Analysis/DivergenceAnalysis/NVPTX/diverge.ll
> +++ b/test/Analysis/DivergenceAnalysis/NVPTX/diverge.ll
> @@ -93,20 +93,20 @@ merge:
>  ; int i = 0;
>  ; do {
>  ;   i++;                  // i here is uniform
> -; } while (i < laneid);
> +; } while (i < tid);
>  ; return i == 10 ? 0 : 1; // i here is divergent
>  ;
>  ; The i defined in the loop is used outside.
>  define i32 @loop() {
>  ; CHECK-LABEL: Printing analysis 'Divergence Analysis' for function 'loop'
>  entry:
> -  %laneid = call i32 @llvm.ptx.read.laneid()
> +  %tid = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
>    br label %loop
>  loop:
>    %i = phi i32 [ 0, %entry ], [ %i1, %loop ]
>  ; CHECK-NOT: DIVERGENT: %i =
>    %i1 = add i32 %i, 1
> -  %exit_cond = icmp sge i32 %i1, %laneid
> +  %exit_cond = icmp sge i32 %i1, %tid
>    br i1 %exit_cond, label %loop_exit, label %loop
>  loop_exit:
>    %cond = icmp eq i32 %i, 10
>
> What is the intent of the change from laneid to tid?
>
>
>
>
> On 7/5/2016 5:56 PM, Justin Bogner wrote:
>>
>> Justin Lebar <jlebar at google.com> writes:
>>>>
>>>> @jlebar, any issues on your end with removing all of the non-nvvm
>>>> intrinsics?
>>>
>>> I don't think so, if the functionality is all there from the nvvm
>>> intrinsics.  Some if it may involve clang changes, which I'm not
>>> exactly volunteering to do, but they should be simple.
>>
>> Okay, so if I've understood you both correctly, the attached six patches
>> should do the trick, for both LLVM and clang. First, they rename the
>> shfl intrinsics from __builtin_ptx_shfl to __nvvm_shfl. Second,
>> they replace uses of cuda.syncthreads with nvvm.barrier0. Third, all of
>> the ptx intrinsics and builtins are removed. Finally, I wrap the nvvm
>> intrinsics with a TargetPrefix.
>>
>> Let me know if you want me to break any of these out into specific
>> review threads on -commits.
>>
>
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information.  Any unauthorized review, use, disclosure or
> distribution
> is prohibited.  If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------


More information about the llvm-commits mailing list