dyung marked an inline comment as done.
dyung added inline comments.

================
Comment at: lib/Headers/mmintrin.h:55
 ///
-/// This intrinsic corresponds to the <c> VMOVD / MOVD </c> instruction.
+/// This intrinsic corresponds to the <c> MOVD </c> instruction.
 ///
----------------
efriedma wrote:
> RKSimon wrote:
> > efriedma wrote:
> > > craig.topper wrote:
> > > > kromanova wrote:
> > > > > I tried clang on Linux, x86_64, and if -mavx option is passed, we 
> > > > > generate VMOVD, if this option is omitted, we generate MOVD.
> > > > > I think I understand the rational behind this change (namely, to keep 
> > > > > MOVD, but remove VMOVD),
> > > > > since this intrinsic should use MMX registers and shouldn't have 
> > > > > corresponding AVX instruction(s).
> > > > > 
> > > > > However, that's what we generate at the moment when -mavx is passed 
> > > > > (I suspect because our MMX support is limited)
> > > > > vmovd   %edi, %xmm0
> > > > > 
> > > > > Since we are writing the documentation for clang compiler, we should 
> > > > > document what clang compiler is doing, not what is should be doing.
> > > > > Craig, what do you think? Should we revert back to VMOVD/MOVD?
> > > > > 
> > > > We can change it back to VMOVD/MOVD
> > > The reference to vmovd seems confusing.  Yes, LLVM compiles 
> > > `_mm_movpi64_epi64(_mm_cvtsi32_si64(i))` to vmovd, but that doesn't mean 
> > > either of those intrinsics "corresponds" to vmovd; that's just the 
> > > optimizer combining two operations into one.
> > Should all these _mm_cvt* intrinsics be replaced with a 'this is a utility 
> > function' style comment like the _mm_set1* intrinsics?
> In general, I think "corresponds to" should mean "if the inputs are produced 
> by an inline asm, and the output is used by an inline asm, and the lowering 
> will produce a single instruction, what instruction will we generate?".  
> That's unambiguous, and will usually give a useful hint to the user.  In this 
> case, on trunk, the answer is consistently "movd".
> 
> Otherwise, it's not clear what it means for an intrinsic to correspond to 
> anything; optimizations exist which can modify the instructions we choose for 
> almost any intrinsic.
Thanks for the feedback. I wrote a few small examples and noted that the 
instructions generated by the compiler were the non-avx form by default for 
this and the next three _mm_cvt* intrinsics as you indicated, so I have updated 
the documentation to list the non-avx instruction. Additionally, in some 
discussions, it was pointed out that these are MMX intrinsics, so the AVX 
instructions would not have been generated.


https://reviews.llvm.org/D41517



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

Reply via email to