[PATCH] D151923: [APFloat] Add APFloat semantic support for TF32

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: llvm/include/llvm/ADT/APFloat.h:190
+// greater throughput than single precision (32-bit) formats.
+S_FloatTF32,
 

Hmm,  this says improved precision than half but the semantics you gave say 11 
digits? Does NVIDIA document how many bits we should expect?



Comment at: llvm/lib/Support/APFloat.cpp:141
 4, -10, 4, 8, fltNonfiniteBehavior::NanOnly, fltNanEncoding::NegativeZero};
+static constexpr fltSemantics semFloatTF32 = {127, -126, 11, 19};
 static constexpr fltSemantics semX87DoubleExtended = {16383, -16382, 64, 80};

NVIDIA's 
[docs](https://docs.nvidia.com/cuda/parallel-thread-execution/#alternate-floating-point-data-formats)
 say:
> This data format is a special 32-bit floating point format supported by the 
> matrix multiply-and-accumulate instructions, with the same range as .f32 and 
> reduced precision (>=10 bits). The internal layout of tf32 format is 
> implementation defined. PTX facilitates conversion from single precision .f32 
> type to tf32 format. A register variable containing tf32 data must be 
> declared with .b32 type.

As written, it's at least 11 bits but it can change over time. Will we need 
corresponding flavors of this for future architectures over time?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151923/new/

https://reviews.llvm.org/D151923

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

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:848
   case APFloat::S_Float8E4M3FNUZ:
+  case APFloat::S_Float8E4M3B11FNUZ:
 llvm_unreachable("Tried to mangle unexpected APFloat semantics");

aaron.ballman wrote:
> Why are there no changes needed for the Itanium mangler? And when did we 
> start adding a bunch of esoteric float formats? Was there an RFC for this 
> that I missed? (If I did miss something, then sorry for the noise.)
Hi Aaron,

The Itanium mangler doesn't depend on the APFloat's semantics, it just depends 
on:
- The Clang AST type of the float.
- The bit pattern of the APFloat.

The code to handle the bit pattern is written generically, regardless of 
APFloat semantics.
Because of this and the lack of a new Clang built-in floating type results in 
no necessary changes to the Itanium mangler.

This just comes down to a quirk in how the MSVC and Itanium manglers are 
written: new APFloat semantics necessarily result in the MSVC mangler 
"noticing" while they don't for the Itanium mangler.

As to your question about an RFC, one was posted for some similar but different 
formats that people are interested in 
[here](https://discourse.llvm.org/t/rfc-adding-the-amd-graphcore-maybe-others-float8-formats-to-apfloat/67969)

The general consensus on that RFC was that APFloat should support formats that 
people use in practice. This format is incorporated as part of real hardware in 
the real world as well as an [open source compiler 
framework](https://github.com/openxla/stablehlo/blob/main/rfcs/20230309-e4m3b11.md).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146441/new/

https://reviews.llvm.org/D146441

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

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

[David Majnemer via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f086f265bf9: [APFloat] Add E4M3B11FNUZ (authored by 
majnemer).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146441/new/

https://reviews.llvm.org/D146441

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  llvm/include/llvm/ADT/APFloat.h
  llvm/lib/Support/APFloat.cpp
  llvm/unittests/ADT/APFloatTest.cpp
  mlir/include/mlir-c/BuiltinTypes.h
  mlir/include/mlir/IR/Builders.h
  mlir/include/mlir/IR/BuiltinTypes.h
  mlir/include/mlir/IR/BuiltinTypes.td
  mlir/include/mlir/IR/OpBase.td
  mlir/include/mlir/IR/Types.h
  mlir/lib/AsmParser/TokenKinds.def
  mlir/lib/AsmParser/TypeParser.cpp
  mlir/lib/Bindings/Python/IRTypes.cpp
  mlir/lib/CAPI/IR/BuiltinTypes.cpp
  mlir/lib/IR/AsmPrinter.cpp
  mlir/lib/IR/Builders.cpp
  mlir/lib/IR/BuiltinTypes.cpp
  mlir/lib/IR/MLIRContext.cpp
  mlir/lib/IR/Types.cpp
  mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
  mlir/test/IR/attribute.mlir
  mlir/test/python/ir/builtin_types.py
  mlir/utils/lldb-scripts/mlirDataFormatters.py

Index: mlir/utils/lldb-scripts/mlirDataFormatters.py
===
--- mlir/utils/lldb-scripts/mlirDataFormatters.py
+++ mlir/utils/lldb-scripts/mlirDataFormatters.py
@@ -54,6 +54,7 @@
 "mlir::Float8E4M3FNType": '"f8E4M3FN"',
 "mlir::Float8E5M2FNUZType": '"f8E5M2FNUZ"',
 "mlir::Float8E4M3FNUZType": '"f8E4M3FNUZ"',
+"mlir::Float8E4M3B11FNUZType": '"f8E4M3B11FNUZ"',
 "mlir::BFloat16Type": '"bf16"',
 "mlir::Float16Type": '"f16"',
 "mlir::Float32Type": '"f32"',
Index: mlir/test/python/ir/builtin_types.py
===
--- mlir/test/python/ir/builtin_types.py
+++ mlir/test/python/ir/builtin_types.py
@@ -202,6 +202,8 @@
 print("float:", Float8E5M2FNUZType.get())
 # CHECK: float: f8E4M3FNUZ
 print("float:", Float8E4M3FNUZType.get())
+# CHECK: float: f8E4M3B11FNUZ
+print("float:", Float8E4M3B11FNUZType.get())
 # CHECK: float: bf16
 print("float:", BF16Type.get())
 # CHECK: float: f16
Index: mlir/test/IR/attribute.mlir
===
--- mlir/test/IR/attribute.mlir
+++ mlir/test/IR/attribute.mlir
@@ -52,6 +52,10 @@
 // CHECK: float_attr = 2.00e+00 : f8E4M3FNUZ
 float_attr = 2. : f8E4M3FNUZ
   } : () -> ()
+  "test.float_attrs"() {
+// CHECK: float_attr = 2.00e+00 : f8E4M3B11FNUZ
+float_attr = 2. : f8E4M3B11FNUZ
+  } : () -> ()
   "test.float_attrs"() {
 // CHECK: float_attr = 2.00e+00 : f16
 float_attr = 2. : f16
Index: mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
===
--- mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
+++ mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
@@ -53,6 +53,7 @@
 "Float8E4M3FNType",
 "Float8E5M2Type",
 "Float8E4M3FNUZType",
+"Float8E4M3B11FNUZType",
 "Float8E5M2FNUZType",
 "F16Type",
 "F32Type",
@@ -602,6 +603,13 @@
 @staticmethod
 def isinstance(arg: Any) -> bool: ...
 
+class Float8E4M3B11FNUZType(Type):
+def __init__(self, cast_from_type: Type) -> None: ...
+@staticmethod
+def get(*args, **kwargs) -> Float8E4M3B11FNUZType: ...
+@staticmethod
+def isinstance(arg: Any) -> bool: ...
+
 class Float8E5M2FNUZType(Type):
 def __init__(self, cast_from_type: Type) -> None: ...
 @staticmethod
Index: mlir/lib/IR/Types.cpp
===
--- mlir/lib/IR/Types.cpp
+++ mlir/lib/IR/Types.cpp
@@ -38,6 +38,7 @@
 bool Type::isFloat8E4M3FN() const { return isa(); }
 bool Type::isFloat8E5M2FNUZ() const { return isa(); }
 bool Type::isFloat8E4M3FNUZ() const { return isa(); }
+bool Type::isFloat8E4M3B11FNUZ() const { return isa(); }
 bool Type::isBF16() const { return isa(); }
 bool Type::isF16() const { return isa(); }
 bool Type::isF32() const { return isa(); }
Index: mlir/lib/IR/MLIRContext.cpp
===
--- mlir/lib/IR/MLIRContext.cpp
+++ mlir/lib/IR/MLIRContext.cpp
@@ -214,6 +214,7 @@
   Float8E4M3FNType f8E4M3FNTy;
   Float8E5M2FNUZType f8E5M2FNUZTy;
   Float8E4M3FNUZType f8E4M3FNUZTy;
+  Float8E4M3B11FNUZType f8E4M3B11FNUZTy;
   BFloat16Type bf16Ty;
   Float16Type f16Ty;
   Float32Type f32Ty;
@@ -288,6 +289,7 @@
   impl->f8E4M3FNTy = TypeUniquer::get(this);
   impl->f8E5M2FNUZTy = TypeUniquer::get(this);
   impl->f8E4M3FNUZTy = TypeUniquer::get(this);
+  impl->f8E4M3B11FNUZTy = TypeUniquer::get(this);
   impl->bf16Ty = TypeUniquer::get(this);
   impl->f16Ty = TypeUniquer::get(this);
   impl->f32Ty = TypeUniquer::get(this);
@@ -892,6 +894,9 @@
 Float8E4M3FNUZType Float8E4M3FNUZType::get(MLIRContext *context) {
   return context->getImpl().f8E4M3FNUZTy;
 }
+Float8E4M3B11FNUZType 

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

[David Majnemer via Phabricator via cfe-commits]

majnemer updated this revision to Diff 506694.
majnemer added a comment.

Fix a small typo in PyFloat8E4M3B11FNUZType


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146441/new/

https://reviews.llvm.org/D146441

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  llvm/include/llvm/ADT/APFloat.h
  llvm/lib/Support/APFloat.cpp
  llvm/unittests/ADT/APFloatTest.cpp
  mlir/include/mlir-c/BuiltinTypes.h
  mlir/include/mlir/IR/Builders.h
  mlir/include/mlir/IR/BuiltinTypes.h
  mlir/include/mlir/IR/BuiltinTypes.td
  mlir/include/mlir/IR/OpBase.td
  mlir/include/mlir/IR/Types.h
  mlir/lib/AsmParser/TokenKinds.def
  mlir/lib/AsmParser/TypeParser.cpp
  mlir/lib/Bindings/Python/IRTypes.cpp
  mlir/lib/CAPI/IR/BuiltinTypes.cpp
  mlir/lib/IR/AsmPrinter.cpp
  mlir/lib/IR/Builders.cpp
  mlir/lib/IR/BuiltinTypes.cpp
  mlir/lib/IR/MLIRContext.cpp
  mlir/lib/IR/Types.cpp
  mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
  mlir/test/IR/attribute.mlir
  mlir/test/python/ir/builtin_types.py
  mlir/utils/lldb-scripts/mlirDataFormatters.py

Index: mlir/utils/lldb-scripts/mlirDataFormatters.py
===
--- mlir/utils/lldb-scripts/mlirDataFormatters.py
+++ mlir/utils/lldb-scripts/mlirDataFormatters.py
@@ -54,6 +54,7 @@
 "mlir::Float8E4M3FNType": '"f8E4M3FN"',
 "mlir::Float8E5M2FNUZType": '"f8E5M2FNUZ"',
 "mlir::Float8E4M3FNUZType": '"f8E4M3FNUZ"',
+"mlir::Float8E4M3B11FNUZType": '"f8E4M3B11FNUZ"',
 "mlir::BFloat16Type": '"bf16"',
 "mlir::Float16Type": '"f16"',
 "mlir::Float32Type": '"f32"',
Index: mlir/test/python/ir/builtin_types.py
===
--- mlir/test/python/ir/builtin_types.py
+++ mlir/test/python/ir/builtin_types.py
@@ -202,6 +202,8 @@
 print("float:", Float8E5M2FNUZType.get())
 # CHECK: float: f8E4M3FNUZ
 print("float:", Float8E4M3FNUZType.get())
+# CHECK: float: f8E4M3B11FNUZ
+print("float:", Float8E4M3B11FNUZType.get())
 # CHECK: float: bf16
 print("float:", BF16Type.get())
 # CHECK: float: f16
Index: mlir/test/IR/attribute.mlir
===
--- mlir/test/IR/attribute.mlir
+++ mlir/test/IR/attribute.mlir
@@ -52,6 +52,10 @@
 // CHECK: float_attr = 2.00e+00 : f8E4M3FNUZ
 float_attr = 2. : f8E4M3FNUZ
   } : () -> ()
+  "test.float_attrs"() {
+// CHECK: float_attr = 2.00e+00 : f8E4M3B11FNUZ
+float_attr = 2. : f8E4M3B11FNUZ
+  } : () -> ()
   "test.float_attrs"() {
 // CHECK: float_attr = 2.00e+00 : f16
 float_attr = 2. : f16
Index: mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
===
--- mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
+++ mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
@@ -53,6 +53,7 @@
 "Float8E4M3FNType",
 "Float8E5M2Type",
 "Float8E4M3FNUZType",
+"Float8E4M3B11FNUZType",
 "Float8E5M2FNUZType",
 "F16Type",
 "F32Type",
@@ -602,6 +603,13 @@
 @staticmethod
 def isinstance(arg: Any) -> bool: ...
 
+class Float8E4M3B11FNUZType(Type):
+def __init__(self, cast_from_type: Type) -> None: ...
+@staticmethod
+def get(*args, **kwargs) -> Float8E4M3B11FNUZType: ...
+@staticmethod
+def isinstance(arg: Any) -> bool: ...
+
 class Float8E5M2FNUZType(Type):
 def __init__(self, cast_from_type: Type) -> None: ...
 @staticmethod
Index: mlir/lib/IR/Types.cpp
===
--- mlir/lib/IR/Types.cpp
+++ mlir/lib/IR/Types.cpp
@@ -38,6 +38,7 @@
 bool Type::isFloat8E4M3FN() const { return isa(); }
 bool Type::isFloat8E5M2FNUZ() const { return isa(); }
 bool Type::isFloat8E4M3FNUZ() const { return isa(); }
+bool Type::isFloat8E4M3B11FNUZ() const { return isa(); }
 bool Type::isBF16() const { return isa(); }
 bool Type::isF16() const { return isa(); }
 bool Type::isF32() const { return isa(); }
Index: mlir/lib/IR/MLIRContext.cpp
===
--- mlir/lib/IR/MLIRContext.cpp
+++ mlir/lib/IR/MLIRContext.cpp
@@ -214,6 +214,7 @@
   Float8E4M3FNType f8E4M3FNTy;
   Float8E5M2FNUZType f8E5M2FNUZTy;
   Float8E4M3FNUZType f8E4M3FNUZTy;
+  Float8E4M3B11FNUZType f8E4M3B11FNUZTy;
   BFloat16Type bf16Ty;
   Float16Type f16Ty;
   Float32Type f32Ty;
@@ -288,6 +289,7 @@
   impl->f8E4M3FNTy = TypeUniquer::get(this);
   impl->f8E5M2FNUZTy = TypeUniquer::get(this);
   impl->f8E4M3FNUZTy = TypeUniquer::get(this);
+  impl->f8E4M3B11FNUZTy = TypeUniquer::get(this);
   impl->bf16Ty = TypeUniquer::get(this);
   impl->f16Ty = TypeUniquer::get(this);
   impl->f32Ty = TypeUniquer::get(this);
@@ -892,6 +894,9 @@
 Float8E4M3FNUZType Float8E4M3FNUZType::get(MLIRContext *context) {
   return context->getImpl().f8E4M3FNUZTy;
 }
+Float8E4M3B11FNUZType Float8E4M3B11FNUZType::get(MLIRContext *context) {
+  return context->getImpl().f8E4M3B11FNUZTy;
+}
 

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

[David Majnemer via Phabricator via cfe-commits]

majnemer created this revision.
majnemer added a reviewer: reedwm.
Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, 
wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, thopre, hiraditya.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a reviewer: ftynse.
Herald added a project: All.
majnemer requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, MLIR, LLVM.

X. Sun et al. (https://dl.acm.org/doi/10./3454287.3454728) published
a paper showing that an FP format with 4 bits of exponent, 3 bits of
significand and an exponent bias of 11 would work quite well for ML
applications.

Google hardware supports a variant of this format where 0x80 is used to
represent NaN, as in the Float8E4M3FNUZ format. Just like the
Float8E4M3FNUZ format, this format does not support -0 and values which
would map to it will become +0.

This format is proposed for inclusion in OpenXLA's StableHLO dialect: 
https://github.com/openxla/stablehlo/pull/1308

As part of inclusion in that dialect, APFloat needs to know how to
handle this format.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146441

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  llvm/include/llvm/ADT/APFloat.h
  llvm/lib/Support/APFloat.cpp
  llvm/unittests/ADT/APFloatTest.cpp
  mlir/include/mlir-c/BuiltinTypes.h
  mlir/include/mlir/IR/Builders.h
  mlir/include/mlir/IR/BuiltinTypes.h
  mlir/include/mlir/IR/BuiltinTypes.td
  mlir/include/mlir/IR/OpBase.td
  mlir/include/mlir/IR/Types.h
  mlir/lib/AsmParser/TokenKinds.def
  mlir/lib/AsmParser/TypeParser.cpp
  mlir/lib/Bindings/Python/IRTypes.cpp
  mlir/lib/CAPI/IR/BuiltinTypes.cpp
  mlir/lib/IR/AsmPrinter.cpp
  mlir/lib/IR/Builders.cpp
  mlir/lib/IR/BuiltinTypes.cpp
  mlir/lib/IR/MLIRContext.cpp
  mlir/lib/IR/Types.cpp
  mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
  mlir/test/IR/attribute.mlir
  mlir/test/python/ir/builtin_types.py
  mlir/utils/lldb-scripts/mlirDataFormatters.py

Index: mlir/utils/lldb-scripts/mlirDataFormatters.py
===
--- mlir/utils/lldb-scripts/mlirDataFormatters.py
+++ mlir/utils/lldb-scripts/mlirDataFormatters.py
@@ -54,6 +54,7 @@
 "mlir::Float8E4M3FNType": '"f8E4M3FN"',
 "mlir::Float8E5M2FNUZType": '"f8E5M2FNUZ"',
 "mlir::Float8E4M3FNUZType": '"f8E4M3FNUZ"',
+"mlir::Float8E4M3B11FNUZType": '"f8E4M3B11FNUZ"',
 "mlir::BFloat16Type": '"bf16"',
 "mlir::Float16Type": '"f16"',
 "mlir::Float32Type": '"f32"',
Index: mlir/test/python/ir/builtin_types.py
===
--- mlir/test/python/ir/builtin_types.py
+++ mlir/test/python/ir/builtin_types.py
@@ -202,6 +202,8 @@
 print("float:", Float8E5M2FNUZType.get())
 # CHECK: float: f8E4M3FNUZ
 print("float:", Float8E4M3FNUZType.get())
+# CHECK: float: f8E4M3B11FNUZ
+print("float:", Float8E4M3B11FNUZType.get())
 # CHECK: float: bf16
 print("float:", BF16Type.get())
 # CHECK: float: f16
Index: mlir/test/IR/attribute.mlir
===
--- mlir/test/IR/attribute.mlir
+++ mlir/test/IR/attribute.mlir
@@ -52,6 +52,10 @@
 // CHECK: float_attr = 2.00e+00 : f8E4M3FNUZ
 float_attr = 2. : f8E4M3FNUZ
   } : () -> ()
+  "test.float_attrs"() {
+// CHECK: float_attr = 2.00e+00 : f8E4M3B11FNUZ
+float_attr = 2. : f8E4M3B11FNUZ
+  } : () -> ()
   "test.float_attrs"() {
 // CHECK: float_attr = 2.00e+00 : f16
 float_attr = 2. : f16
Index: mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
===
--- mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
+++ mlir/python/mlir/_mlir_libs/_mlir/ir.pyi
@@ -53,6 +53,7 @@
 "Float8E4M3FNType",
 "Float8E5M2Type",
 "Float8E4M3FNUZType",
+"Float8E4M3B11FNUZType",
 "Float8E5M2FNUZType",
 "F16Type",
 "F32Type",
@@ -602,6 +603,13 @@
 @staticmethod
 def isinstance(arg: Any) -> bool: ...
 
+class Float8E4M3B11FNUZType(Type):
+def __init__(self, cast_from_type: Type) -> None: ...
+@staticmethod
+def get(*args, **kwargs) -> Float8E4M3B11FNUZType: ...
+@staticmethod
+def isinstance(arg: Any) -> bool: ...
+
 class Float8E5M2FNUZType(Type):
 def __init__(self, cast_from_type: Type) -> None: ...
 @staticmethod
Index: mlir/lib/IR/Types.cpp
===
--- mlir/lib/IR/Types.cpp
+++ mlir/lib/IR/Types.cpp
@@ -38,6 +38,7 @@
 bool Type::isFloat8E4M3FN() const { return isa(); }
 bool Type::isFloat8E5M2FNUZ() const { return isa(); }
 bool Type::isFloat8E4M3FNUZ() const { return isa(); }
+bool Type::isFloat8E4M3B11FNUZ() const { 

[PATCH] D141347: Remove the ThreadLocal template from LLVM.

[David Majnemer via Phabricator via cfe-commits]

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

LGTM




Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:23
 struct CrashRecoveryContextImpl;
-
-sys::ThreadLocal () {
-  static sys::ThreadLocal CurrentContext;
-  return CurrentContext;
-}
+LLVM_THREAD_LOCAL static const  CrashRecoveryContextImpl *CurrentContext;
 

nit: Extra space after const


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141347/new/

https://reviews.llvm.org/D141347

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

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:12452
+int Ilogb;
+if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St))
+  return false;

Izaron wrote:
> jcranmer-intel wrote:
> > Izaron wrote:
> > > aaron.ballman wrote:
> > > > jcranmer-intel wrote:
> > > > > `long double` is `ppc_fp128` on at least some PPC targets, and while 
> > > > > I'm not entirely certain of what `ilogb` properly returns in the 
> > > > > corner cases of the `ppc_fp128`, I'm not entirely confident that the 
> > > > > implementation of `APFloat` is correct in those cases. I'd like 
> > > > > someone with PPC background to comment in here.
> > > > Paging @hubert.reinterpretcast for help finding a good person to 
> > > > comment on the PPC questions.
> > > @jcranmer-intel constexpr evaluation is quite 
> > > machine-/target-independent. Clang evaluates it based on its **internal** 
> > > representation of float variables.
> > > 
> > > [[ 
> > > https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/llvm/include/llvm/ADT/APFloat.h#L1256
> > >  | int ilogb ]] uses `Arg.getIEEE()`, that [[ 
> > > https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/llvm/include/llvm/ADT/APFloat.h#L819-L825
> > >  | returns Clang's internal float representation ]].
> > > 
> > > Whichever float semantics is being used, [[ 
> > > https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/llvm/lib/Support/APFloat.cpp#L54-L61
> > >  | minExponent and maxExponent are representable as 
> > > APFloatBase::ExponentType ]], which is `int32_t`:
> > > ```
> > > /// A signed type to represent a floating point numbers unbiased exponent.
> > > typedef int32_t ExponentType;
> > > ```
> > > 
> > > We already use `int ilogb` in some constexpr evaluation code: [[ 
> > > https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/clang/lib/AST/ExprConstant.cpp#L14592
> > >  | link ]], it is working correct because it is working on Clang's float 
> > > representations.
> > > We already use `int ilogb` in some constexpr evaluation code: [[ 
> > > https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/clang/lib/AST/ExprConstant.cpp#L14592
> > >  | link ]], it is working correct because it is working on Clang's float 
> > > representations.
> > 
> > `APFloat::getIEEE()`, if I'm following it correctly, only returns the 
> > details of the high double in `ppc_fp128` floats, and I'm not sufficiently 
> > well-versed in the `ppc_fp128` format to establish whether or not the low 
> > double comes into play here. glibc seems to think that the low double comes 
> > into play in at least one case: 
> > https://github.com/bminor/glibc/blob/30891f35fa7da832b66d80d0807610df361851f3/sysdeps/ieee754/ldbl-128ibm/e_ilogbl.c
> Thanks for the link to the glibc code! It helped me to understand the IEEE754 
> standard better.
> 
> I did some research and it seems like AST supports a fixed set of float 
> types, each working good with `ilogb`:
> ```
> half (__fp16, only for OpenCL), float16, float, double, long double, float128
> ```
> [[ 
> https://github.com/llvm/llvm-project/blob/7846d590033e8d661198f4c00f56f46a4993c526/clang/lib/Sema/SemaExpr.cpp#L3911-L3931
>  | link to SemaExpr.cpp ]]
> 
> It means that the constant evaluator doesn't deal with other float types 
> including `ppc_fp128`.
> If `ppc_fp128` was supported on the AST level, it would anyway come through 
> type casting, and `__builtin_ilog` would deal with a value of a known 
> type.
> 
> I checked the list of builtins - each builtin argument of float type also 
> supports only common float types:
> [[ 
> https://github.com/llvm/llvm-project/blob/7846d590033e8d661198f4c00f56f46a4993c526/clang/include/clang/Basic/Builtins.def#L27-L31
>  | link to Builtins.def 1 ]]
> [[ 
> https://github.com/llvm/llvm-project/blob/7846d590033e8d661198f4c00f56f46a4993c526/clang/include/clang/Basic/Builtins.def#L53-L54
>  | link to Builtins.def 2 ]]
Won't long double map to ppc_fp128 for some targets?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136568/new/

https://reviews.llvm.org/D136568

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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Your example is different from mine as it nests the constexpr variable inside 
the function rather than having it at translation-unit scope.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117569/new/

https://reviews.llvm.org/D117569

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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In D117569#3258307 , @zahiraam wrote:

> In D117569#3257121 , @majnemer 
> wrote:
>
>> I have a question regarding how this work with respect to the dllimport 
>> semantics known by the linker.
>> IIUC, we will now allow a program like:
>>
>>   extern int __declspec(dllimport) dll_import_int;
>>   constexpr int& dll_import_constexpr_ref = dll_import_int;
>>   int& get() {
>>   return dll_import_constexpr_ref;
>>   }
>>
>> Here, `get` will load `dll_import_constexpr_ref`. However, what will 
>> `dll_import_constexpr_ref` hold? It ought to hold the contents of 
>> `__imp_?dll_import_int@@3HA`. However, we can't dereference 
>> `__imp_?dll_import_int@@3HA` to get to its contents.
>
> @majnemer Thanks for the review.
>
> This test case doesn't link with MSVC. It will generate this error:
> Microsoft (R) Incremental Linker Version 14.29.30133.0
> Copyright (C) Microsoft Corporation.  All rights reserved.
>
> /out:test1.exe
> test1.obj
> test1.obj : error LNK2001: unresolved external symbol "int dll_import_int" 
> (?dll_import_int@@3HA)
> test1.exe : fatal error LNK1120: 1 unresolved externals
>
> The symbols generated with MSVC are:
> 07  UNDEF  notype   External | ?dll_import_int@@3HA (int 
> dll_import_int)
> 015  SECT6  notype   Static   | 
> ?dll_import_constexpr_ref@@3AEAHEA (int & dll_import_constexpr_ref)
>
> Without this patch this test case errors. With this patch clang will generate 
> these symbols:
>
> 010  UNDEF  notype   External | __imp_?dll_import_int@@3HA 
> (__declspec(dllimport) int dll_import_int)
> 012  SECT5  notype   External | 
> ?dll_import_constexpr_ref@@3AEAHEA (int & dll_import_constexpr_ref)
> 013  UNDEF  notype   External | ?dll_import_int@@3HA (int 
> dll_import_int)
>
> and will error at link time with this error:
> test1-f1f63b.o : error LNK2019: unresolved external symbol 
> "__declspec(dllimport) int dll_import_int" (__imp_?dll_import_int@@3HA) 
> referenced in function "int & __cdecl get(void)" (?get@@YAAEAHXZ)
> test1-f1f63b.o : error LNK2001: unresolved external symbol "int 
> dll_import_int" (?dll_import_int@@3HA)
> a.exe : fatal error LNK1120: 2 unresolved externals
>
> I think that's the behavior expected, right?

My interpretation is that MSVC has a bug: it is forming a reference to 
`?dll_import_int@@3HA` and not `__imp_?dll_import_int@@3HA`. Could you run a 
more complete experiment where you have a dll which exports the symbol and you 
try to import it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117569/new/

https://reviews.llvm.org/D117569

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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/test/CodeGenCXX/dllimport.cpp:2
 // RUN: %clang_cc1 -disable-noundef-analysis -triple i686-windows-msvc   
-fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o 
- %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M32 %s
-// RUN: %clang_cc1 -disable-noundef-analysis -triple x86_64-windows-msvc 
-fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o 
- %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M64 %s
+// RUN: %clang_cc1 -disable-noundef-analysis -triple x86_64-windows-msvc 
-fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o 
- %s -DMSABI -w | FileCheck --check-prefix=MSC64 --check-prefix=M64 %s
 // RUN: %clang_cc1 -disable-noundef-analysis -triple i686-windows-gnu
-fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o 
- %s -w | FileCheck --check-prefix=GNU --check-prefix=G32 %s

Why did this check-prefix change to `MSC64`? There is already a unique prefix 
for this `RUN` line: `M64`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117569/new/

https://reviews.llvm.org/D117569

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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2219
+  Info.getLangOpts().CPlusPlus && !isForManglingOnly(Kind) &&
+  Var->hasAttr())
 // FIXME: Diagnostic!

The summary and the bug both mention dllimport but this is examining dllexport. 
Is this intended? If so, please update the comment above this if-statement.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117569/new/

https://reviews.llvm.org/D117569

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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[David Majnemer via Phabricator via cfe-commits]

majnemer requested changes to this revision.
majnemer added a comment.
This revision now requires changes to proceed.

I have a question regarding how this work with respect to the dllimport 
semantics known by the linker.
IIUC, we will now allow a program like:

  extern int __declspec(dllimport) dll_import_int;
  constexpr int& dll_import_constexpr_ref = dll_import_int;
  int& get() {
  return dll_import_constexpr_ref;
  }

Here, `get` will load `dll_import_constexpr_ref`. However, what will 
`dll_import_constexpr_ref` hold? It ought to hold the contents of 
`__imp_?dll_import_int@@3HA`. However, we can't dereference 
`__imp_?dll_import_int@@3HA` to get to its contents.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117569/new/

https://reviews.llvm.org/D117569

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

[PATCH] D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG072e2a7c67b7: [MS] Implement on-demand TLS initialization 
for Microsoft CXX ABI (authored by momo5502, committed by majnemer).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115456/new/

https://reviews.llvm.org/D115456

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/ms-thread_local.cpp

Index: clang/test/CodeGenCXX/ms-thread_local.cpp
===
--- clang/test/CodeGenCXX/ms-thread_local.cpp
+++ clang/test/CodeGenCXX/ms-thread_local.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 %s  -std=c++1y -triple=i686-pc-win32 -ftls-model=local-dynamic -emit-llvm -o - | FileCheck %s -check-prefix=CHECK-LD
+// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -fms-compatibility-version=19.25 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -fms-compatibility-version=19.20 -emit-llvm -o - | FileCheck %s -check-prefix=CHECK-LEGACY
+// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -fms-compatibility-version=19.25 -ftls-model=local-dynamic -emit-llvm -o - | FileCheck %s -check-prefix=CHECK-LD
 
 struct A {
   A();
@@ -17,14 +18,22 @@
 
 // CHECK-DAG: @"?b@@3UA@@A" = dso_local thread_local global %struct.A zeroinitializer, align 1
 // CHECK-DAG: @"__tls_init$initializer$" = internal constant void ()* @__tls_init, section ".CRT$XDU"
+// CHECK-DAG: @__tls_guard = external dso_local thread_local global i8
 // CHECK-LD-DAG: @"?b@@3UA@@A" = dso_local thread_local(localdynamic) global %struct.A zeroinitializer, align 1
 // CHECK-LD-DAG: @"__tls_init$initializer$" = internal constant void ()* @__tls_init, section ".CRT$XDU"
+// CHECK-LD-DAG: @__tls_guard = external dso_local thread_local global i8
+// CHECK-LEGACY-NOT: @__tls_guard = external dso_local thread_local global i8
 thread_local A b;
 
-// CHECK-LABEL: define internal void @__tls_init()
-// CHECK: call void @"??__Eb@@YAXXZ"
-// CHECK-LD-LABEL: define internal void @__tls_init()
-// CHECK-LD: call void @"??__Eb@@YAXXZ"
+// CHECK-LABEL: declare dso_local void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: declare dso_local void @__dyn_tls_on_demand_init()
+// CHECK-LEGACY-NOT: declare dso_local void @__dyn_tls_on_demand_init()
+
+// CHECK-LABEL: define dso_local void @"?f@@YA?AUA@@XZ"(%struct.A* noalias sret(%struct.A) align 1 %agg.result)
+// CHECK: call void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: define dso_local void @"?f@@YA?AUA@@XZ"(%struct.A* noalias sret(%struct.A) align 1 %agg.result)
+// CHECK-LD: call void @__dyn_tls_on_demand_init()
+// CHECK-LEGACY-NOT: call void @__dyn_tls_on_demand_init()
 
 thread_local A  = b;
 thread_local A  = c;
@@ -35,6 +44,22 @@
   return c;
 }
 
+// CHECK-LABEL: define dso_local i32 @"?g@@YAHXZ"()
+// CHECK-NOT: call void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: define dso_local i32 @"?g@@YAHXZ"()
+// CHECK-LD-NOT: call void @__dyn_tls_on_demand_init()
+
+thread_local int e = 2;
+
+int g() {
+  return e;
+}
+
+// CHECK-LABEL: define internal void @__tls_init()
+// CHECK: call void @"??__Eb@@YAXXZ"
+// CHECK-LD-LABEL: define internal void @__tls_init()
+// CHECK-LD: call void @"??__Eb@@YAXXZ"
+
 // CHECK: !llvm.linker.options = !{![[dyn_tls_init:[0-9]+]]}
 // CHECK: ![[dyn_tls_init]] = !{!"/include:___dyn_tls_init@12"}
 // CHECK-LD: !llvm.linker.options = !{![[dyn_tls_init:[0-9]+]]}
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -401,7 +401,9 @@
   ArrayRef CXXThreadLocalInitVars) override;
 
   bool usesThreadWrapperFunction(const VarDecl *VD) const override {
-return false;
+return getContext().getLangOpts().isCompatibleWithMSVC(
+   LangOptions::MSVC2019_5) &&
+   (!isEmittedWithConstantInitializer(VD) || mayNeedDestruction(VD));
   }
   LValue EmitThreadLocalVarDeclLValue(CodeGenFunction , const VarDecl *VD,
   QualType LValType) override;
@@ -2397,11 +2399,97 @@
   }
 }
 
+static llvm::GlobalValue *getTlsGuardVar(CodeGenModule ) {
+  // __tls_guard comes from the MSVC runtime and reflects
+  // whether TLS has been initialized for a particular thread.
+  // It is set from within __dyn_tls_init by the runtime.
+  // Every library and executable has its own variable.
+  llvm::Type *VTy = llvm::Type::getInt8Ty(CGM.getLLVMContext());
+  llvm::Constant *TlsGuardConstant =
+  CGM.CreateRuntimeVariable(VTy, "__tls_guard");
+  llvm::GlobalValue *TlsGuard = 

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

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

Looks great! Please give others some time to review it as it is a holiday 
season...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115456/new/

https://reviews.llvm.org/D115456

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

[PATCH] D116020: [clang][#52782] Bail on incomplete parameter type in stdcall name mangling

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

I wonder if we should have different behavior for MSVC targets.

If I do:

  class Incomplete;
  extern "C" int __stdcall Fn(int, Incomplete, long long);
  auto fnptr = 

MSVC generates:

  EXTRN   _Fn@12:PROC

It appears that they skip over incomplete types.

Should the behavior for incomplete types depend on the target?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116020/new/

https://reviews.llvm.org/D116020

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In D114425#3217478 , @Quuxplusone 
wrote:

> In D114425#3216802 , @majnemer 
> wrote:
>
>> OOC, how hard would it be to generalize this builtin a little? It is nice 
>> that we have builtins like `__builtin_add_overflow` which do the right thing 
>> regardless of their input.
>>
>> It seems like it would be nice if we started to expose more intrinsics which 
>> did the right thing regardless of operand width; another bonus is that it 
>> composes well with language features like `_BitInt`.
>
> IMHO such builtins are nice iff the programmer can be 100% sure that the 
> compiler will interpret them the same way as a human reader. 
> `__builtin_add_overflow` is easy because its first two arguments are 
> "mathematical integers" (where integer promotion doesn't matter) and its 
> third argument is a pointer (where integer promotion can't happen). So you 
> can really throw any combination of types at it, and it'll do "the right 
> thing" https://godbolt.org/z/sa7b894oa (although I admit I was surprised that 
> this worked).
> For a hypothetical `__builtin_bswap`, you would probably need a similar 
> pointer-based interface like
>
>   short s16 = 0xFEDC;
>   __builtin_bswap();  // hypothetically
>   assert(s16 == 0xDCFE);
>   
>   assert(__builtin_bswap16(s16) == 0xDCFE);
>   assert(__builtin_bswap32(s16) == 0xDCFE);  // the problem to solve: s16 
> eagerly promotes to int, which changes the result
>
> The downside is that the pointer-based interface is less ergonomic than 
> today's value-based signatures, and probably worse codegen at `-O0` (because 
> the programmer has to materialize the operand into a named variable, and then 
> the compiler won't remove that variable because you might want to debug it). 
> The upside (as you said) is that a generic builtin could work with `_ExtInt` 
> types and so on.

I think one way of side stepping that problem would be to say that 
`__builtin_bswap` only works with unsigned types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114425/new/

https://reviews.llvm.org/D114425

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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In D115456#3217595 , @momo5502 wrote:

> In D115456#3216811 , @majnemer 
> wrote:
>
>> This is looking great! Just a few more questions.
>>
>> What is the behavior with something like:
>>
>>   thread_local int x = 2;
>>   int f() {
>> return x;
>>   }
>>
>> I'm wondering if we need to move this logic 
>> 
>>  into the generic C++ ABI implementation.
>
> The MS compiler only emits the dynamic initializers for variables with 
> constructors/destructors, just like it is currently done here for the Itanium 
> ABI.
> I also thought about adopting that behaviour, but I think threre are 
> edge-cases when triggering dynamic TLS initialization even for constant 
> variables is useful.
> For example there might be custom TLS callbacks that can affect the value of 
> this variable.
>
> If desired, I can change it to match the behaviour of MS, but I thought it 
> could be beneficial to diverge in this case.

IMO, it is probably best to match behavior here within reason (ignoring bugs 
latent in MSVC) here. My thinking is that in the face of COMDATs, we cannot 
ensure which copy of an inline function will make its way to the binary and 
different link orders would provide different behavior.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115456/new/

https://reviews.llvm.org/D115456

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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

This is looking great! Just a few more questions.

What is the behavior with something like:

  thread_local int x = 2;
  int f() {
return x;
  }

I'm wondering if we need to move this logic 

 into the generic C++ ABI implementation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115456/new/

https://reviews.llvm.org/D115456

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

OOC, how hard would it be to generalize this builtin a little? It is nice that 
we have builtins like `__builtin_add_overflow` which do the right thing 
regardless of their input.

It seems like it would be nice if we started to expose more intrinsics which 
did the right thing regardless of operand width; another bonus is that it 
composes well with language features like `_BitInt`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114425/new/

https://reviews.llvm.org/D114425

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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:404
   bool usesThreadWrapperFunction(const VarDecl *VD) const override {
-return false;
+return true;
   }

Should this depend on the MSVC compatibility version?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115456/new/

https://reviews.llvm.org/D115456

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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Is this a new feature in MSVC? Seems like it might 

 be. If so, should it be predicated on `isCompatibleWithMSVC(1925)` or 
something?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115456/new/

https://reviews.llvm.org/D115456

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

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2782
+
+// construct the vector of 'unsigned char' type
+QualType CharVecTy = Ctx.getVectorType(Ctx.CharTy, NumVectorBytes,

The code as written seems to be 'char' type, not 'unsigned char' type.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2084-2085
 Dst.isVolatileQualified());
+  auto *IRStoreTy = dyn_cast(Vec->getType());
+  if (IRStoreTy) {
+auto *IRVecTy = llvm::FixedVectorType::get(

`if (auto *IRStoreTy = dyn_cast(Vec->getType()))`



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:97
+  if (VT && VT->isExtVectorBoolean()) {
+auto *FixedVT = dyn_cast(R);
+// Pad to at least one byte.

`dyn_cast` -> `cast`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88905/new/

https://reviews.llvm.org/D88905

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

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4121-4125
-  if (isa(TrueVal))   // select ?, undef, X -> X
-return FalseVal;
-  if (isa(FalseVal))   // select ?, X, undef -> X
-return TrueVal;
-

Can we still do these optimizations when `X` is a frozen value?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83360/new/

https://reviews.llvm.org/D83360



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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:1783
+
+  bool IsSideEntry() const { return SideEntry; }
+  void setSideEntry() { SideEntry = true; }

I think this should be isSideEntry to be consistent.



Comment at: clang/lib/CodeGen/CGException.cpp:603-609
+  // For IsEHa catch(...) must handle HW exception
+  // Adjective = HT_IsStdDotDot (0x40), only catch C++ exceptions
+  // Also mark scope with SehTryBegin
+  if (getLangOpts().EHAsynch) {
+TypeInfo.Flags = 0;
+EmitRuntimeCallOrInvoke(getSehTryBeginFn(CGM));
+  }

I think this logic should move into MicrosoftCXXABI::getCatchAllTypeInfo.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80344/new/

https://reviews.llvm.org/D80344



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

[PATCH] D76077: [ARM] Add __bf16 as new Bfloat16 C Type

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/include/clang-c/Index.h:3254
   CXType_FirstBuiltin = CXType_Void,
   CXType_LastBuiltin = CXType_ULongAccum,
 

Should this be:
  CXType_LastBuiltin = CXType_BFloat16,




Comment at: clang/lib/AST/ItaniumMangle.cpp:3186
+case BuiltinType::Half:  EltName = "float16_t"; break;
+case BuiltinType::BFloat16:  EltName = "bfloat16x1_t"; break;
 default:

Why is this x1?



Comment at: clang/lib/Sema/SemaOverload.cpp:1873-1874
 // We of course allow this conversion if long double is really double.
+if (FromType == S.Context.BFloat16Ty || ToType == S.Context.BFloat16Ty)
+  return false;
 if ((FromType) !=

This probably needs an explanation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76077/new/

https://reviews.llvm.org/D76077



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

[PATCH] D77777: [nvptx] Add `nvvm.texsurf.handle` internalizer.

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In D7#1975618 , @hliao wrote:

> In D7#1975406 , @tra wrote:
>
> > In D7#1975178 , @hliao wrote:
> >
> > > the 1st argument in `llvm.nvvm.texsurf.hande.internal` or the 2nd one in 
> > > `llvm.nvvm.texsurf.handle` must be kept as an immediate or constant 
> > > value, i.e. that global variable. However, optimizations will find common 
> > > code in the following
> > >
> > >   if (cond) {
> > > %hnd = texsurf.handle.internal(@tex1);
> > >   } else {
> > > %hnd = texsurf.handle.internal(@tex2)
> > >   }
> > >   = use(%hnd)
> > >
> > >
> > > and hoist or sink it into
> > >
> > >   if (cond) {
> > > %ptr = @tex1;
> > >   } else {
> > > %ptr = @tex2;
> > >   }
> > >   %hnd = texsurf.handle.intenal(%ptr);
> > >   = use(%hnd)
> > >
> > >
> > > The backend cannot handle non immediate operand in `texsurf.handle`. The 
> > > similar thing happens to `read.register` as well as it also assumes its 
> > > argument is always an immediate value.
> >
> >
> > I wonder if we can use `token` types to represent the handle? 
> > https://reviews.llvm.org/D11861
> >  @majnemer -- would this use case be suitable for the `token` type?
>
>
> If we still could make PHI over token, it canont serve this purpose. Check 
> `llvm::canReplaceOperandWithVariable` for operand for details.


It is not possible to PHI a token value. Token values disable the call to 
canReplaceOperandWithVariable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D7/new/

https://reviews.llvm.org/D7



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

[PATCH] D66827: Add support for MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:1718
  QualType PointeeType) 
{
-  if (PointersAre64Bit &&
-  (PointeeType.isNull() || !PointeeType->isFunctionType()))
+  // Check if this is a defulat 64-bit pointer or has __ptr64 qualifier.
+  bool is64Bit = PointeeType.isNull() ? PointersAre64Bit :

defulat -> default


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66827/new/

https://reviews.llvm.org/D66827



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

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:12
+// Test the conversion function that truncates floating point types to the
+// closes representable value for the specified integer type, or
+// numeric_limits::max()/min() if the value isn't representable.

closes -> closest


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66836/new/

https://reviews.llvm.org/D66836



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

[PATCH] D56466: [CodeGen] Clarify comment about COFF common symbol alignment

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:3765-3766
+
+  // MSVC doesn't support alignments greater than 32 for common symbols, so
+  // symbols with greater alignment requirements cannot be common. Non-MSVC 
COFF
+  // environments support arbitrary power-of-two alignments for common symbols

I think it is most clear if this comment said "Microsoft's link.exe" instead of 
MSVC.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56466/new/

https://reviews.llvm.org/D56466



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

[PATCH] D56391: Limit COFF 'common' emission to <=32 alignment types.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:3766
+  // in common.
+  if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF() &&
+  Context.getTypeAlignIfKnown(D->getType()) > 32)

I think this should be isKnownWindowsMSVCEnvironment


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56391/new/

https://reviews.llvm.org/D56391



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

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2722-2725
+if (ConstantExpr *CE = dyn_cast(E)) {
+  E = CE->getSubExpr();
+  continue;
+}

Just curious, why not go even further and use FullExpr rather than ConstantExpr?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55853/new/

https://reviews.llvm.org/D55853



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

[PATCH] D55715: Add AddressSpace mangling to MS mode

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:1806-1836
+Extra.mangleSourceName("AS_");
+Extra.mangleIntegerLiteral(llvm::APSInt::getUnsigned(TargetAS),
+   /*IsBoolean*/ false);
+  } else {
+switch (AS) {
+default:
+  llvm_unreachable("Not a language specific address space");

Don't these need to be _AS_Foobar to avoid collisions with normal code?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55715/new/

https://reviews.llvm.org/D55715



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

[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/DeclBase.cpp:1711-1712
+  // contexts are always skipped.
+  while (SkipRecords ? Ctx->isTransparentContext() || Ctx->isRecord()
+ : Ctx->isTransparentContext())
 Ctx = Ctx->getParent();

Seems simpler as:
  while ((SkipRecords && Ctx->isRecord()) || Ctx->isTransparentContext())


https://reviews.llvm.org/D52384



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

[PATCH] D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074)

[David Majnemer via Phabricator via cfe-commits]

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

FWIW, Microsoft's newest documentation does not say that `/O2` and `/O1` imply 
`/Gs`: 
https://docs.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed?view=vs-2017


https://reviews.llvm.org/D52499



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

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

How does MSVC handle this case? What mangled name does it generate?


https://reviews.llvm.org/D50877



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

[PATCH] D49354: [MinGW] Automatically mangle Windows-specific entry points as C

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D49354



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

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D47875



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

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:888-891
 auto EnumeratorI = ED->enumerator_begin();
-assert(EnumeratorI != ED->enumerator_end());
-Name += "getName();
+if (EnumeratorI == ED->enumerator_end()) {
+  Name += " and .


https://reviews.llvm.org/D47875



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

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CGException.cpp:1173
+cast(CatchStartBlock->getFirstNonPHI());
+CurrentFuncletPad = CPI;
+  }

aheejin wrote:
> majnemer wrote:
> > Hmm, why is this done? Won't RestoreCurrentFuncletPad undo this?
> Isn't `RestoreCurrentFuncletPad` outside of `if 
> (EHPersonality::get(*this).isWasmPersonality())`? Isn't this supposed to stay 
> until this function finishes?
Ah, true! Nevermind!



Comment at: lib/CodeGen/CGException.cpp:1241-1245
+while (llvm::TerminatorInst *TI = RethrowBlock->getTerminator()) {
+  llvm::BranchInst *BI = cast(TI);
+  assert(BI->isConditional());
+  RethrowBlock = BI->getSuccessor(1);
+}

aheejin wrote:
> aheejin wrote:
> > majnemer wrote:
> > > This seems pretty fragile, why is this guaranteed to work? Could we 
> > > maintain a map from CatchSwitchInst to catch-all block?
> > The function call sequence here is `CodeGenFunction::ExitCXXTryStmt` -> 
> > `emitCatchDispatchBlock` (static) -> `emitWasmCatchDispatchBlock` (static) 
> > and `emitCatchDispatchBlock` also has other callers, so it is a little 
> > cumbersome to pass a map to those functions to be filled in. (We have to 
> > make a parameter that's only gonna be used for wasm to both 
> > `emitCatchDispatchBlock` and `emitWasmCatchDispatchBlock`)
> > 
> > The other way is also change those static `emit` functions into 
> > `CodeGenFunction` class's member functions and make the map as a member 
> > variable.
> > 
> > But first, in which case do you think this will be fragile? 
> > `emitWasmCatchDispatchBlock` follows the structure of the landingpad model, 
> > so for a C++ code like this
> > ```
> > try {
> >   ...
> > } catch (int) {
> >   ...
> > } catch (float) {
> >   ...
> > }
> > ```
> > the BB structure that starts from wasm's `catch.start` block will look like
> > ```
> > catch.dispatch:   ; preds = %entry
> >   %0 = catchswitch within none [label %catch.start] unwind to caller
> > 
> > catch.start:  ; preds = %catch.dispatch
> >   %1 = catchpad within %0 [i8* bitcast (i8** @_ZTIi to i8*), i8* bitcast 
> > (i8** @_ZTIf to i8*)]
> >   %2 = call i8* @llvm.wasm.get.exception()
> >   %3 = call i32 @llvm.wasm.get.ehselector()
> >   %4 = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIi to i8*)) #2
> >   %matches = icmp eq i32 %3, %4
> >   br i1 %matches, label %catch12, label %catch.fallthrough
> > 
> > catch12:  ; preds = %catch.start
> >   body of catch (int)
> > 
> > catch.fallthrough:; preds = %catch.start
> >   %8 = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIf to i8*)) #2
> >   %matches1 = icmp eq i32 %3, %8
> >   br i1 %matches1, label %catch, label %rethrow
> > 
> > catch:; preds = 
> > %catch.fallthrough
> >   body of catch (float)
> > 
> > rethrow:  ; preds = 
> > %catch.fallthrough
> >   call void @__cxa_rethrow() #5 [ "funclet"(token %1) ]
> >   unreachable
> > ```
> > 
> > So to me it looks like, no matter how the bodies of `catch (int)` or `catch 
> > (float)` are complicated, there should always be blocks like `catch.start` 
> > and `catch.fallthrough`, which compares typeids and divide control flow 
> > depending on the typeid comparison. I could very well be mistaken, so 
> > please let me know if so.
> Oh and the `RethrowBlock` in the code is not the same as the `catch_all` 
> block... cleanuppads will be `catch_all` blocks in wasm, and catchpads will 
> be `catch `. That `RethrowBlock` belongs to `catch ` block, and is 
> entered when the current exception caught is a C++ exception but does not 
> match any of the catch clauses, so it can be rethrown to the enclosing scope.
I guess I'm worried that we could have emitted statements inside the catch(int) 
and catch(float) blocks and we'd either run into a terminator which isn't a 
BranchInst.
If we could not emit any statements yet, then I think this is OK...


Repository:
  rC Clang

https://reviews.llvm.org/D44931



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

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CGException.cpp:1164
+  CurrentFuncletPad);
+  llvm::BasicBlock *CatchStartBlock = nullptr;
+  if (EHPersonality::get(*this).isWasmPersonality()) {

Maybe this should be called WasmCatchStartBlock?



Comment at: lib/CodeGen/CGException.cpp:1173
+cast(CatchStartBlock->getFirstNonPHI());
+CurrentFuncletPad = CPI;
+  }

Hmm, why is this done? Won't RestoreCurrentFuncletPad undo this?



Comment at: lib/CodeGen/CGException.cpp:1241-1245
+while (llvm::TerminatorInst *TI = RethrowBlock->getTerminator()) {
+  llvm::BranchInst *BI = cast(TI);
+  assert(BI->isConditional());
+  RethrowBlock = BI->getSuccessor(1);
+}

This seems pretty fragile, why is this guaranteed to work? Could we maintain a 
map from CatchSwitchInst to catch-all block?


Repository:
  rC Clang

https://reviews.llvm.org/D44931



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

[PATCH] D45839: [analyzer] Add support for WebKit "unified sources".

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:145-147
+  if (Name.endswith_lower(".c") || Name.endswith_lower(".cpp") ||
+  Name.endswith_lower(".cc") || Name.endswith_lower(".cxx") ||
+  Name.endswith_lower(".m") || Name.endswith_lower(".mm")) {

C++ source code is also found in files which end in .C, this code will match 
against strange file endings like .cXx and .mM

I think the above logic should be changed to match 
https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/FrontendOptions.cpp#L27


https://reviews.llvm.org/D45839



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

[PATCH] D45738: Add Microsoft mangling for _Float16, similar to technique used for _Complex

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D45738



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

[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In https://reviews.llvm.org/D45174#1055125, @rsmith wrote:

> In https://reviews.llvm.org/D45174#1055048, @rsmith wrote:
>
> > I wonder if we can delete the `getNonVirtualSize()` check now -- I don't 
> > see any way that an empty class can have a nonzero nvsize except by this 
> > nonempty anonymous bit-fields situation.
>
>
> Yup, looks like:
>
>  if (!FoundBase) {
>   -if (MDCUsesEBO && BaseDecl->isEmpty() &&
>   -BaseLayout.getNonVirtualSize() == CharUnits::Zero()) {
>   +if (MDCUsesEBO && BaseDecl->isEmpty()) {
>   +  assert(BaseLayout.getNonVirtualSize() == CharUnits::Zero());
>  BaseOffset = CharUnits::Zero();
>} else {
>
>
> Zero test failures.


Yeah, I think this should be NFC for the MS ABI.




Comment at: ReleaseNotes.rst:68-72
+- Clang implements the proposed resolution of LWG issue 2358, along with the
+  `corresponding change to the Itanium C++ ABI
+  `_, which make classes
+  containing only unnamed non-zero-length bit-fields be considered non-empty.
+  This is an ABI break compared to prior Clang releases, but makes Clang

The "Clang" in the above paragraph has a lowercase "clang". I think we should 
choose a consistent capitalization.


Repository:
  rC Clang

https://reviews.llvm.org/D45174



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

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Does this help PR25641?


https://reviews.llvm.org/D45112



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

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Some quick first pass comments.




Comment at: lib/CodeGen/CGCleanup.cpp:985
+// does not have a runtime support for that.
+if (!Personality.usesFuncletPads() || Personality.isWasmPersonality()) {
+  EHStack.pushTerminate();

I think this condition can be simplified to `!isMSVCPersonality()` with a 
slight tweak of the comment.



Comment at: test/CodeGenCXX/wasm-eh.cpp:33
+// CHECK-NEXT:   %[[CATCHPAD:.*]] = catchpad within %[[CATCHSWITCH]] [i8* 
bitcast (i8** @_ZTIi to i8*), i8* bitcast (i8** @_ZTId to i8*)]
+// CHECK-NEXT:   %[[EXN:.*]] = call i8* @llvm.wasm.get.exception()
+// CHECK-NEXT:   store i8* %[[EXN]], i8** %exn.slot

I'd expect a funclet bundle operand here..


Repository:
  rC Clang

https://reviews.llvm.org/D44931



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

[PATCH] D44327: ObjCARC: teach the cloner about funclets

[David Majnemer via Phabricator via cfe-commits]

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

LGTM




Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:692
+   DenseMap ) {
+  auto *CI = dyn_cast();
+  assert(CI && "CloneCallInst must receive a CallInst");

dyn_cast -> cast

Actually, why not just take a CallInst &?



Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:698
+auto Bundle = CI->getOperandBundleAt(I);
+// funclets will be reassociated in the future
+if (Bundle.getTagID() == LLVMContext::OB_funclet)

Start the comment with an uppercase and end with a period.


Repository:
  rL LLVM

https://reviews.llvm.org/D44327



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

[PATCH] D44327: ObjCARC: teach the cloner about funclets

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:701
+
+  if (auto *CleanupPad = dyn_cast(BB.getFirstNonPHI()))
+OpBundles.emplace_back("funclet", CleanupPad);

What if the cleanuppad was introduced in a block which branched to this one?


Repository:
  rC Clang

https://reviews.llvm.org/D44327



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

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In https://reviews.llvm.org/D43576#1016512, @rsmith wrote:

> Do we need to also track whether the argument is a pointer or reference to a 
> UUID (and also the cv-qualifiers)? For the `Declaration` case, we track this 
> by tracking the corresponding parameter type; the same thing would presumably 
> work here.
>
> In https://reviews.llvm.org/D43576#1016295, @majnemer wrote:
>
> > We should really, really avoid making this sort of change without first 
> > trying to desugar uuidof into a reference to a variable. That would solve a 
> > ton of problems, problems like this one.
>
>
> This desugaring approach is not how we generally do things in Clang. The fact 
> that MS exposes a variable that can be named from user code is, in my 
> opinion, simply a bug in their implementation -- their implementation details 
> are leaking -- and not part of the actual semantics here. I view this as 
> exactly analogous to `typeid` (which would have exactly the same problems if 
> its result could be used as a non-type template parameter); as with `typeid`, 
> `__uuidof` notionally produces a global object not corresponding to any 
> variable. If we want to model this as a declaration, we could add a new 
> `Decl` subclass for these uuid objects (and eventually also for objects 
> produced by `typeid`). But I don't think we should model them as variables 
> unless that's actually part of their intended semantics.


Here's my thinking: the `__uuidof` expression literally declares a variable 
called `_GUID_ddb47a6a_0f23_11d5_9109_00e0296b75d3` of type `__s_GUID` which is 
why it behaves the way it does: https://godbolt.org/g/74FY7U

I don't think it is reasonable to invent new semantics which are different from 
the MSVC ones because we find the MSVC ones inelegant. The huge upside I see 
from matching their behavior is that the implementation is dramatically 
simpler, we have a considerable amount of mumbo jumbo in the template 
instantiation code to handle `__uuidof` correctly and it looks like we will 
need some more.

What is the relative upside to a new kind of Decl? Better AST fidelity?


https://reviews.llvm.org/D43576



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

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In https://reviews.llvm.org/D43576#1016418, @zahiraam wrote:

> In https://reviews.llvm.org/D43576#1016295, @majnemer wrote:
>
> > We should really, really avoid making this sort of change without first 
> > trying to desugar uuidof into a reference to a variable. That would solve a 
> > ton of problems, problems like this one.
>
>
> Not sure I fully understand what you are proposing?
>
> Are you proposing that generated symbols like this:
>  
> ??4?$A@$E?_GUID_ddb47a6a_0f23_11d5_9109_00e0296b75d3@@3U__s_GUID@@B@@QEAAAEAV0@AEBV0@@Z
>  
>  Be de-sugared? Wouldn't that be different that what MS is doing? 
>  Can you please give me more details about what you are thinking of?
>  Thanks.


We create an AST such that the following:

  struct _GUID;
  
  template 
  class A {};
  
  struct __declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}")) S {};
  
  struct __declspec(dllexport) C : public A<&__uuidof(S)> {};

is turned into something like:

  struct _GUID;
  
  __declspec(selectany) __s_GUID const 
_GUID_ddb47a6a_0f23_11d5_9109_00e0296b75d3 = {...};
  
  template 
  class A {};
  
  struct __declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}")) S {};
  
  struct __declspec(dllexport) C : public 
A<&_GUID_ddb47a6a_0f23_11d5_9109_00e0296b75d3> {};


https://reviews.llvm.org/D43576



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

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

We should really, really avoid making this sort of change without first trying 
to desugar uuidof into a reference to a variable. That would solve a ton of 
problems, problems like this one.


https://reviews.llvm.org/D43576



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

[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Do I understand correctly that this workarounds a feature missing in lld? Does 
MinGW emit the same sorts of object files as clang in these scenarios?


Repository:
  rC Clang

https://reviews.llvm.org/D43184



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

[PATCH] D43033: [WinEH] Put funclet bundles on inline asm calls

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D43033



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

[PATCH] D42758: Support `#pragma comment(lib, "name")` in the frontend for ELF

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

docs/LanguageExtensions.html should be updated to mention that we support this 
extension on all ELF targets.


Repository:
  rC Clang

https://reviews.llvm.org/D42758



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

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CGVTables.cpp:887-888
 
+  if (CGM.getTriple().isWindowsGNUEnvironment() && 
RD->hasAttr())
+return true;
+

Maybe a comment like "VTables of classes declared as dllimport are always 
considered to be external." ?



Comment at: lib/CodeGen/CGVTables.cpp:896-900
   // Otherwise, if the class is an instantiated template, the
   // vtable must be defined here.
   if (TSK == TSK_ImplicitInstantiation ||
   TSK == TSK_ExplicitInstantiationDefinition)
 return false;

It would be good to have tests for what would have happened if these paths got 
hit.


https://reviews.llvm.org/D42641



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

[PATCH] D42508: AST: support protocol conformances on id/class/interfaces in MS ABI

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D42508



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

[PATCH] D42248: Always allow "#pragma region".

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D42248



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

[PATCH] D42508: AST: support protocol conformances on id/class/interfaces in MS ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:2459-2465
+  if (T->isObjCId())
+mangleSourceName("objc_object");
+  else if (T->isObjCClass())
+mangleSourceName("objc_class");
+  else
+mangleSourceName(T->getInterface()->getName());
+

Hmm, this is a template... I don't think this will do the right thing wrt 
backreferences.



Comment at: lib/AST/MicrosoftMangle.cpp:2468
+Out << 'Y';
+mangleSourceName(Q->getName());
+Out << '@';

Probably want a comment stating that this is cointerface.


Repository:
  rC Clang

https://reviews.llvm.org/D42508



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

[PATCH] D42343: [coroutines] Fix application of NRVO to Coroutine "Gro" or return object.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: test/CodeGenCoroutines/coro-gro-nrvo.cpp:17
+using SizeT = decltype(sizeof(int));
+void* operator new(SizeT __sz, const std::nothrow_t&) noexcept;
+void  operator delete(void* __p, const std::nothrow_t&) noexcept;

`SizeT` -> `__SIZE_TYPE__` ?


https://reviews.llvm.org/D42343



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

[PATCH] D42248: [LangOpts] Add a LangOpt to represent "#pragma region" support.

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Why not always support the pragma regardless of the compiler mode? Our 
"support" for it just ignores it anyway...


https://reviews.llvm.org/D42248



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

[PATCH] D41032: [CodeGen][X86] Implement _InterlockedCompareExchange128 intrinsic

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: llvm/tools/clang/lib/CodeGen/CGBuiltin.cpp:8439
+
+llvm::Type *Int128Ty = llvm::IntegerType::get(getLLVMContext(), 128);
+llvm::Type *Int128PtrTy = Int128Ty->getPointerTo();

Builder.getInt128Ty()


https://reviews.llvm.org/D41032



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

[PATCH] D40071: [libcxx] Implement exception_ptr on Windows without msvcprt.dll

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: src/support/runtime/exception_pointer_msvc.ipp:119-123
+#ifdef _M_AMD64
+RtlPcToFileHeader(
+reinterpret_cast(const_cast(throw_info)),
+_base);
+#endif

Can't you use the image_base field in throw_info?


https://reviews.llvm.org/D40071



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In https://reviews.llvm.org/D40023#933466, @asb wrote:

> In https://reviews.llvm.org/D40023#933464, @majnemer wrote:
>
> > So how does something like the following work:
> >
> >   union U { float f; int i; };
> >   void f(union U u);
> >   
> >
> > The flattening described in 
> > https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#hardware-floating-point-calling-convention
> >  makes sense for arrays and structs but I couldn't find where unions were 
> > described.
>
>
> You're right that's not currently described - I have an issue tracking this 
> here: https://github.com/riscv/riscv-elf-psabi-doc/issues/24. Last time I 
> tried to check gcc behaviour it seemed that such cases would be passed 
> according to the integer calling convention, but I'd be happier if one of the 
> GCC devs would confirm.


Should we have a test which tries to make sure our behavior matches GCC's? ISTM 
that floats in unions are always in GPRs (at least according to abicop).

How are empty unions/arrays handled? Like a function which takes no args? 
abicop seemed upset when I asked it:

  >>> m.call([Union()])
  Traceback (most recent call last):
File "", line 1, in 
File "~/majnemer/abicop/rvcc.py", line 186, in __init__
  self.alignment = max(m.alignment for m in members)
  ValueError: max() arg is an empty sequence

It'd also be good to have a test for bool and tests for bitfields. Am I correct 
that struct S { int x : 2; int y : 2; }; would get passed in two GPRs which 
were sign extended from 2 to 32?


https://reviews.llvm.org/D40023



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

So how does something like the following work:

  union U { float f; int i; };
  void f(union U u);

The flattening described in 
https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#hardware-floating-point-calling-convention
 makes sense for arrays and structs but I couldn't find where unions were 
described.


https://reviews.llvm.org/D40023



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

[PATCH] D40218: [Clang] Add __builtin_launder

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

A test with restrict and __restrict might be interesting.


https://reviews.llvm.org/D40218



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

It'd be good to have tests which have alignment directives on bitfields.


https://reviews.llvm.org/D39347



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

[PATCH] D38704: [libunwind] Abstract rwlocks into a class, provide a SRW lock implementation for windows

[David Majnemer via Phabricator via cfe-commits]

majnemer accepted this revision.
majnemer added a comment.

LGTM


https://reviews.llvm.org/D38704



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

[PATCH] D38704: [libunwind] Emulate pthread rwlocks via SRW locks for windows

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

I don't think we should depend on LLVM for the locking stuff. This sort of 
infrastructure is in the same bucket as the demangler which we haven't really 
solved.

I *do* find it weird to do it this way though. I think you should have some 
mutex functions which include read/write unlock. This way you don't need the 
weird state.


https://reviews.llvm.org/D38704



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

[PATCH] D38656: [CGExprScalar] In EmitCompare trunc the result if it has different type as E->getType()

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:3223-3224
+  // crash later.
+  llvm::IntegerType *ResultTy =
+ dyn_cast(Result->getType());
+  if ((ResultTy->getBitWidth() > 1) &&

Is this clang-format'd ?



Comment at: lib/CodeGen/CGExprScalar.cpp:3224
+  llvm::IntegerType *ResultTy =
+ dyn_cast(Result->getType());
+  if ((ResultTy->getBitWidth() > 1) &&

You are unconditionally dereferencing the result of a dyn_cast. You are either 
missing a null-check or this should be a cast<>



Comment at: lib/CodeGen/CGExprScalar.cpp:3225-3226
+ dyn_cast(Result->getType());
+  if ((ResultTy->getBitWidth() > 1) &&
+  (E->getType() == CGF.getContext().BoolTy))
+Result = Builder.CreateTrunc(Result, Builder.getInt1Ty());

Extra parens.


https://reviews.llvm.org/D38656



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:455
+for (const auto  : PathSearch) {
+  if(FunctionDeclPath.find(FileMatch) != std::string::npos) {
+return false;

Space after if.



Comment at: lib/CodeGen/CodeGenFunction.cpp:473
+  for (const auto  : FunctionSearch) {
+if(FunctionName.find(FuncMatch) != std::string::npos) {
+  return false;

Ditto.


https://reviews.llvm.org/D37624



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

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list=<arg1, arg2, ...> to match gcc options.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:463
+
+  // Skip demangling if decl is extern "C"
+  if (ActualFuncDecl && !ActualFuncDecl->isExternC()) {

Is this comment still correct?



Comment at: lib/CodeGen/CodeGenModule.h:503
+  /// Mapping from SourceLocation to PresumedLoc FileName
+  llvm::DenseMap SourceLocToFileNameMap;
+

Pointers lean right.



Comment at: lib/CodeGen/CodeGenModule.h:1212
+  /// Get SourceLoc to FileName map cache
+  inline llvm::DenseMap () {
+return SourceLocToFileNameMap;

inline is redundant here.


https://reviews.llvm.org/D37624



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.h:32
 #include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/DenseMap.h"

Any reason to do this? I'd just keep getNestedExpressionLocationsEnabled in the 
.cpp file.


https://reviews.llvm.org/D37529



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

[PATCH] D33852: Enable __declspec(selectany) on linux

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:3154
+
+def SelectAnyDocs : Documentation {
+   let Content = [{This attribute makes global symbol have a weak definition

aaron.ballman wrote:
> Prazek wrote:
> > aaron.ballman wrote:
> > > Prazek wrote:
> > > > aaron.ballman wrote:
> > > > > I think you need to set the `Category` as well.
> > > > > 
> > > > > To test this you should run something like (replacing  and 
> > > > > fixing up path separators as needed):
> > > > > ```
> > > > > clang-tblgen -gen-attr-docs -I \llvm\tools\clang\include 
> > > > > \llvm\tools\clang\include\clang\Basic\Attr.td -o 
> > > > > \llvm\tools\clang\docs\AttributeReference.rst
> > > > > ```
> > > > Thanks for the testing command. Should I do anything in order to check 
> > > > if docs build?
> > > > I enabled docs with -DLLVM_BUILD_DOCS=ON, and I have sphinx. Everything 
> > > > builds, but I don't see docs anywhere.
> > > > 
> > > > the docs looks like this:
> > > > +selectany (gnu::selectany)
> > > > +--
> > > > +.. csv-table:: Supported Syntaxes
> > > > +   :header: "GNU", "C++11", "__declspec", "Keyword", "Pragma", "Pragma 
> > > > clang attribute"
> > > > +
> > > > +   "X","X","X","", "", ""
> > > > +
> > > > +This attribute appertains to a global symbol, causing it to have a weak
> > > > +definition (
> > > > +.. _`linkonce`: https://llvm.org/docs/LangRef.html#linkage-types
> > > > +), allowing the linker to select any definition.
> > > > +
> > > > +For more information see
> > > > +.. `gcc documentation`: 
> > > > https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Microsoft-Windows-Variable-Attributes.html
> > > > +
> > > > Thanks for the testing command. Should I do anything in order to check 
> > > > if docs build?
> > > 
> > > On Windows, I use `make html` within the clang\docs directory to generate 
> > > the actual sphinx docs -- that will tell you if there are sphinx issues. 
> > > Be sure you do *not* commit the AttributeReference.rst file that it 
> > > generates, however.
> > I tried building docs but it doesn't seem that there is any target like 
> > "html". Do you know if there is documentation on how to build docs? I 
> > couldn't find any.
> I don't know that we have any docs on that, but I believe the command you 
> want to execute is `sphinx-build -b html -d _build/doctrees . _build/html` (I 
> pulled this from make.bat in the docs directory.)
This is not the first time this has come up. Can you please add a comment to 
the top of AttrDocs.td which explains how to generate the HTML?


https://reviews.llvm.org/D33852



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

[PATCH] D37206: [ItaniumCXXABI] Always use linkonce_odr linkage for RTTI data on MinGW

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D37206



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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In https://reviews.llvm.org/D37042#849726, @majnemer wrote:

> I'd expect this to be more limited.
>
> Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of 
> NullToPointer and 'X', emit inttoptr(X)"


Although maybe that is too strict... I guess stuff gets hairy quickly if you 
want to be able to also catch (char*)NULL + x


https://reviews.llvm.org/D37042



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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

I'd expect this to be more limited.

Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of 
NullToPointer and 'X', emit inttoptr(X)"


https://reviews.llvm.org/D37042



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

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: include/clang/AST/Expr.h:3816
+/// __builtin_LINE(), __builtin_COLUMN(), __builtin_FUNCTION(), or
+/// __BUILTIN_FILE()
+class SourceLocExpr final : public Expr {

__BUILTIN_FILE -> __builtin_FILE



Comment at: include/clang/AST/Expr.h:3838
+  /// function.
+  const char *getBuiltinStr() const LLVM_READONLY;
+

StringRef?


https://reviews.llvm.org/D37035



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

[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: include/clang/Lex/TokenLexer.h:169
+  bool PasteTokens(Token ,
+   llvm::ArrayRef AltTokens = llvm::ArrayRef(),
+   unsigned int *const AltCurTokenIdx = nullptr);

I think `llvm::ArrayRef()` can just be `llvm::None`.


Repository:
  rL LLVM

https://reviews.llvm.org/D35782



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

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:5625
+  // The Run-time ABI for the ARM Architecture section 4.1.2 requires
+  // AEABI-complying FP helper functions to use the base AAPCS
+  // These AEABI functions are expanded in the ARM llvm backend, all the 
builtin

Period at the end of a sentence.



Comment at: test/CodeGen/arm-float-helpers.c:32
+// Note that it is only the functions in 4.1.2 that must use the base AAPCS,
+// other runtime functions such as the _Complex helper routines are not covered
+

Ditto.



Comment at: test/CodeGen/complex-math.c:482
+  // Run-time ABI for the ARM architecture document so they must not always
+  // use the base AAPCS
+

Ditto.


https://reviews.llvm.org/D35538



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

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

[David Majnemer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL309058: [CodeGen] Correctly model std::byte's aliasing 
properties (authored by majnemer).

Changed prior to commit:
  https://reviews.llvm.org/D35824?vs=108166=108185#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35824

Files:
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/lib/AST/Type.cpp
  cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
  cfe/trunk/test/CodeGenCXX/std-byte.cpp


Index: cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
@@ -139,6 +139,12 @@
 }
   }
 
+  // C++1z [basic.lval]p10: "If a program attempts to access the stored value 
of
+  // an object through a glvalue of other than one of the following types the
+  // behavior is undefined: [...] a char, unsigned char, or std::byte type."
+  if (Ty->isStdByteType())
+return MetadataCache[Ty] = getChar();
+
   // Handle pointers.
   // TODO: Implement C++'s type "similarity" and consider dis-"similar"
   // pointers distinct.
Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -2313,6 +2313,15 @@
   return false;
 }
 
+bool Type::isStdByteType() const {
+  if (auto *ET = getAs()) {
+auto *II = ET->getDecl()->getIdentifier();
+if (II && II->isStr("byte") && ET->getDecl()->isInStdNamespace())
+  return true;
+  }
+  return false;
+}
+
 bool Type::isPromotableIntegerType() const {
   if (const BuiltinType *BT = getAs())
 switch (BT->getKind()) {
Index: cfe/trunk/include/clang/AST/Type.h
===
--- cfe/trunk/include/clang/AST/Type.h
+++ cfe/trunk/include/clang/AST/Type.h
@@ -1752,6 +1752,7 @@
   bool isTemplateTypeParmType() const;  // C++ template type parameter
   bool isNullPtrType() const;   // C++11 std::nullptr_t
   bool isAlignValT() const; // C++17 std::align_val_t
+  bool isStdByteType() const;   // C++17 std::byte
   bool isAtomicType() const;// C11 _Atomic()
 
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
Index: cfe/trunk/test/CodeGenCXX/std-byte.cpp
===
--- cfe/trunk/test/CodeGenCXX/std-byte.cpp
+++ cfe/trunk/test/CodeGenCXX/std-byte.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++1z -Werror -triple i386-unknown-unknown -emit-llvm 
-O1 -disable-llvm-passes -o - %s | FileCheck %s
+
+// std::byte should be considered equivalent to char for aliasing.
+
+namespace std {
+enum byte : unsigned char {};
+}
+
+// CHECK-LABEL: define void @test0(
+extern "C" void test0(std::byte *sb, int *i) {
+  // CHECK: store i8 0, i8* %{{.*}} !tbaa [[TAG_CHAR:!.*]]
+  *sb = std::byte{0};
+
+  // CHECK: store i32 1, i32* %{{.*}} !tbaa [[TAG_INT:!.*]]
+  *i = 1;
+}
+
+enum byte : unsigned char {};
+namespace my {
+enum byte : unsigned char {};
+namespace std {
+enum byte : unsigned char {};
+} // namespace std
+} // namespace my
+
+// Make sure we don't get confused with other enums named 'byte'.
+
+// CHECK-LABEL: define void @test1(
+extern "C" void test1(::byte *b, ::my::byte *mb, ::my::std::byte *msb) {
+  *b = ::byte{0};
+  *mb = ::my::byte{0};
+  *msb = ::my::std::byte{0};
+  // CHECK-NOT: store i8 0, i8* %{{.*}} !tbaa [[TAG_CHAR]]
+}
+
+// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
+// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
+// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C++ TBAA"}
+// CHECK: [[TAG_CHAR]] = !{[[TYPE_CHAR:!.*]], [[TYPE_CHAR]], i64 0}
+// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
+// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]


Index: cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenTBAA.cpp
@@ -139,6 +139,12 @@
 }
   }
 
+  // C++1z [basic.lval]p10: "If a program attempts to access the stored value of
+  // an object through a glvalue of other than one of the following types the
+  // behavior is undefined: [...] a char, unsigned char, or std::byte type."
+  if (Ty->isStdByteType())
+return MetadataCache[Ty] = getChar();
+
   // Handle pointers.
   // TODO: Implement C++'s type "similarity" and consider dis-"similar"
   // pointers distinct.
Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -2313,6 +2313,15 @@
   return false;
 }
 
+bool Type::isStdByteType() const {
+  if (auto *ET = getAs()) {
+auto *II = ET->getDecl()->getIdentifier();
+if (II && II->isStr("byte") && ET->getDecl()->isInStdNamespace())
+  return true;
+  }
+  return false;

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

[David Majnemer via Phabricator via cfe-commits]

majnemer updated this revision to Diff 108166.
majnemer added a comment.

- Address review comments


https://reviews.llvm.org/D35824

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/CodeGen/CodeGenTBAA.cpp
  test/CodeGenCXX/std-byte.cpp


Index: test/CodeGenCXX/std-byte.cpp
===
--- /dev/null
+++ test/CodeGenCXX/std-byte.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++1z -Werror -triple i386-unknown-unknown -emit-llvm 
-O1 -disable-llvm-passes -o - %s | FileCheck %s
+
+// std::byte should be considered equivalent to char for aliasing.
+
+namespace std {
+enum byte : unsigned char {};
+}
+
+// CHECK-LABEL: define void @test0(
+extern "C" void test0(std::byte *sb, int *i) {
+  // CHECK: store i8 0, i8* %{{.*}} !tbaa [[TAG_CHAR:!.*]]
+  *sb = std::byte{0};
+
+  // CHECK: store i32 1, i32* %{{.*}} !tbaa [[TAG_INT:!.*]]
+  *i = 1;
+}
+
+enum byte : unsigned char {};
+namespace my {
+enum byte : unsigned char {};
+namespace std {
+enum byte : unsigned char {};
+} // namespace std
+} // namespace my
+
+// Make sure we don't get confused with other enums named 'byte'.
+
+// CHECK-LABEL: define void @test1(
+extern "C" void test1(::byte *b, ::my::byte *mb, ::my::std::byte *msb) {
+  *b = ::byte{0};
+  *mb = ::my::byte{0};
+  *msb = ::my::std::byte{0};
+  // CHECK-NOT: store i8 0, i8* %{{.*}} !tbaa [[TAG_CHAR]]
+}
+
+// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
+// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
+// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C++ TBAA"}
+// CHECK: [[TAG_CHAR]] = !{[[TYPE_CHAR:!.*]], [[TYPE_CHAR]], i64 0}
+// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
+// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
Index: lib/CodeGen/CodeGenTBAA.cpp
===
--- lib/CodeGen/CodeGenTBAA.cpp
+++ lib/CodeGen/CodeGenTBAA.cpp
@@ -85,6 +85,12 @@
 return TypeHasMayAlias(TTy->desugar());
   }
 
+  // C++1z [intro.object]p3: If a complete object is created in storage
+  // associated with another object e of type [...] "array of N std::byte", 
that
+  // array provides storage for the created object [...].
+  if (QTy->isStdByteType())
+return true;
+
   return false;
 }
 
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2313,6 +2313,15 @@
   return false;
 }
 
+bool Type::isStdByteType() const {
+  if (auto *ET = getAs()) {
+auto *II = ET->getDecl()->getIdentifier();
+if (II && II->isStr("byte") && ET->getDecl()->isInStdNamespace())
+  return true;
+  }
+  return false;
+}
+
 bool Type::isPromotableIntegerType() const {
   if (const BuiltinType *BT = getAs())
 switch (BT->getKind()) {
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1752,6 +1752,7 @@
   bool isTemplateTypeParmType() const;  // C++ template type parameter
   bool isNullPtrType() const;   // C++11 std::nullptr_t
   bool isAlignValT() const; // C++17 std::align_val_t
+  bool isStdByteType() const;   // C++17 std::byte
   bool isAtomicType() const;// C11 _Atomic()
 
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \


Index: test/CodeGenCXX/std-byte.cpp
===
--- /dev/null
+++ test/CodeGenCXX/std-byte.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++1z -Werror -triple i386-unknown-unknown -emit-llvm -O1 -disable-llvm-passes -o - %s | FileCheck %s
+
+// std::byte should be considered equivalent to char for aliasing.
+
+namespace std {
+enum byte : unsigned char {};
+}
+
+// CHECK-LABEL: define void @test0(
+extern "C" void test0(std::byte *sb, int *i) {
+  // CHECK: store i8 0, i8* %{{.*}} !tbaa [[TAG_CHAR:!.*]]
+  *sb = std::byte{0};
+
+  // CHECK: store i32 1, i32* %{{.*}} !tbaa [[TAG_INT:!.*]]
+  *i = 1;
+}
+
+enum byte : unsigned char {};
+namespace my {
+enum byte : unsigned char {};
+namespace std {
+enum byte : unsigned char {};
+} // namespace std
+} // namespace my
+
+// Make sure we don't get confused with other enums named 'byte'.
+
+// CHECK-LABEL: define void @test1(
+extern "C" void test1(::byte *b, ::my::byte *mb, ::my::std::byte *msb) {
+  *b = ::byte{0};
+  *mb = ::my::byte{0};
+  *msb = ::my::std::byte{0};
+  // CHECK-NOT: store i8 0, i8* %{{.*}} !tbaa [[TAG_CHAR]]
+}
+
+// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
+// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
+// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C++ TBAA"}
+// CHECK: [[TAG_CHAR]] = !{[[TYPE_CHAR:!.*]], [[TYPE_CHAR]], i64 0}
+// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
+// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
Index: lib/CodeGen/CodeGenTBAA.cpp

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

[David Majnemer via Phabricator via cfe-commits]

majnemer updated this revision to Diff 108105.
majnemer added a comment.

- Address review feedback.


https://reviews.llvm.org/D35824

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/CodeGen/CodeGenTBAA.cpp
  test/CodeGenCXX/std-byte.cpp


Index: test/CodeGenCXX/std-byte.cpp
===
--- /dev/null
+++ test/CodeGenCXX/std-byte.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++1z -Werror -triple i386-unknown-unknown -emit-llvm 
-O1 -disable-llvm-passes -o - %s | FileCheck %s
+
+// std::byte should be considered equivalent to char for aliasing.
+
+namespace std {
+enum byte : unsigned char {};
+}
+
+void test0(std::byte *sb, int *i) {
+  // CHECK: store i8 0, i8* %{{.*}}, !tbaa [[TAG_CHAR:!.*]]
+  // PATH: store i8 0, i8* %{{.*}}, !tbaa [[TAG_CHAR:!.*]]
+  *sb = std::byte{0};
+
+  // CHECK: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  // PATH: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  *i = 1;
+}
+
+// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
+// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
+// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C++ TBAA"}
+// CHECK: [[TAG_CHAR]] = !{[[TYPE_CHAR:!.*]], [[TYPE_CHAR]], i64 0}
+// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
+// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
Index: lib/CodeGen/CodeGenTBAA.cpp
===
--- lib/CodeGen/CodeGenTBAA.cpp
+++ lib/CodeGen/CodeGenTBAA.cpp
@@ -85,6 +85,12 @@
 return TypeHasMayAlias(TTy->desugar());
   }
 
+  // C++1z [intro.object]p3: If a complete object is created in storage
+  // associated with another object e of type [...] "array of N std::byte", 
that
+  // array provides storage for the created object [...].
+  if (QTy->isStdByteType())
+return true;
+
   return false;
 }
 
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2313,6 +2313,15 @@
   return false;
 }
 
+bool Type::isStdByteType() const {
+  if (auto *ET = getAs()) {
+auto *II = ET->getDecl()->getIdentifier();
+if (II && II->isStr("byte") && ET->getDecl()->isInStdNamespace())
+  return true;
+  }
+  return false;
+}
+
 bool Type::isPromotableIntegerType() const {
   if (const BuiltinType *BT = getAs())
 switch (BT->getKind()) {
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1752,6 +1752,7 @@
   bool isTemplateTypeParmType() const;  // C++ template type parameter
   bool isNullPtrType() const;   // C++11 std::nullptr_t
   bool isAlignValT() const; // C++17 std::align_val_t
+  bool isStdByteType() const;   // C++17 std::byte
   bool isAtomicType() const;// C11 _Atomic()
 
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \


Index: test/CodeGenCXX/std-byte.cpp
===
--- /dev/null
+++ test/CodeGenCXX/std-byte.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++1z -Werror -triple i386-unknown-unknown -emit-llvm -O1 -disable-llvm-passes -o - %s | FileCheck %s
+
+// std::byte should be considered equivalent to char for aliasing.
+
+namespace std {
+enum byte : unsigned char {};
+}
+
+void test0(std::byte *sb, int *i) {
+  // CHECK: store i8 0, i8* %{{.*}}, !tbaa [[TAG_CHAR:!.*]]
+  // PATH: store i8 0, i8* %{{.*}}, !tbaa [[TAG_CHAR:!.*]]
+  *sb = std::byte{0};
+
+  // CHECK: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  // PATH: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  *i = 1;
+}
+
+// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
+// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
+// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C++ TBAA"}
+// CHECK: [[TAG_CHAR]] = !{[[TYPE_CHAR:!.*]], [[TYPE_CHAR]], i64 0}
+// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
+// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
Index: lib/CodeGen/CodeGenTBAA.cpp
===
--- lib/CodeGen/CodeGenTBAA.cpp
+++ lib/CodeGen/CodeGenTBAA.cpp
@@ -85,6 +85,12 @@
 return TypeHasMayAlias(TTy->desugar());
   }
 
+  // C++1z [intro.object]p3: If a complete object is created in storage
+  // associated with another object e of type [...] "array of N std::byte", that
+  // array provides storage for the created object [...].
+  if (QTy->isStdByteType())
+return true;
+
   return false;
 }
 
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2313,6 +2313,15 @@
   return false;
 }
 
+bool Type::isStdByteType() const {
+  if (auto *ET = getAs()) {
+auto *II = ET->getDecl()->getIdentifier();
+if (II && II->isStr("byte") && ET->getDecl()->isInStdNamespace())
+ 

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

[David Majnemer via Phabricator via cfe-commits]

majnemer created this revision.

std::byte, when defined as an enum, needs to be given special treatment
with regards to its aliasing properties. An array of std::byte is
allowed to be used as storage for other types.

This fixes PR33916.


https://reviews.llvm.org/D35824

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGenCXX/std-byte.cpp


Index: test/CodeGenCXX/std-byte.cpp
===
--- /dev/null
+++ test/CodeGenCXX/std-byte.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c++1z -Werror -triple i386-unknown-unknown -emit-llvm 
-O1 -disable-llvm-passes -o - %s | FileCheck %s
+
+// std::byte should be considered equivalent to char for aliasing.
+
+namespace std {
+enum byte : unsigned char {};
+}
+
+void test0(std::byte *sb, int *i) {
+  // CHECK: store i8 0, i8* %{{.*}}, !tbaa [[TAG_STD_BYTE:!.*]]
+  // PATH: store i8 0, i8* %{{.*}}, !tbaa [[TAG_STD_BYTE:!.*]]
+  *sb = std::byte{0};
+
+  // CHECK: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  // PATH: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  *i = 1;
+}
+
+// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
+// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
+// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C++ TBAA"}
+// CHECK: [[TAG_STD_BYTE]] = !{[[TYPE_STD_BYTE:!.*]], [[TYPE_STD_BYTE]], i64 0}
+// CHECK: [[TYPE_STD_BYTE]] = !{!"_ZTSSt4byte", [[TYPE_CHAR]], i64 0}
+// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
+// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13227,6 +13227,7 @@
   DeclContext *DC = CurContext;
   bool isStdBadAlloc = false;
   bool isStdAlignValT = false;
+  bool isStdByte = false;
 
   RedeclarationKind Redecl = ForRedeclaration;
   if (TUK == TUK_Friend || TUK == TUK_Reference)
@@ -13444,6 +13445,8 @@
   isStdAlignValT = true;
   if (Previous.empty() && StdAlignValT)
 Previous.addDecl(getStdAlignValT());
+} else if (Name->isStr("byte")) {
+  isStdByte = true;
 }
   }
 
@@ -13845,6 +13848,13 @@
 if (isStdAlignValT && (!StdAlignValT || getStdAlignValT()->isImplicit()))
   StdAlignValT = cast(New);
 
+// C++1z [intro.object]p3: If a complete object is created (5.3.4) in
+// storage associated with another object e of type "array of N unsigned
+// char" or "array of N std::byte", that array provides storage for the
+// created object [...].
+if (isStdByte)
+  New->addAttr(MayAliasAttr::CreateImplicit(Context, New->getLocation()));
+
 // If this is an undefined enum, warn.
 if (TUK != TUK_Definition && !Invalid) {
   TagDecl *Def;


Index: test/CodeGenCXX/std-byte.cpp
===
--- /dev/null
+++ test/CodeGenCXX/std-byte.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c++1z -Werror -triple i386-unknown-unknown -emit-llvm -O1 -disable-llvm-passes -o - %s | FileCheck %s
+
+// std::byte should be considered equivalent to char for aliasing.
+
+namespace std {
+enum byte : unsigned char {};
+}
+
+void test0(std::byte *sb, int *i) {
+  // CHECK: store i8 0, i8* %{{.*}}, !tbaa [[TAG_STD_BYTE:!.*]]
+  // PATH: store i8 0, i8* %{{.*}}, !tbaa [[TAG_STD_BYTE:!.*]]
+  *sb = std::byte{0};
+
+  // CHECK: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  // PATH: store i32 1, i32* %{{.*}}, !tbaa [[TAG_INT:!.*]]
+  *i = 1;
+}
+
+// CHECK:  !"any pointer", [[TYPE_CHAR:!.*]],
+// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", [[TAG_CXX_TBAA:!.*]],
+// CHECK: [[TAG_CXX_TBAA]] = !{!"Simple C++ TBAA"}
+// CHECK: [[TAG_STD_BYTE]] = !{[[TYPE_STD_BYTE:!.*]], [[TYPE_STD_BYTE]], i64 0}
+// CHECK: [[TYPE_STD_BYTE]] = !{!"_ZTSSt4byte", [[TYPE_CHAR]], i64 0}
+// CHECK: [[TAG_INT]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0}
+// CHECK: [[TYPE_INT]] = !{!"int", [[TYPE_CHAR]]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13227,6 +13227,7 @@
   DeclContext *DC = CurContext;
   bool isStdBadAlloc = false;
   bool isStdAlignValT = false;
+  bool isStdByte = false;
 
   RedeclarationKind Redecl = ForRedeclaration;
   if (TUK == TUK_Friend || TUK == TUK_Reference)
@@ -13444,6 +13445,8 @@
   isStdAlignValT = true;
   if (Previous.empty() && StdAlignValT)
 Previous.addDecl(getStdAlignValT());
+} else if (Name->isStr("byte")) {
+  isStdByte = true;
 }
   }
 
@@ -13845,6 +13848,13 @@
 if (isStdAlignValT && (!StdAlignValT || getStdAlignValT()->isImplicit()))
   StdAlignValT = cast(New);
 
+// C++1z [intro.object]p3: If a complete object is created (5.3.4) in
+// storage associated with another object e of type "array of N unsigned
+// char" or "array of N std::byte", that array provides storage for the
+// created object [...].
+if (isStdByte)
+  

[PATCH] D34972: [CodeGen] Propagate dllexport to thunks

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D34972



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

[PATCH] D34972: [CodeGen] Propagate dllexport to thunks

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

OK, so we are exporting the thunks so that the linker will generate import 
thunks for the thunks. I think that we should have a comment to that effect 
near the code you added.


https://reviews.llvm.org/D34972



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

[PATCH] D35715: Preserve typedef names in debug info for template type parameters

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

This might be a silly question but why not do this by default?


https://reviews.llvm.org/D35715



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

[PATCH] D35472: Implement P0463R1: "Endian just Endian"

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: test/std/utilities/meta/meta.type.synop/endian.pass.cpp:39-42
+union {
+uint32_t i;
+char c[4];
+} u = {0x01020304};

This is undefined behavior as-is because you are reading from a union member 
other than the active union member.

I'd recommend a sequence like:
  uint32_t i = 0x01020304;
  char c[4];
  memcpy(c, , 4);


https://reviews.llvm.org/D35472



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

[PATCH] D35180: Expose the Clang::QualType to llvm::Type conversion functions

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/CodeGen/CodeGenABITypes.cpp:71
+  assert(FD != nullptr && "Expected a non-null function declaration!");
+  llvm::Type* T = CGM.getTypes().ConvertFunctionType(FD->getType(), FD);
+

Pointers lean right.



Comment at: lib/CodeGen/CodeGenABITypes.cpp:73
+
+  if(auto FT = dyn_cast(T))
+return FT;

need a space between the `if` and the parenthesis.


https://reviews.llvm.org/D35180



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

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In https://reviews.llvm.org/D34714#793205, @rnk wrote:

> Did you locally add a test case for the dllimport member function pointer 
> template argument?


Arg, yes. Forgot to add the file...




Comment at: lib/Sema/SemaTemplate.cpp:5704
+  else
+NPV = isNullPointerValueTemplateArgument(S, Param, ParamType, ResultArg);
+

rnk wrote:
> Think we should pass in Entity as an optional parameter to 
> isNullPointerValueTemplateArgument to share the logic?
Seems reasonable, I'll give that a go.


https://reviews.llvm.org/D34714



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

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

[David Majnemer via Phabricator via cfe-commits]

majnemer created this revision.

r306137 made dllimport pointers to member functions non-constant. This
is correct because a load must be executed to resolve any dllimported
data. However, r306137 did not account for the use of dllimport member
function pointers used as template arguments.

This change piggie-backs almost entirely on Reid's r306137, I just added
the template instantiation fix.

This fixes PR33570.


https://reviews.llvm.org/D34714

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaTemplate.cpp
  test/CodeGenCXX/dllimport-memptr-global.cpp

Index: test/CodeGenCXX/dllimport-memptr-global.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllimport-memptr-global.cpp
@@ -0,0 +1,58 @@
+// Also check that -Wglobal-constructors does the right thing. Strictly
+// speaking, this is a Sema test, but this avoids test case duplication.
+// RUN: %clang_cc1 -Wglobal-constructors %s -verify -triple i686-windows-msvc -fms-extensions -std=c++11
+//
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple i686-windows-msvc -fms-extensions -std=c++11 | FileCheck %s
+
+struct __declspec(dllimport) Single {
+  void nonvirt();
+  virtual void virt();
+};
+
+struct A { int a; };
+struct B { int b; };
+struct __declspec(dllimport) Multi : A, B {
+  void nonvirt();
+  virtual void virt();
+};
+
+struct __declspec(dllimport) Virtual : virtual A {
+  void nonvirt();
+  virtual void virt();
+};
+
+struct General;
+static_assert(sizeof(void (General::*)()) == 16, "force general memptr model");
+struct __declspec(dllimport) General {
+  void nonvirt();
+  virtual void virt();
+};
+
+auto mp_single_nv = ::nonvirt; // expected-warning {{global constructor}}
+auto mp_multi_nv = ::nonvirt; // expected-warning {{global constructor}}
+auto mp_virtual_nv = ::nonvirt; // expected-warning {{global constructor}}
+auto mp_general_nv = ::nonvirt; // expected-warning {{global constructor}}
+
+auto mp_single_v = ::virt;
+auto mp_multi_v = ::virt;
+auto mp_virtual_v = ::virt;
+auto mp_general_v = ::virt;
+
+// All of the non-virtual globals need dynamic initializers.
+
+// CHECK: @"\01?mp_single_nv@@3P8Single@@AEXXZQ1@" = global i8* null, align 4
+// CHECK: @"\01?mp_multi_nv@@3P8Multi@@AEXXZQ1@" = global { i8*, i32 } zeroinitializer, align 4
+// CHECK: @"\01?mp_virtual_nv@@3P8Virtual@@AEXXZQ1@" = global { i8*, i32, i32 } zeroinitializer, align 4
+// CHECK: @"\01?mp_general_nv@@3P8General@@AEXXZQ1@" = global { i8*, i32, i32, i32 } zeroinitializer, align 4
+
+// CHECK: @"\01?mp_single_v@@3P8Single@@AEXXZQ1@" = global i8* bitcast (void (%struct.Single*, ...)* @"\01??_9Single@@$BA@AE" to i8*), align 4
+// CHECK: @"\01?mp_multi_v@@3P8Multi@@AEXXZQ1@" = global { i8*, i32 } { i8* bitcast (void (%struct.Multi*, ...)* @"\01??_9Multi@@$BA@AE" to i8*), i32 0 }, align 4
+// CHECK: @"\01?mp_virtual_v@@3P8Virtual@@AEXXZQ1@" = global { i8*, i32, i32 } { i8* bitcast (void (%struct.Virtual*, ...)* @"\01??_9Virtual@@$BA@AE" to i8*), i32 0, i32 0 }, align 4
+// CHECK: @"\01?mp_general_v@@3P8General@@AEXXZQ1@" = global { i8*, i32, i32, i32 } { i8* bitcast (void (%struct.General*, ...)* @"\01??_9General@@$BA@AE" to i8*), i32 0, i32 0, i32 0 }, align 4
+
+// CHECK: define internal void @_GLOBAL__sub_I{{.*}}() {{.*}} {
+// CHECK:   call void @"\01??__Emp_single_nv@@YAXXZ"()
+// CHECK:   call void @"\01??__Emp_multi_nv@@YAXXZ"()
+// CHECK:   call void @"\01??__Emp_virtual_nv@@YAXXZ"()
+// CHECK:   call void @"\01??__Emp_general_nv@@YAXXZ"()
+// CHECK: }
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -5636,39 +5636,8 @@
  TemplateArgument ) {
   bool Invalid = false;
 
-  // Check for a null pointer value.
   Expr *Arg = ResultArg;
-  switch (isNullPointerValueTemplateArgument(S, Param, ParamType, Arg)) {
-  case NPV_Error:
-return true;
-  case NPV_NullPointer:
-S.Diag(Arg->getExprLoc(), diag::warn_cxx98_compat_template_arg_null);
-Converted = TemplateArgument(S.Context.getCanonicalType(ParamType),
- /*isNullPtr*/true);
-return false;
-  case NPV_NotNullPointer:
-break;
-  }
-
   bool ObjCLifetimeConversion;
-  if (S.IsQualificationConversion(Arg->getType(),
-  ParamType.getNonReferenceType(),
-  false, ObjCLifetimeConversion)) {
-Arg = S.ImpCastExprToType(Arg, ParamType, CK_NoOp,
-  Arg->getValueKind()).get();
-ResultArg = Arg;
-  } else if (!S.Context.hasSameUnqualifiedType(Arg->getType(),
-ParamType.getNonReferenceType())) {
-// We can't perform this conversion.
-S.Diag(Arg->getLocStart(), diag::err_template_arg_not_convertible)
-  << Arg->getType() << ParamType << Arg->getSourceRange();
-S.Diag(Param->getLocation(), diag::note_template_param_here);
-return true;

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

[David Majnemer via Phabricator via cfe-commits]

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

Looks good to me but I definitely want to hear what @efriedma has to say.


Repository:
  rL LLVM

https://reviews.llvm.org/D34523



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

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:993-994
+  Out << "YAX";
+  // struct __block_literal *
+  Out << "PA";
+  mangleArtificalTagType(TTK_Struct,

compnerd wrote:
> majnemer wrote:
> > Shouldn't we also mangle an 'E' in here on 64-bit platforms?
> I suppose that the default handling in x86_64 would give that.  I don't have 
> a strong enough opinion on that.  I can add that if you think it makes a 
> difference.
It is consistent with other pointer manglings; I think it'd be best to mangle 
pointer types as being 64-bit consistently.


Repository:
  rL LLVM

https://reviews.llvm.org/D34523



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

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

We need tests that show that it does the right thing in blocks defined in 
classes in classes and other nested concepts.




Comment at: lib/AST/MicrosoftMangle.cpp:980-981
+  unsigned Discriminator = BD->getBlockManglingNumber();
+  if (!Discriminator)
+Discriminator = Context.getBlockId(BD, /*Local=*/false);
+

Why isn't it local?



Comment at: lib/AST/MicrosoftMangle.cpp:991-992
+Out << '@';
+  // void __cdecl
+  Out << "YAX";
+  // struct __block_literal *

Can blocks not be given a specific calling convention?



Comment at: lib/AST/MicrosoftMangle.cpp:993-994
+  Out << "YAX";
+  // struct __block_literal *
+  Out << "PA";
+  mangleArtificalTagType(TTK_Struct,

Shouldn't we also mangle an 'E' in here on 64-bit platforms?



Comment at: lib/AST/MicrosoftMangle.cpp:999-1001
+  if (const auto *MC = BD->getBlockManglingContextDecl())
+if (const auto *ND = dyn_cast(MC))
+  mangleUnqualifiedName(ND);

This logic should be explained.



Comment at: lib/AST/MicrosoftMangle.cpp:1003-1005
+  if (isa(DC))
+break;
+  continue;

This logic should be explained.


Repository:
  rL LLVM

https://reviews.llvm.org/D34523



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

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:985
+
+  Out << '?' << Discriminate("_block_invoke", Discriminator) << '@';
+  if (const auto *RD = dyn_cast(DC))

Should this be `Out << '?' << mangleSourceName(Discriminate("_block_invoke", 
Discriminator));`



Comment at: lib/AST/MicrosoftMangle.cpp:990-994
+  Out << "YAX";
+  Out << "PA";
+  mangleArtificalTagType(TTK_Struct,
+ Discriminate("__block_literal", Discriminator));
+  Out << "@Z";

You should probably add comments explaining these bits.


Repository:
  rL LLVM

https://reviews.llvm.org/D34523



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

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: include/__config:234-235
+// a MS compatibility version is specified.
 #  ifndef __MINGW32__
-#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library
+#ifdef _MSC_VER
+#  define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library

compnerd wrote:
> smeenai wrote:
> > You can combine this into just
> > 
> > ```
> > #  if defined(_MSC_VER) && !defined(__MINGW32__)
> > ```
> > 
> > I don't know if `__MINGW32__` and `_MSC_VER` will ever be compiled 
> > simultaneously. (clang never defines `_MSC_VER` for its MinGW triples, for 
> > example.)
> What if MinGW is built with clang/c2 and MSVC extensions?  I think that the 
> two could be defined together.  What about cygwin and clang/c2?  I guess we 
> can ignore that since cygwin is not under active development.
> 
> I think this really goes back to my idea for an additional flag to indicate 
> the C library in use.  We can interpret it from the canonicalized triple that 
> LLVM/clang use.
clang/c2 is dead.


https://reviews.llvm.org/D34588



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

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:981-984
+  Out << "YAXPAU__block_literal";
+  if (Discriminator)
+Out<< '_' << Discriminator;
+  Out << "@@@Z";

compnerd wrote:
> majnemer wrote:
> > I think you want to use mangleArtificalTagType here.
> I was considering that.  The addition of the `Discriminator` makes that 
> harder.  I can create a local buffer and create the name and then mangle that 
> though if you feel strongly about that.
I do, it will ensure it correctly backreferences.


Repository:
  rL LLVM

https://reviews.llvm.org/D34523



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

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:981-984
+  Out << "YAXPAU__block_literal";
+  if (Discriminator)
+Out<< '_' << Discriminator;
+  Out << "@@@Z";

I think you want to use mangleArtificalTagType here.


Repository:
  rL LLVM

https://reviews.llvm.org/D34523



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

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Can you please have a test where you define blocks w/ static variables in 
several default arguments of the same function? Also would be good to have this 
in NSDMIs in class definitions.


Repository:
  rL LLVM

https://reviews.llvm.org/D34523



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

[PATCH] D34377: Support MS builtins using 'long' on Darwin/LP64

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: lib/AST/ASTContext.cpp:8551
+  break;
+}
 case 'W':

bruno wrote:
> bruno wrote:
> > rnk wrote:
> > > compnerd wrote:
> > > > I agree with @majnemer.  Why not base this on the Int64Type?
> > > I'd suggest this code:
> > >   IsSpecialLong = true;
> > >   // Use "long" if is 32 bits. This prefix is used by intrinsics that 
> > > need 32-bit types on LP64 platforms, but need to use "long" in the 
> > > prototype on LLP64 platforms like Win64.
> > >   if (Context.getTargetInfo().getLongWidth() == 32)
> > > HowLong = 1;
> > >   break;
> > See below.
> I tried something similar before, but I get two tests failing 
> CodeGen/ms-intrinsics-rotations.c and CodeGen/pr27892.c. Your suggestion hits 
> the same failing tests. Both fails because of the Linux issue mentioned 
> above: i32 codegen where i64 is expected. Of course I could improve the 
> condition to handle Linux, but at that point I just thing it's better to use 
> Darwin, which is what the fix is towards anyway. Additional ideas?
I don't think we should sweep this under the rug just because there are some 
test failures. There is probably some latent bug worth investigating.


https://reviews.llvm.org/D34377



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

[PATCH] D32428: Fix for Itanium mangler issue with templates (bug: 31405)

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D32428



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

[PATCH] D33852: Enable __declspec(selectany) on linux

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: include/clang/Basic/Attr.td:2421
 
-def SelectAny : InheritableAttr, TargetSpecificAttr {
+def SelectAny : InheritableAttr, TargetSpecificAttr {
   let Spellings = [Declspec<"selectany">, GCC<"selectany">];

selectany should work on targets other than "x86", "x86_64", "arm", "thumb", 
etc. I think it is only necessary to require that it be a COFF or ELF target.


https://reviews.llvm.org/D33852



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