[PATCH] D25264: Implement MS _BitScan intrinsics

[David Majnemer via cfe-commits]

majnemer added inline comments.


> CGBuiltin.cpp:2640-2647
> +// Many of MSVC builtins are on both x64 and ARM; to avoid repeating code, we
> +// handle them here.
> +namespace MSVC {
> +  enum {
> +    _BitScanForward,
> +    _BitScanReverse
> +  };

This should be in an anonymous namespace. Also, consider using an `enum class` instead of an `enum` nested inside a namespace.

> CGBuiltin.cpp:2656-2684
> +    llvm::Type *ArgType = ArgValue->getType();
> +    llvm::Type *IndexType =
> +        EmitScalarExpr(E->getArg(0))->getType()->getPointerElementType();
> +    llvm::Type *ResultType = ConvertType(E->getType());
> +
> +    Value *ArgZero = llvm::Constant::getNullValue(ArgType);
> +    Value *ResZero = llvm::Constant::getNullValue(ResultType);

Does this do the right thing if the arg is zero?  I think it would if you gave the call to the intrinsic an operand of false instead of true.

https://reviews.llvm.org/D25264