[PATCH] D25334: Implement __stosb intrinsic as a volatile memset

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 6 10:41:38 PDT 2016


hans added inline comments.


> CGBuiltin.cpp:7604
> +  case X86::BI__stosb: {
> +    // we treat __stosb as volatile memset  - it may not generate "rep stosb"
> +    // instruction, but it will create a memset that won't be optimized away

Nit: I'd suggest capital w for "We" and ending the sentence with a period. I see many other comments in the file don't do this, but we can at least set a good example :-)

> CGBuiltin.cpp:7610
> +    Value *SizeVal = EmitScalarExpr(E->getArg(2));
> +    EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(),
> +                        E->getArg(0)->getExprLoc(), FD, 0);

Hmm, does the __stosb intrinsic require Dest to be non-null (e.g. would Dest=NULL, Count=0 be OK?) I'm not even sure what llvm's memset requires actually.

> CGBuiltin.cpp:7613
> +    Builder.CreateMemSet(Dest, ByteVal, SizeVal, true);
> +    return Dest.getPointer();
>    }

Why is it returning Dest here, and not the result of Builder.CreateMemSet?

https://reviews.llvm.org/D25334





More information about the cfe-commits mailing list