Separate builtins for x84-64 and i386; implement __mulh and __umulh

[agutowski]

Summary

We need x86-64-specific builtins if we want to implement some of the MS intrinsics - winnt.h contains definitions of some functions for i386, but not for x86-64 (for example _InterlockedOr64), which means that we cannot treat them as builtins for both i386 and x86-64, because then we have definitions of builtin functions in winnt.h on i386.

[agutowski]

include/clang/Basic/BuiltinsX86_32.def

1 ↗

(On Diff #71470)

Will we ever need that file? It seems to nicely complement the 64-bit one, but if it's going to be empty forever maybe it's better to remove it.

[rnk]

lib/Basic/Targets.cpp

2319 ↗

(On Diff #71470)

I'd rather not duplicate this readonly data. I had this clever idea that we'd do something like:

const Builtin::Info BuiltinInfo[] = {
...
#include "BuiltinsX86_32.def"
#include "BuiltinsX86.def"
#include "BuiltinsX86_64.def"
};

And then our makeArrayRef call would take the appropriate parts.

[rnk]

include/clang/Basic/BuiltinsX86_32.def

1 ↗

(On Diff #71470)

I guess we should remove it. Take a look at isX86_64Builtin in Sema/SemaChecking.cpp. It might be good to, as a follow-up change, move those builtins over to BuiltinsX86_64.def.

[agutowski]

remove BuiltinsX86_32.def; reduce redundancy in x86 builtins info

[rnk]

Let's avoid the duplicate enum, otherwise looks good

include/clang/Basic/TargetBuiltins.h

100 ↗

(On Diff #73375)

I think this would be better with just one enum to reduce compilation time:

  /// \brief X86 builtins
  namespace X86 {
  enum {
    LastTIBuiltin = clang::Builtin::FirstTSBuiltin - 1,
#define BUILTIN(ID, TYPE, ATTRS) BI##ID,
#include "clang/Basic/BuiltinsX86.def"
    FirstX86_64Builtin,
    LastX86CommonBuiltin = FirstX86_64Builtin - 1,
#define BUILTIN(ID, TYPE, ATTRS) BI##ID,
#include "clang/Basic/BuiltinsX86_64.def"
    LastTSBuiltin
  };
  }

[agutowski]

merge enums

[agutowski]

include/clang/Basic/TargetBuiltins.h

100 ↗

(On Diff #73375)

Nice, thanks!
As far as I see, it creates some inconsistency in usage of the word "last", because it's used wrong everywhere else - LastTSBuiltin is the number of the last target-specific builtin plus one, while LastX86CommonBuiltin doesn't have this additional one. I would leave it like that, but wanted to point that out, in case you think it's not acceptable.

[agutowski]

include/clang/Basic/TargetBuiltins.h

100 ↗

(On Diff #73375)

Ah, now I see that LastTIBuiltin is also what its name suggests, so it's only LastTSBuiltin that's different.

[rnk]

lgtm

cfe/trunk/include/clang/Basic/BuiltinsX86_64.def

+//===--- BuiltinsX86_64.def - X86-64 Builtin function database --*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the X86-64-specific builtin function database. Users of
+// this file must define the BUILTIN macro to make use of this information.
+//
+//===----------------------------------------------------------------------===//
+
+// The format of this database matches clang/Basic/Builtins.def.
+
+#if defined(BUILTIN) && !defined(TARGET_HEADER_BUILTIN)
+#  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) BUILTIN(ID, TYPE, ATTRS)
+#endif
+
+TARGET_HEADER_BUILTIN(__mulh,  "LLiLLiLLi",    "nch", "intrin.h", ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(__umulh, "ULLiULLiULLi", "nch", "intrin.h", ALL_MS_LANGUAGES, "")
+
+#undef BUILTIN
+#undef TARGET_HEADER_BUILTIN

cfe/trunk/include/clang/Basic/TargetBuiltins.h

#include "clang/Basic/BuiltinsNVPTX.def"
   #define BUILTIN(ID, TYPE, ATTRS) BI##ID,
   #include "clang/Basic/BuiltinsAMDGPU.def"
     LastTSBuiltin
   };
   }
 
   /// \brief X86 builtins
   namespace X86 {
-    enum {
-        LastTIBuiltin = clang::Builtin::FirstTSBuiltin-1,
+    LastTIBuiltin = clang::Builtin::FirstTSBuiltin - 1,
 #define BUILTIN(ID, TYPE, ATTRS) BI##ID,
 #include "clang/Basic/BuiltinsX86.def"
-        LastTSBuiltin
-    };
+    FirstX86_64Builtin,
+    LastX86CommonBuiltin = FirstX86_64Builtin - 1,
+#define BUILTIN(ID, TYPE, ATTRS) BI##ID,
+#include "clang/Basic/BuiltinsX86_64.def"
   }
 
   /// \brief Flags to identify the types for overloaded Neon builtins.
   ///
   /// These must be kept in sync with the flags in utils/TableGen/NeonEmitter.h.
   class NeonTypeFlags {
     enum {
       EltTypeMask = 0xf,

cfe/trunk/lib/Basic/Targets.cpp

if (getTriple().getArch() == llvm::Triple::amdgcn) {
     default:
       llvm_unreachable("unhandled subtarget");
     }
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeatureVec);
 }
 
-// Namespace for x86 abstract base class
-const Builtin::Info BuiltinInfo[] = {
+const Builtin::Info BuiltinInfoX86[] = {
 #define BUILTIN(ID, TYPE, ATTRS)                                               \
   { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr },
-#define LIBBUILTIN(ID, TYPE, ATTRS, HEADER)                                    \
-  { #ID, TYPE, ATTRS, HEADER, ALL_LANGUAGES, nullptr },
 #define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE)                               \
   { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE },
 #define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANGS, FEATURE)         \
   { #ID, TYPE, ATTRS, HEADER, LANGS, FEATURE },
 #include "clang/Basic/BuiltinsX86.def"
+
+#define BUILTIN(ID, TYPE, ATTRS)                                               \
+  { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr },
+#define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANGS, FEATURE)         \
+  { #ID, TYPE, ATTRS, HEADER, LANGS, FEATURE },
+#include "clang/Basic/BuiltinsX86_64.def"
 };
 
+
 static const char* const GCCRegNames[] = {
   "ax", "dx", "cx", "bx", "si", "di", "bp", "sp",
   "st", "st(1)", "st(2)", "st(3)", "st(4)", "st(5)", "st(6)", "st(7)",
   "argp", "flags", "fpcr", "fpsr", "dirflag", "frame",
   "xmm0", "xmm1", "xmm2", "xmm3", "xmm4", "xmm5", "xmm6", "xmm7",
   "mm0", "mm1", "mm2", "mm3", "mm4", "mm5", "mm6", "mm7",
   "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
   "xmm8", "xmm9", "xmm10", "xmm11", "xmm12", "xmm13", "xmm14", "xmm15",
public:
   X86TargetInfo(const llvm::Triple &Triple, const TargetOptions &)
       : TargetInfo(Triple) {
     LongDoubleFormat = &llvm::APFloat::x87DoubleExtended;
   }
   unsigned getFloatEvalMethod() const override {
     // X87 evaluates with 80 bits "long double" precision.
     return SSELevel == NoSSE ? 2 : 0;
   }
-  ArrayRef<Builtin::Info> getTargetBuiltins() const override {
-    return llvm::makeArrayRef(BuiltinInfo,
-                             clang::X86::LastTSBuiltin-Builtin::FirstTSBuiltin);
-  }
   ArrayRef<const char *> getGCCRegNames() const override {
     return llvm::makeArrayRef(GCCRegNames);
   }
   ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override {
     return None;
   }
   ArrayRef<TargetInfo::AddlRegName> getGCCAddlRegNames() const override {
     return llvm::makeArrayRef(AddlRegNames);
bool validateOperandSize(StringRef Constraint,
     case 'D':
       return Size <= 32;
     case 'A':
       return Size <= 64;
     }
 
     return X86TargetInfo::validateOperandSize(Constraint, Size);
   }
+  ArrayRef<Builtin::Info> getTargetBuiltins() const override {
+    return llvm::makeArrayRef(BuiltinInfoX86, clang::X86::LastX86CommonBuiltin -
+                                                  Builtin::FirstTSBuiltin + 1);
+  }
 };
 
 class NetBSDI386TargetInfo : public NetBSDTargetInfo<X86_32TargetInfo> {
 public:
   NetBSDI386TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : NetBSDTargetInfo<X86_32TargetInfo>(Triple, Opts) {}
 
   unsigned getFloatEvalMethod() const override {
if (RegName.equals("rsp") || RegName.equals("rbp")) {
       HasSizeMismatch = RegSize != 64;
       return true;
     }
 
     // Check if the register is a 32-bit register the backend can handle.
     return X86TargetInfo::validateGlobalRegisterVariable(RegName, RegSize,
                                                          HasSizeMismatch);
   }
+  ArrayRef<Builtin::Info> getTargetBuiltins() const override {
+    return llvm::makeArrayRef(BuiltinInfoX86,
+                              X86::LastTSBuiltin - Builtin::FirstTSBuiltin);
+  }
 };
 
 // x86-64 Windows target
 class WindowsX86_64TargetInfo : public WindowsTargetInfo<X86_64TargetInfo> {
 public:
   WindowsX86_64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : WindowsTargetInfo<X86_64TargetInfo>(Triple, Opts) {
     WCharType = UnsignedShort;

cfe/trunk/lib/CodeGen/CGBuiltin.cpp

#undef INTRINSIC_X86_XSAVE_ID
   case X86::BI__builtin_ia32_cmpneqsd:
     return getCmpIntrinsicCall(Intrinsic::x86_sse2_cmp_sd, 4);
   case X86::BI__builtin_ia32_cmpnltsd:
     return getCmpIntrinsicCall(Intrinsic::x86_sse2_cmp_sd, 5);
   case X86::BI__builtin_ia32_cmpnlesd:
     return getCmpIntrinsicCall(Intrinsic::x86_sse2_cmp_sd, 6);
   case X86::BI__builtin_ia32_cmpordsd:
     return getCmpIntrinsicCall(Intrinsic::x86_sse2_cmp_sd, 7);
+
+  case X86::BI__mulh:
+  case X86::BI__umulh: {
+    Value *LHS = EmitScalarExpr(E->getArg(0));
+    Value *RHS = EmitScalarExpr(E->getArg(1));
+    llvm::Type *ResType = ConvertType(E->getType());
+    llvm::Type *Int128Ty = llvm::IntegerType::get(getLLVMContext(), 128);
+
+    bool IsSigned = (BuiltinID == X86::BI__mulh);
+    LHS = Builder.CreateIntCast(LHS, Int128Ty, IsSigned);
+    RHS = Builder.CreateIntCast(RHS, Int128Ty, IsSigned);
+
+    Value *MulResult, *HigherBits;
+    if (IsSigned) {
+      MulResult = Builder.CreateNSWMul(LHS, RHS);
+      HigherBits = Builder.CreateAShr(MulResult, 64);
+    } else {
+      MulResult = Builder.CreateNUWMul(LHS, RHS);
+      HigherBits = Builder.CreateLShr(MulResult, 64);
+    }
+
+    HigherBits = Builder.CreateIntCast(HigherBits, ResType, IsSigned);
+    return HigherBits;
+  }
   }
 }
 
 
 Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,
                                            const CallExpr *E) {
   SmallVector<Value*, 4> Ops;
 

cfe/trunk/lib/Headers/intrin.h

 void __incgsdword(unsigned long);
 void __incgsqword(unsigned long);
 void __incgsword(unsigned long);
 unsigned char __lwpins64(unsigned __int64, unsigned int, unsigned int);
 void __lwpval64(unsigned __int64, unsigned int, unsigned int);
 unsigned __int64 __lzcnt64(unsigned __int64);
 static __inline__
 void __movsq(unsigned long long *, unsigned long long const *, size_t);
+static __inline__
 __int64 __mulh(__int64, __int64);
 static __inline__
 unsigned __int64 __popcnt64(unsigned __int64);
 static __inline__
 unsigned char __readgsbyte(unsigned long);
 static __inline__
 unsigned long __readgsdword(unsigned long);
 static __inline__

 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 __DEFAULT_FN_ATTRS
-__umulh(unsigned __int64 _Multiplier, unsigned __int64 _Multiplicand) {
-  unsigned __int128 _FullProduct =
-      (unsigned __int128)_Multiplier * (unsigned __int128)_Multiplicand;
-  return _FullProduct >> 64;
-}
+static __inline__
+unsigned __int64 __umulh(unsigned __int64, unsigned __int64);
 
 #endif /* __x86_64__ */
 
 /*----------------------------------------------------------------------------*\
 |* Multiplication
 \*----------------------------------------------------------------------------*/
 static __inline__ __int64 __DEFAULT_FN_ATTRS
 __emul(int __in1, int __in2) {

cfe/trunk/test/CodeGen/ms-intrinsics.c

 // CHECK-I386: define i32 @test__readfsdword(i32 %Offset){{.*}}{
 // CHECK-I386:   [[PTR:%[0-9]+]] = inttoptr i32 %Offset to i32 addrspace(257)*
 // CHECK-I386:   [[VALUE:%[0-9]+]] = load volatile i32, i32 addrspace(257)* [[PTR]], align 4
 // CHECK-I386:   ret i32 [[VALUE:%[0-9]+]]
 // CHECK-I386: }
 #endif
 
 #if defined(__x86_64__)
+__int64 test__mulh(__int64 a, __int64 b) {
+  return __mulh(a, b);
+}
+// CHECK-X64-LABEL: define i64 @test__mulh(i64 %a, i64 %b)
+// CHECK-X64: = mul nsw i128 %
+
 unsigned __int64 test__umulh(unsigned __int64 a, unsigned __int64 b) {
   return __umulh(a, b);
 }
 // CHECK-X64-LABEL: define i64 @test__umulh(i64 %a, i64 %b)
 // CHECK-X64: = mul nuw i128 %
-
 #endif
 
 char test_InterlockedExchange8(char volatile *value, char mask) {
   return _InterlockedExchange8(value, mask);
 }
 // CHECK: define{{.*}}i8 @test_InterlockedExchange8(i8*{{[a-z_ ]*}}%value, i8{{[a-z_ ]*}}%mask){{.*}}{
 // CHECK:   [[RESULT:%[0-9]+]] = atomicrmw xchg i8* %value, i8 %mask seq_cst
 // CHECK:   ret i8 [[RESULT:%[0-9]+]]

cfe/trunk/test/Sema/implicit-ms-builtin-decl.c

-// RUN: %clang_cc1 -fsyntax-only -verify %s -fms-extensions
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s -fms-extensions
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -verify %s -fms-extensions
 
 void f() {
-  (void)_byteswap_ushort(42); // expected-warning{{implicitly declaring library function '_byteswap_ushort}} \
+  (void)_byteswap_ushort(42); // expected-warning{{implicitly declaring library function '_byteswap_ushort'}} \
   // expected-note{{include the header <stdlib.h> or explicitly provide a declaration for '_byteswap_ushort'}}
-  (void)_byteswap_uint64(42LL); // expected-warning{{implicitly declaring library function '_byteswap_uint64}} \
+  (void)_byteswap_uint64(42LL); // expected-warning{{implicitly declaring library function '_byteswap_uint64'}} \
   // expected-note{{include the header <stdlib.h> or explicitly provide a declaration for '_byteswap_uint64'}}
 }
 
 void _byteswap_ulong(); // expected-warning{{incompatible redeclaration of library function '_byteswap_ulong'}} \
 // expected-note{{'_byteswap_ulong' is a builtin}}
 
 unsigned short _byteswap_ushort(unsigned short);
 unsigned long long _byteswap_uint64(unsigned long long);
 
 void g() {
   (void)_byteswap_ushort(42);
   (void)_byteswap_uint64(42LL);
 }
+
+#if defined(__x86_64__)
+void h() {
+  (void)__mulh(21, 2);  // expected-warning{{implicitly declaring library function '__mulh'}} \
+  // expected-note{{include the header <intrin.h> or explicitly provide a declaration for '__mulh'}}
+  (void)__umulh(21, 2); // expected-warning{{implicitly declaring library function '__umulh'}} \
+  // expected-note{{include the header <intrin.h> or explicitly provide a declaration for '__umulh'}}
+}
+
+long long __mulh(long long, long long);
+unsigned long long __umulh(unsigned long long, unsigned long long);
+
+void i() {
+  (void)__mulh(21, 2);
+  (void)__umulh(21, 2);
+}
+#endif
+
+#if defined(i386)
+void h() {
+  (void)__mulh(21LL, 2LL);  // expected-warning{{implicit declaration of function '__mulh' is invalid}}
+  (void)__umulh(21ULL, 2ULL);  // expected-warning{{implicit declaration of function '__umulh' is invalid}}
+}
+#endif