Resolve PR reviews

Author: badumbatish

clang/include/clang/CIR/Dialect/IR/CIROps.td

@@ -425,6 +425,32 @@ def CIR_ConstantOp : CIR_Op<"const", [
         return boolAttr.getValue();
       llvm_unreachable("Expected a BoolAttr in ConstantOp");
     }
+    static bool isAllOnesValue(mlir::Value value) {
+      auto constOp = mlir::dyn_cast_or_null<cir::ConstantOp>(value.getDefiningOp());
+      if (!constOp)
+        return false;
+
+      // Check for -1 integers
+      if (auto intAttr = constOp.getValueAttr<cir::IntAttr>())
+        return intAttr.getValue().isAllOnes();
+
+      // Check for FP which are bitcasted from -1 integers
+      if (auto fpAttr = constOp.getValueAttr<cir::FPAttr>())
+        return fpAttr.getValue().bitcastToAPInt().isAllOnes();
+
+
+      // Check for constant vectors with splat values
+      if (cir::VectorType v = mlir::dyn_cast<cir::VectorType>(constOp.getType()))
+        if (auto vecAttr = constOp.getValueAttr<mlir::DenseElementsAttr>())
+          if (vecAttr.isSplat()) {
+            auto splatAttr = vecAttr.getSplatValue<mlir::Attribute>();
+            if (auto splatInt = mlir::dyn_cast<cir::IntAttr>(splatAttr)) {
+              return splatInt.getValue().isAllOnes();
+          }
+      }
+
+      return false;
+    }
   }];
 
   let hasFolder = 1;
@@ -1885,8 +1911,12 @@ def CIR_SelectOp : CIR_Op<"select", [
     ```
   }];
 
-  let arguments = (ins CIR_ScalarOrVectorOf<CIR_BoolType>:$condition, CIR_AnyType:$true_value,
-                       CIR_AnyType:$false_value);
+  let arguments = (ins 
+    CIR_ScalarOrVectorOf<CIR_BoolType>:$condition,
+    CIR_AnyType:$true_value,
+    CIR_AnyType:$false_value
+  );
+
   let results = (outs CIR_AnyType:$result);
 
   let assemblyFormat = [{

clang/include/clang/CIR/Dialect/IR/CIRTypeConstraints.td

@@ -266,15 +266,8 @@ class CIR_VectorTypeOf<list<Type> types, string summary = "">
             "vector of " # CIR_TypeSummaries<types>.value,
             summary)>;
 
-class CIR_VectorOf<Type T> : CIR_ConfinedType<
-  CIR_AnyVectorType,
-  [CIR_ElementTypePred<T.predicate>],
-  "CIR vector of " # T.summary>;
-
 // Type constraint accepting a either a type T or a vector of type T
-// Mimicking LLVMIR's LLVM_ScalarOrVectorOf
-class CIR_ScalarOrVectorOf<Type T> :
-    AnyTypeOf<[T, CIR_VectorOf<T>]>;
+class CIR_ScalarOrVectorOf<Type T> : AnyTypeOf<[T, CIR_VectorTypeOf<[T]>]>;
 
 // Vector of integral type
 def IntegerVector : Type<

clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp

@@ -163,6 +163,7 @@ static mlir::Value getBoolMaskVecValue(CIRGenBuilderTy &builder,
 
   if (numElems < 8) {
     SmallVector<mlir::Attribute, 4> indices;
+    indices.reserve(numElems);
     mlir::Type i32Ty = builder.getSInt32Ty();
     for (auto i : llvm::seq<unsigned>(0, numElems))
       indices.push_back(cir::IntAttr::get(i32Ty, i));
@@ -172,43 +173,11 @@ static mlir::Value getBoolMaskVecValue(CIRGenBuilderTy &builder,
   return maskVec;
 }
 
-// Helper function mirroring OG's bool Constant::isAllOnesValue()
-static bool isAllOnesValue(mlir::Value value) {
-  auto constOp = mlir::dyn_cast_or_null<cir::ConstantOp>(value.getDefiningOp());
-  if (!constOp)
-    return false;
-
-  // Check for -1 integers
-  if (auto intAttr = constOp.getValueAttr<cir::IntAttr>()) {
-    return intAttr.getValue().isAllOnes();
-  }
-
-  // Check for FP which are bitcasted from -1 integers
-  if (auto fpAttr = constOp.getValueAttr<cir::FPAttr>()) {
-    return fpAttr.getValue().bitcastToAPInt().isAllOnes();
-  }
-
-  // Check for constant vectors with splat values
-  if (cir::VectorType v = dyn_cast<cir::VectorType>(constOp.getType())) {
-    if (auto vecAttr = constOp.getValueAttr<mlir::DenseElementsAttr>()) {
-      if (vecAttr.isSplat()) {
-        auto splatAttr = vecAttr.getSplatValue<mlir::Attribute>();
-        if (auto splatInt = mlir::dyn_cast<cir::IntAttr>(splatAttr)) {
-          return splatInt.getValue().isAllOnes();
-        }
-      }
-    }
-  }
-
-  return false;
-}
-
 static mlir::Value emitX86Select(CIRGenBuilderTy &builder, mlir::Location loc,
                                  mlir::Value mask, mlir::Value op0,
                                  mlir::Value op1) {
-
   // If the mask is all ones just return first argument.
-  if (isAllOnesValue(mask))
+  if (cir::ConstantOp::isAllOnesValue(mask))
     return op0;
 
   mask = getBoolMaskVecValue(builder, loc, mask,
@@ -958,22 +927,21 @@ mlir::Value CIRGenFunction::emitX86BuiltinExpr(unsigned builtinID,
     unsigned numElts = dstTy.getSize();
     unsigned srcNumElts = cast<cir::VectorType>(ops[0].getType()).getSize();
     unsigned subVectors = srcNumElts / numElts;
+    assert(llvm::isPowerOf2_32(subVectors) && "Expected power of 2 subvectors");
     unsigned index =
         ops[1].getDefiningOp<cir::ConstantOp>().getIntValue().getZExtValue();
 
     index &= subVectors - 1; // Remove any extra bits.
     index *= numElts;
 
     int64_t indices[16];
-    for (unsigned i = 0; i != numElts; ++i)
-      indices[i] = i + index;
+    std::iota(indices, indices + numElts, index);
 
     mlir::Value zero = builder.getNullValue(ops[0].getType(), loc);
     mlir::Value res =
         builder.createVecShuffle(loc, ops[0], zero, ArrayRef(indices, numElts));
-    if (ops.size() == 4) {
+    if (ops.size() == 4)
       res = emitX86Select(builder, loc, ops[3], res, ops[2]);
-    }
 
     return res;
   }