[PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
Katya Romanova via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 27 15:41:51 PST 2016
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.
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. :)
More information about the cfe-commits