craig.topper added inline comments.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:8420
+ if (IsUnsigned) {
+ MinVal = (IsDW) ? llvm::APInt::getMinValue(16).getZExtValue()
+ : llvm::APInt::getMinValue(8).getZExtValue();
----------------
Why can't these just be APInts instead of uint64_t? Is this so that APInt
widths don't have to match RTy below? I'd rather you just created the narrow
APInt and then called sext/zext on it to get it to the right width.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:8420
+ if (IsUnsigned) {
+ MinVal = (IsDW) ? llvm::APInt::getMinValue(16).getZExtValue()
+ : llvm::APInt::getMinValue(8).getZExtValue();
----------------
craig.topper wrote:
> Why can't these just be APInts instead of uint64_t? Is this so that APInt
> widths don't have to match RTy below? I'd rather you just created the narrow
> APInt and then called sext/zext on it to get it to the right width.
Pre-select the 8 or 16 based on IsDW. Then you don't need to check IsDW 4
times. You just need to pass the right width.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:8432
+ SmallVector<uint32_t, 16> ShuffleMask;
+ ShuffleMask.clear();
+ for (int i = 0, i1 = 0, i2 = 0, d = (IsDW) ? 4 : 8; i < NumElts; ++i)
----------------
Clearing isn't necessary if you just created it.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:8433
+ ShuffleMask.clear();
+ for (int i = 0, i1 = 0, i2 = 0, d = (IsDW) ? 4 : 8; i < NumElts; ++i)
+ if ((i / d) & 1)
----------------
This loop could probably use some comments. The multiple variables make the
logic hard to follow
================
Comment at: lib/CodeGen/CGBuiltin.cpp:8443
+ Value *MaxVec = llvm::ConstantInt::get(RTy, MaxVal);
+ Res = EmitX86MinMax(CGF, ICmpInst::ICMP_SLT, {Res, MaxVec});
+ Res = EmitX86MinMax(CGF, ICmpInst::ICMP_SGT, {Res, MinVec});
----------------
Why arent' these unsigned compares for Unsigned?
================
Comment at: lib/CodeGen/CGBuiltin.cpp:8446
+ llvm::Type *VTy = llvm::VectorType::get(
+ (IsDW) ? CGF.Builder.getInt16Ty() : CGF.Builder.getInt8Ty(), NumElts);
+ return CGF.Builder.CreateTrunc(Res, VTy);
----------------
If you have the 8 or 16 selected above, you can use getIntNTy here I think.
Repository:
rC Clang
https://reviews.llvm.org/D45720
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
