[VP][NFCI] Address various clang-tidy warnings

[simoll]

LGTM with nit.

llvm/unittests/IR/VPIntrinsicTest.cpp

253

I know const auto* follows the convention. However, i like just const auto here because do we really want to assert the type structure of what is actually just 'some' iterator to us?

[frasercrmck]

llvm/unittests/IR/VPIntrinsicTest.cpp

253

Yes I agree it's a bit weird because it looks like an iterator but it's ultimately a Type * const *. Alternatively we could perhaps explicitly use FunctionType::param_iterator. That's also a fairly common way of doing it in llvm, relatively speaking. I'm a bit wary of overriding clang-tidy in general as its opinions -- rightly or wrongly -- are what decides whether tools like editors show warnings, whether patches are landed "despite ongoing or failed builds" etc.

[simoll]

llvm/unittests/IR/VPIntrinsicTest.cpp

253

+1 for FunctionType::param_iterator

[frasercrmck]

Use FunctionType::param_iterator over auto.

[frasercrmck]

llvm/unittests/IR/VPIntrinsicTest.cpp

253

You have yourself a deal! I take it your LGTM still stands? I'll wait for your okay.

[frasercrmck]

• rebase

[frasercrmck]

reflow to 80 cols

[frasercrmck]

• undo unrelated change

llvm/lib/IR/IntrinsicInst.cpp

#include "llvm/IR/ConstrainedOps.def"
     return true;
   default:
     return false;
   }
 }
 
 ElementCount VPIntrinsic::getStaticVectorLength() const {
   auto GetVectorLengthOfType = [](const Type *T) -> ElementCount {
-    auto VT = cast<VectorType>(T);
+    const auto *VT = cast<VectorType>(T);
     auto ElemCount = VT->getElementCount();
     return ElemCount;
   };
 
   Value *VPMask = getMaskParam();
   assert(VPMask && "No mask param?");
   return GetVectorLengthOfType(VPMask->getType());
 }
