[PATCH] D25353: Implement __emul, __emulu, _mul128 and _umul128 MS intrinsics

[Hans Wennborg via cfe-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm with some nits



================
Comment at: lib/CodeGen/CGBuiltin.cpp:7592
     Value *LHS = EmitScalarExpr(E->getArg(0));
     Value *RHS = EmitScalarExpr(E->getArg(1));
     llvm::Type *ResType = ConvertType(E->getType());
----------------
While you're here, or if you want to do a follow-up, I think this code could just use Ops[0] and Ops[1] directly, as the "EmitScaleExpr" was done earlier.


================
Comment at: lib/Headers/intrin.h:420
 unsigned __int64 _shrx_u64(unsigned __int64, unsigned int);
-/*
- * Multiply two 64-bit integers and obtain a 64-bit result.
- * The low-half is returned directly and the high half is in an out parameter.
- */
-static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_umul128(unsigned __int64 _Multiplier, unsigned __int64 _Multiplicand,
-         unsigned __int64 *_HighProduct) {
-  unsigned __int128 _FullProduct =
-      (unsigned __int128)_Multiplier * (unsigned __int128)_Multiplicand;
-  *_HighProduct = _FullProduct >> 64;
-  return _FullProduct;
-}
+static __inline__ unsigned __int64 _umul128(unsigned __int64 _Multiplier,
+                                            unsigned __int64 _Multiplicand,
----------------
Maybe move this one next to __mul128 (or move that here), so they're next to each other in the file.


================
Comment at: test/CodeGen/ms-x86-intrinsics.c:57
+// CHECK-X64-LABEL: define i64 @test_mul128(i64 %Multiplier, i64 %Multiplicand, i64*{{[a-z_ ]*}}%HighProduct)
+// CHECK-X64: = mul nsw i128 %
+// CHECK-X64: store i64 %
----------------
it would be good to check for the sext's too (and zext's for _umul128)


https://reviews.llvm.org/D25353