kromanova added a comment.

In http://reviews.llvm.org/D15999#335653, @echristo wrote:

> Honestly if they've been reviewed like that internally I'm ok with you just 
> committing them - especially if they look like this.
>
> The only concerns I'd have are in the case of "This intrinsic corresponds to 
> the <blank> instruction" (side note, use the "the"? I commented on a case 
> inline). This isn't always the case with all of our intrinsics when the 
> compiler lowers them to a shuffle intrinsic or some such, or it's optimized, 
> etc. Personally I'd leave that line out, though I understand it exists in a 
> lot of similar documentation.


Hi Eric,

I agree. Sometimes the instruction that corresponds to a specific intrinsic is 
optimized out, sometimes it will get lowered to something else, etc.

However, I think keeping the instruction name in the documentation is extremely 
useful. In general, intrinsic documentation (especially in the form of 
comments) is not very complete. When I need to know what a specific intrinsic 
is doing (and I very often have to look up intrinsics!), I find the 
corresponding instruction name and go dig in AMD's or Intel's Architecture 
Programmer's manuals, where I could find all the details I need. Programmer's 
manuals instruction descriptions are much more detailed and complete. However, 
it's too much information to add to the comments.  :)

As you know, Intel's and MS's intrinsics guides are also specifying 
corresponding instruction names for the intrinsics. I suspect they had the same 
idea that I just described.

I briefly chatted with Paul Robinson and he suggested to say "This intrinsic is 
equivalent to the <blah> instruction" instead, because this sentence doesn't 
give a false impression that one will definitely see this particular 
instruction in the generated code. Intel's intrinsics documentation says 
something like that, e.g: "The corresponding IntelĀ® AVX instruction is VBLENDPD"

What do you think/prefer?

And, yes, I will add "the" before "corresponds to". Thanks! Easy enough with 
the generator. :)


http://reviews.llvm.org/D15999



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to