bool VPIntrinsic::canIgnoreVectorLengthParam() const {
   // Note that the VP intrinsic causes undefined behavior if the Explicit Vector
   // Length parameter is strictly greater-than the number of vector elements of
   // the operation. This function returns true when this is detected statically
   // in the IR.
 
   // Check whether "W == vscale * EC.getKnownMinValue()"
   if (EC.isScalable()) {
     // Undig the DL
-    auto ParMod = this->getModule();
+    const auto *ParMod = this->getModule();
     if (!ParMod)
       return false;
     const auto &DL = ParMod->getDataLayout();
 
     // Compare vscale patterns
     uint64_t VScaleFactor;
     if (match(VLParam, m_c_Mul(m_ConstantInt(VScaleFactor), m_VScale(DL))))
       return VScaleFactor >= EC.getKnownMinValue();
     return (EC.getKnownMinValue() == 1) && match(VLParam, m_VScale(DL));
   }
 
   // standard SIMD operation
-  auto VLConst = dyn_cast<ConstantInt>(VLParam);
+  const auto *VLConst = dyn_cast<ConstantInt>(VLParam);
   if (!VLConst)
     return false;
 
   uint64_t VLNum = VLConst->getZExtValue();
   if (VLNum >= EC.getKnownMinValue())
     return true;
 
   return false;

llvm/unittests/IR/VPIntrinsicTest.cpp

 protected:
   LLVMContext Context;
 
   VPIntrinsicTest() : Context() {}
 
   LLVMContext C;
   SMDiagnostic Err;
 
-  std::unique_ptr<Module> CreateVPDeclarationModule() {
+  std::unique_ptr<Module> createVPDeclarationModule() {
     const char *BinaryIntOpcodes[] = {"add",  "sub",  "mul", "sdiv", "srem",
                                       "udiv", "urem", "and", "xor",  "or",
                                       "ashr", "lshr", "shl"};
     std::stringstream Str;
     for (const char *BinaryIntOpcode : BinaryIntOpcodes)
       Str << " declare <8 x i32> @llvm.vp." << BinaryIntOpcode
           << ".v8i32(<8 x i32>, <8 x i32>, <8 x i1>, i32) ";
 
#include "llvm/IR/VPIntrinsics.def"
 
   ASSERT_FALSE(ScopeVPID.hasValue());
   ASSERT_FALSE(ScopeOPC.hasValue());
 }
 
 /// Check that every VP intrinsic in the test module is recognized as a VP
 /// intrinsic.
 TEST_F(VPIntrinsicTest, VPModuleComplete) {
-  std::unique_ptr<Module> M = CreateVPDeclarationModule();
+  std::unique_ptr<Module> M = createVPDeclarationModule();
   assert(M);
 
   // Check that all @llvm.vp.* functions in the module are recognized vp
   // intrinsics.
   std::set<Intrinsic::ID> SeenIDs;
   for (const auto &VPDecl : *M) {
     ASSERT_TRUE(VPDecl.isIntrinsic());
     ASSERT_TRUE(VPIntrinsic::isVPIntrinsic(VPDecl.getIntrinsicID()));

 "  %r11 = call <vscale x 2 x i64> @llvm.vp.mul.nxv2i64(<vscale x 2 x i64> %si0x2, <vscale x 2 x i64> %si1x2, <vscale x 2 x i1> %smx2, i32 %vs.wat)"
 "  ret void "
 "}",
           Err, C);
 
   auto *F = M->getFunction("test_static_vlen");
   assert(F);
 
-  const int NumExpected = 12;
   const bool Expected[] = {false, true,  false, false, false, true,
                            false, false, true,  false, true,  false};
-  int i = 0;
+  const auto *ExpectedIt = std::begin(Expected);
   for (auto &I : F->getEntryBlock()) {
     VPIntrinsic *VPI = dyn_cast<VPIntrinsic>(&I);
     if (!VPI)
       continue;
 
-    ASSERT_LT(i, NumExpected);
-    ASSERT_EQ(Expected[i], VPI->canIgnoreVectorLengthParam());
-    ++i;
+    ASSERT_NE(ExpectedIt, std::end(Expected));
+    ASSERT_EQ(*ExpectedIt, VPI->canIgnoreVectorLengthParam());
+    ++ExpectedIt;
   }
 }
 
 /// Check that the argument returned by
 /// VPIntrinsic::get<X>ParamPos(Intrinsic::ID) has the expected type.
 TEST_F(VPIntrinsicTest, GetParamPos) {
-  std::unique_ptr<Module> M = CreateVPDeclarationModule();
+  std::unique_ptr<Module> M = createVPDeclarationModule();
   assert(M);
 
   for (Function &F : *M) {
     ASSERT_TRUE(F.isIntrinsic());
     Optional<unsigned> MaskParamPos =
         VPIntrinsic::getMaskParamPos(F.getIntrinsicID());
     if (MaskParamPos.hasValue()) {
       Type *MaskParamType = F.getArg(MaskParamPos.getValue())->getType();
for (unsigned OC : Opcodes) {
     ++FullTripCounts;
   }
   ASSERT_NE(FullTripCounts, 0u);
 }
 
 /// Check that going from VP intrinsic to Opcode and back results in the same
 /// intrinsic id.
 TEST_F(VPIntrinsicTest, IntrinsicIDRoundTrip) {
-  std::unique_ptr<Module> M = CreateVPDeclarationModule();
+  std::unique_ptr<Module> M = createVPDeclarationModule();
   assert(M);
 
   unsigned FullTripCounts = 0;
   for (const auto &VPDecl : *M) {
     auto VPID = VPDecl.getIntrinsicID();
     Optional<unsigned> OC = VPIntrinsic::getFunctionalOpcodeForVP(VPID);
 
     // no equivalent Opcode available
     if (!OC)
       continue;
 
     Intrinsic::ID RoundTripVPID = VPIntrinsic::getForOpcode(*OC);
 
     ASSERT_EQ(RoundTripVPID, VPID);
     ++FullTripCounts;
   }
   ASSERT_NE(FullTripCounts, 0u);
 }
 
 /// Check that VPIntrinsic::getDeclarationForParams works.
 TEST_F(VPIntrinsicTest, VPIntrinsicDeclarationForParams) {
-  std::unique_ptr<Module> M = CreateVPDeclarationModule();
+  std::unique_ptr<Module> M = createVPDeclarationModule();
   assert(M);
 
   auto OutM = std::make_unique<Module>("", M->getContext());
 
   for (auto &F : *M) {
     auto *FuncTy = F.getFunctionType();
 
     // Declare intrinsic anew with explicit types.
     std::vector<Value *> Values;
     for (auto *ParamTy : FuncTy->params())
       Values.push_back(UndefValue::get(ParamTy));
 
     ASSERT_NE(F.getIntrinsicID(), Intrinsic::not_intrinsic);
     auto *NewDecl = VPIntrinsic::getDeclarationForParams(
         OutM.get(), F.getIntrinsicID(), Values);
     ASSERT_TRUE(NewDecl);
 
     // Check that 'old decl' == 'new decl'.
     ASSERT_EQ(F.getIntrinsicID(), NewDecl->getIntrinsicID());
-    auto ItNewParams = NewDecl->getFunctionType()->param_begin();
-    auto EndItNewParams = NewDecl->getFunctionType()->param_end();
+    const auto *ItNewParams = NewDecl->getFunctionType()->param_begin();
+    const auto *EndItNewParams = NewDecl->getFunctionType()->param_end();
     for (auto *ParamTy : FuncTy->params()) {
       ASSERT_NE(ItNewParams, EndItNewParams);
       ASSERT_EQ(*ItNewParams, ParamTy);
       ++ItNewParams;
     }
   }
 }