[llvm] r241884 - [NVPTX] declare no vector registers

[Hal Finkel]

----- Original Message -----
> From: "Jingyue Wu" <jingyue at google.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Thursday, July 9, 2015 11:31:56 PM
> Subject: [llvm] r241884 - [NVPTX] declare no vector registers
> 
> Author: jingyue
> Date: Thu Jul  9 23:31:56 2015
> New Revision: 241884
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=241884&view=rev
> Log:
> [NVPTX] declare no vector registers
> 
> Summary:
> Without this patch, LoopVectorizer in certain cases (see
> loop-vectorize.ll)
> produces code with complex control flow which hurts later
> optimizations. Since
> NVPTX doesn't have vector registers in LLVM's sense
> (NVPTXTTI::getRegisterBitWidth(true) == 32), we for now declare no
> vector
> registers to effectively disable loop vectorization.

I'm surprised that this is necessary, but indeed, the default implementation that we have is:

  unsigned getRegisterBitWidth(bool Vector) { return 32; }

in both include/llvm/Analysis/TargetTransformInfoImpl.h and in include/llvm/CodeGen/BasicTTIImpl.h. I don't believe that we should be "vectorizing" anything if we don't really have target information. We have a related inconsistency is that include/llvm/Analysis/TargetTransformInfoImpl.h has:

  unsigned getNumberOfRegisters(bool Vector) { return 8; }

but include/llvm/CodeGen/BasicTTIImpl.h has:

  unsigned getNumberOfRegisters(bool Vector) { return 1; }

This latter function is generally the way we expect targets to disable vectorization. The loop vectorizer turns itself off early when:

    // If the target claims to have no vector registers don't attempt
    // vectorization.
    if (!TTI->getNumberOfRegisters(true))
      return false;

but this is somewhat broken too because it also gets rid of scalar interleaving (unrolling), even if the target would request it, if the target has no vector registers.

I'd prefer that we do the following:

 1. Partially revert this commit: Leave the test case, but instead, make the default implementation of getNumberOfRegisters return 0 for the vector case. There is no reason why every generic target should need to add this override to prevent the loop vectorizer from doing silly things.

 2. Fix the loop vectorizer so that its initialization check is really:

 if (!TTI->getNumberOfRegisters(true) && TTI.getMaxInterleaveFactor(1) < 2)

Thanks again,
Hal

> 
> Reviewers: jholewinski
> 
> Subscribers: jingyue, llvm-commits, jholewinski
> 
> Differential Revision: http://reviews.llvm.org/D11089
> 
> Added:
>     llvm/trunk/test/CodeGen/NVPTX/loop-vectorize.ll
> Modified:
>     llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
>     llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.h
> 
> Modified: llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp?rev=241884&r1=241883&r2=241884&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
> (original)
> +++ llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp Thu Jul
>  9 23:31:56 2015
> @@ -117,3 +117,9 @@ unsigned NVPTXTTIImpl::getArithmeticInst
>                                           Opd1PropInfo,
>                                           Opd2PropInfo);
>    }
>  }
> +
> +unsigned NVPTXTTIImpl::getNumberOfRegisters(bool Vector) {
> +  if (Vector)
> +    return 0;
> +  return BaseT::getNumberOfRegisters(Vector);
> +}
> 
> Modified: llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.h?rev=241884&r1=241883&r2=241884&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.h (original)
> +++ llvm/trunk/lib/Target/NVPTX/NVPTXTargetTransformInfo.h Thu Jul  9
> 23:31:56 2015
> @@ -58,6 +58,8 @@ public:
>        TTI::OperandValueKind Opd2Info = TTI::OK_AnyValue,
>        TTI::OperandValueProperties Opd1PropInfo = TTI::OP_None,
>        TTI::OperandValueProperties Opd2PropInfo = TTI::OP_None);
> +
> +  unsigned getNumberOfRegisters(bool Vector);
>  };
>  
>  } // end namespace llvm
> 
> Added: llvm/trunk/test/CodeGen/NVPTX/loop-vectorize.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/NVPTX/loop-vectorize.ll?rev=241884&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/NVPTX/loop-vectorize.ll (added)
> +++ llvm/trunk/test/CodeGen/NVPTX/loop-vectorize.ll Thu Jul  9
> 23:31:56 2015
> @@ -0,0 +1,39 @@
> +; RUN: opt < %s -O3 -S | FileCheck %s
> +
> +target datalayout = "e-i64:64-v16:16-v32:32-n16:32:64"
> +target triple = "nvptx64-nvidia-cuda"
> +
> +define void @no_vectorization(i32 %n, i32 %a, i32 %b) {
> +; CHECK-LABEL: no_vectorization(
> +; CHECK-NOT: <4 x i32>
> +; CHECK-NOT: <4 x i1>
> +entry:
> +  %cmp.5 = icmp sgt i32 %n, 0
> +  br i1 %cmp.5, label %for.body.preheader, label %for.cond.cleanup
> +
> +for.body.preheader:                               ; preds = %entry
> +  br label %for.body
> +
> +for.cond.cleanup.loopexit:                        ; preds =
> %for.body
> +  br label %for.cond.cleanup
> +
> +for.cond.cleanup:                                 ; preds =
> %for.cond.cleanup.loopexit, %entry
> +  ret void
> +
> +for.body:                                         ; preds =
> %for.body.preheader, %for.body
> +  %i.06 = phi i32 [ %inc, %for.body ], [ 0, %for.body.preheader ]
> +  %add = add nsw i32 %i.06, %a
> +  %mul = mul nsw i32 %add, %b
> +  %cmp1 = icmp sgt i32 %mul, -1
> +  tail call void @llvm.assume(i1 %cmp1)
> +  %inc = add nuw nsw i32 %i.06, 1
> +  %exitcond = icmp eq i32 %inc, %n
> +  br i1 %exitcond, label %for.cond.cleanup.loopexit, label %for.body
> +}
> +
> +declare void @llvm.assume(i1) #0
> +
> +attributes #0 = { nounwind }
> +
> +!nvvm.annotations = !{!0}
> +!0 = !{void (i32, i32, i32)* @no_vectorization, !"kernel", i32 1}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory