r341489 - Add -Wobjc-property-assign-on-object-type.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Sep  5 12:02:00 2018
New Revision: 341489

URL: http://llvm.org/viewvc/llvm-project?rev=341489&view=rev
Log:
Add -Wobjc-property-assign-on-object-type.

This is a warning about using 'assign' instead of 'unsafe_unretained'
in Objective-C property declarations.  It's off by default because there
isn't consensus in the Objective-C steering group that this is the right
thing to do, but we're nonetheless okay with adding it because there's a
substantial pool of Objective-C programmers who will appreciate the warning.

Patch by Alfred Zien!

Added:
cfe/trunk/test/SemaObjC/property-assign-on-object-type.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaObjCProperty.cpp
cfe/trunk/test/SemaObjC/property-in-class-extension-1.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=341489&r1=341488&r2=341489&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Sep  5 12:02:00 2018
@@ -380,6 +380,7 @@ def FunctionDefInObjCContainer : DiagGro
 def BadFunctionCast : DiagGroup<"bad-function-cast">;
 def ObjCPropertyImpl : DiagGroup<"objc-property-implementation">;
 def ObjCPropertyNoAttribute : DiagGroup<"objc-property-no-attribute">;
+def ObjCPropertyAssignOnObjectType : 
DiagGroup<"objc-property-assign-on-object-type">;
 def ObjCProtocolQualifiers : DiagGroup<"objc-protocol-qualifiers">;
 def ObjCMissingSuperCalls : DiagGroup<"objc-missing-super-calls">;
 def ObjCDesignatedInit : DiagGroup<"objc-designated-initializers">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=341489&r1=341488&r2=341489&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep  5 12:02:00 
2018
@@ -1046,6 +1046,9 @@ def err_objc_property_attr_mutually_excl
   "property attributes '%0' and '%1' are mutually exclusive">;
 def err_objc_property_requires_object : Error<
   "property with '%0' attribute must be of object type">;
+def warn_objc_property_assign_on_object : Warning<
+  "'assign' property of object type may become a dangling reference; consider 
using 'unsafe_unretained'">,
+  InGroup, DefaultIgnore;
 def warn_objc_property_no_assignment_attribute : Warning<
   "no 'assign', 'retain', or 'copy' attribute is specified - "
   "'assign' is assumed">,

Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=341489&r1=341488&r2=341489&view=diff
==
--- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
+++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Wed Sep  5 12:02:00 2018
@@ -2557,6 +2557,14 @@ void Sema::CheckObjCPropertyAttributes(D
 PropertyDecl->setInvalidDecl();
   }
 
+  // Check for assign on object types.
+  if ((Attributes & ObjCDeclSpec::DQ_PR_assign) &&
+  !(Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) &&
+  PropertyTy->isObjCRetainableType() &&
+  !PropertyTy->isObjCARCImplicitlyUnretainedType()) {
+Diag(Loc, diag::warn_objc_property_assign_on_object);
+  }
+
   // Check for more than one of { assign, copy, retain }.
   if (Attributes & ObjCDeclSpec::DQ_PR_assign) {
 if (Attributes & ObjCDeclSpec::DQ_PR_copy) {

Added: cfe/trunk/test/SemaObjC/property-assign-on-object-type.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-assign-on-object-type.m?rev=341489&view=auto
==
--- cfe/trunk/test/SemaObjC/property-assign-on-object-type.m (added)
+++ cfe/trunk/test/SemaObjC/property-assign-on-object-type.m Wed Sep  5 
12:02:00 2018
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wobjc-property-assign-on-object-type 
%s
+
+@interface Foo @end
+@protocol Prot @end
+
+@interface Bar
+@property(assign, readonly) Foo* o1; // expected-warning {{'assign' property 
of object type may become a dangling reference; consider using 
'unsafe_unretained'}}
+@property(unsafe_unretained, readonly) Foo* o2;
+
+@property(assign) Class classProperty;
+@property(assign) Class classWithProtocolProperty;
+@property(assign) int s1;
+@property(assign) int* s2;
+@end
+
+@interface Bar ()
+@property(readwrite) Foo* o1;
+@property(readwrite) Foo* o2;
+@end

Modified: cfe/trunk/test/SemaObjC/property-in-class-extension-1.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-in-class-exten

r341491 - Forbid address spaces on compound literals in local scope.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Sep  5 12:22:40 2018
New Revision: 341491

URL: http://llvm.org/viewvc/llvm-project?rev=341491&view=rev
Log:
Forbid address spaces on compound literals in local scope.

Patch by Bevin Hansson!

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/address_spaces.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=341491&r1=341490&r2=341491&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep  5 12:22:40 
2018
@@ -2612,6 +2612,8 @@ def err_arg_with_address_space : Error<
   "parameter may not be qualified with an address space">;
 def err_field_with_address_space : Error<
   "field may not be qualified with an address space">;
+def err_compound_literal_with_address_space : Error<
+  "compound literal in function scope may not be qualified with an address 
space">;
 def err_attr_objc_ownership_redundant : Error<
   "the type %0 is already explicitly ownership-qualified">;
 def err_invalid_nsnumber_type : Error<

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=341491&r1=341490&r2=341491&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Sep  5 12:22:40 2018
@@ -5727,12 +5727,20 @@ Sema::BuildCompoundLiteralExpr(SourceLoc
   LiteralExpr = Result.get();
 
   bool isFileScope = !CurContext->isFunctionOrMethod();
-  if (isFileScope &&
-  !LiteralExpr->isTypeDependent() &&
-  !LiteralExpr->isValueDependent() &&
-  !literalType->isDependentType()) { // 6.5.2.5p3
-if (CheckForConstantInitializer(LiteralExpr, literalType))
-  return ExprError();
+  if (isFileScope) {
+if (!LiteralExpr->isTypeDependent() &&
+!LiteralExpr->isValueDependent() &&
+!literalType->isDependentType()) // C99 6.5.2.5p3
+  if (CheckForConstantInitializer(LiteralExpr, literalType))
+return ExprError();
+  } else if (literalType.getAddressSpace() != LangAS::opencl_private &&
+ literalType.getAddressSpace() != LangAS::Default) {
+// Embedded-C extensions to C99 6.5.2.5:
+//   "If the compound literal occurs inside the body of a function, the
+//   type name shall not be qualified by an address-space qualifier."
+Diag(LParenLoc, diag::err_compound_literal_with_address_space)
+  << SourceRange(LParenLoc, LiteralExpr->getSourceRange().getEnd());
+return ExprError();
   }
 
   // In C, compound literals are l-values for some reason.

Modified: cfe/trunk/test/Sema/address_spaces.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/address_spaces.c?rev=341491&r1=341490&r2=341491&view=diff
==
--- cfe/trunk/test/Sema/address_spaces.c (original)
+++ cfe/trunk/test/Sema/address_spaces.c Wed Sep  5 12:22:40 2018
@@ -73,3 +73,17 @@ __attribute__((address_space("12"))) int
 char* cmp(_AS1 char *x,  _AS2 char *y) {
   return x < y ? x : y; // expected-error{{conditional operator with the 
second and third operands of type  ('__attribute__((address_space(1))) char *' 
and '__attribute__((address_space(2))) char *') which are pointers to 
non-overlapping address spaces}}
 }
+
+struct SomeStruct {
+  int a;
+  long b;
+  int c;
+};
+
+// Compound literals in function scope are lvalues with automatic storage 
duration,
+// so they cannot realistically be qualified with an address space.
+void as_compound_literal() {
+  (_AS1 struct SomeStruct){1, 2, 3}; // expected-error {{compound literal in 
function scope may not be qualified with an address space}}
+  (_AS1 char[]){"test"}; // expected-error {{compound literal in function 
scope may not be qualified with an address space}}
+  (_AS1 char[]){'a', 'b', 'c'}; // expected-error {{compound literal in 
function scope may not be qualified with an address space}}
+}


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

r337525 - Document -fobjc-weak as an extension.

[John McCall via cfe-commits]

Author: rjmccall
Date: Thu Jul 19 22:40:12 2018
New Revision: 337525

URL: http://llvm.org/viewvc/llvm-project?rev=337525&view=rev
Log:
Document -fobjc-weak as an extension.

Fixes rdar://24091053.

Modified:
cfe/trunk/docs/LanguageExtensions.rst

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=337525&r1=337524&r2=337525&view=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Thu Jul 19 22:40:12 2018
@@ -1200,6 +1200,51 @@ Objective-C objects. ``__has_feature(obj
 are allowed to have fields that are pointers to Objective-C objects managed by
 automatic reference counting.
 
+.. _objc-weak:
+
+Weak references
+---
+
+Clang supports ARC-style weak and unsafe references in Objective-C even
+outside of ARC mode.  Weak references must be explicitly enabled with
+the ``-fobjc-weak`` option; use ``__has_feature((objc_arc_weak))``
+to test whether they are enabled.  Unsafe references are enabled
+unconditionally.  ARC-style weak and unsafe references cannot be used
+when Objective-C garbage collection is enabled.
+
+Except as noted below, the language rules for the ``__weak`` and
+``__unsafe_unretained`` qualifiers (and the ``weak`` and
+``unsafe_unretained`` property attributes) are just as laid out
+in the :doc:`ARC specification `.
+In particular, note that some classes do not support forming weak
+references to their instances, and note that special care must be
+taken when storing weak references in memory where initialization
+and deinitialization are outside the responsibility of the compiler
+(such as in ``malloc``-ed memory).
+
+Loading from a ``__weak`` variable always implicitly retains the
+loaded value.  In non-ARC modes, this retain is normally balanced
+by an implicit autorelease.  This autorelease can be suppressed
+by performing the load in the receiver position of a ``-retain``
+message send (e.g. ``[weakReference retain]``); note that this performs
+only a single retain (the retain done when primitively loading from
+the weak reference).
+
+For the most part, ``__unsafe_unretained`` in non-ARC modes is just the
+default behavior of variables and therefore is not needed.  However,
+it does have an effect on the semantics of block captures: normally,
+copying a block which captures an Objective-C object or block pointer
+causes the captured pointer to be retained or copied, respectively,
+but that behavior is suppressed when the captured variable is qualified
+with ``__unsafe_unretained``.
+
+Note that the ``__weak`` qualifier formerly meant the GC qualifier in
+all non-ARC modes and was silently ignored outside of GC modes.  It now
+means the ARC-style qualifier in all non-GC modes and is no longer
+allowed if not enabled by either ``-fobjc-arc`` or ``-fobjc-weak``.
+It is expected that ``-fobjc-weak`` will eventually be enabled by default
+in all non-GC Objective-C modes.
+
 .. _objc-fixed-enum:
 
 Enumerations with a fixed underlying type


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

r337524 - Fix and improve the ARC spec's wording about unmanaged objects.

[John McCall via cfe-commits]

Author: rjmccall
Date: Thu Jul 19 22:40:09 2018
New Revision: 337524

URL: http://llvm.org/viewvc/llvm-project?rev=337524&view=rev
Log:
Fix and improve the ARC spec's wording about unmanaged objects.

Modified:
cfe/trunk/docs/AutomaticReferenceCounting.rst

Modified: cfe/trunk/docs/AutomaticReferenceCounting.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/AutomaticReferenceCounting.rst?rev=337524&r1=337523&r2=337524&view=diff
==
--- cfe/trunk/docs/AutomaticReferenceCounting.rst (original)
+++ cfe/trunk/docs/AutomaticReferenceCounting.rst Thu Jul 19 22:40:09 2018
@@ -974,28 +974,66 @@ It is undefined behavior to access an ow
 lvalue of a differently-qualified type, except that any non-``__weak`` object
 may be read through an ``__unsafe_unretained`` lvalue.
 
-It is undefined behavior if a managed operation is performed on a ``__strong``
-or ``__weak`` object without a guarantee that it contains a primitive zero
-bit-pattern, or if the storage for such an object is freed or reused without 
the
-object being first assigned a null pointer.
+It is undefined behavior if the storage of a ``__strong`` or ``__weak``
+object is not properly initialized before the first managed operation
+is performed on the object, or if the storage of such an object is freed
+or reused before the object has been properly deinitialized.  Storage for
+a ``__strong`` or ``__weak`` object may be properly initialized by filling
+it with the representation of a null pointer, e.g. by acquiring the memory
+with ``calloc`` or using ``bzero`` to zero it out.  A ``__strong`` or
+``__weak`` object may be properly deinitialized by assigning a null pointer
+into it.  A ``__strong`` object may also be properly initialized
+by copying into it (e.g. with ``memcpy``) the representation of a
+different ``__strong`` object whose storage has been properly initialized;
+doing this properly deinitializes the source object and causes its storage
+to no longer be properly initialized.  A ``__weak`` object may not be
+representation-copied in this way.
+
+These requirements are followed automatically for objects whose
+initialization and deinitialization are under the control of ARC:
+
+* objects of static, automatic, and temporary storage duration
+* instance variables of Objective-C objects
+* elements of arrays where the array object's initialization and
+  deinitialization are under the control of ARC
+* fields of Objective-C struct types where the struct object's
+  initialization and deinitialization are under the control of ARC
+* non-static data members of Objective-C++ non-union class types
+* Objective-C++ objects and arrays of dynamic storage duration created
+  with the ``new`` or ``new[]`` operators and destroyed with the
+  corresponding ``delete`` or ``delete[]`` operator
+
+They are not followed automatically for these objects:
+
+* objects of dynamic storage duration created in other memory, such as
+  that returned by ``malloc``
+* union members
 
 .. admonition:: Rationale
 
-  ARC cannot differentiate between an assignment operator which is intended to
-  "initialize" dynamic memory and one which is intended to potentially replace
-  a value.  Therefore the object's pointer must be valid before letting ARC at
-  it.  Similarly, C and Objective-C do not provide any language hooks for
-  destroying objects held in dynamic memory, so it is the programmer's
-  responsibility to avoid leaks (``__strong`` objects) and consistency errors
-  (``__weak`` objects).
-
-These requirements are followed automatically in Objective-C++ when creating
-objects of retainable object owner type with ``new`` or ``new[]`` and 
destroying
-them with ``delete``, ``delete[]``, or a pseudo-destructor expression.  Note
-that arrays of nontrivially-ownership-qualified type are not ABI compatible 
with
-non-ARC code because the element type is non-POD: such arrays that are
-``new[]``'d in ARC translation units cannot be ``delete[]``'d in non-ARC
-translation units and vice-versa.
+  ARC must perform special operations when initializing an object and
+  when destroying it.  In many common situations, ARC knows when an
+  object is created and when it is destroyed and can ensure that these
+  operations are performed correctly.  Otherwise, however, ARC requires
+  programmer cooperation to establish its initialization invariants
+  because it is infeasible for ARC to dynamically infer whether they
+  are intact.  For example, there is no syntactic difference in C between
+  an assignment that is intended by the programmer to initialize a variable
+  and one that is intended to replace the existing value stored there,
+  but ARC must perform one operation or the other.  ARC chooses to always
+  assume that objects are initialized (except when it is in charge of
+  initializing them) because the only workable alternative would be to
+  ban all code patterns that could potentiall

r324377 - Pass around function pointers as CGCallees, not bare llvm::Value*s.

[John McCall via cfe-commits]

Author: rjmccall
Date: Tue Feb  6 10:52:44 2018
New Revision: 324377

URL: http://llvm.org/viewvc/llvm-project?rev=324377&view=rev
Log:
Pass around function pointers as CGCallees, not bare llvm::Value*s.

The intention here is to make it easy to write frontend-assisted CFI
systems by propagating extra information in the CGCallee.

Modified:
cfe/trunk/lib/CodeGen/CGCXXABI.h
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGCall.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=324377&r1=324376&r2=324377&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.h Tue Feb  6 10:52:44 2018
@@ -413,10 +413,10 @@ public:
 CharUnits VPtrOffset) = 0;
 
   /// Build a virtual function pointer in the ABI-specific way.
-  virtual llvm::Value *getVirtualFunctionPointer(CodeGenFunction &CGF,
- GlobalDecl GD, Address This,
- llvm::Type *Ty,
- SourceLocation Loc) = 0;
+  virtual CGCallee getVirtualFunctionPointer(CodeGenFunction &CGF,
+ GlobalDecl GD, Address This,
+ llvm::Type *Ty,
+ SourceLocation Loc) = 0;
 
   /// Emit the ABI-specific virtual destructor call.
   virtual llvm::Value *

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=324377&r1=324376&r2=324377&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Feb  6 10:52:44 2018
@@ -4052,14 +4052,8 @@ RValue CodeGenFunction::EmitCall(const C
 }
   }
 
-  llvm::Value *CalleePtr;
-  if (Callee.isVirtual()) {
-const CallExpr *CE = Callee.getVirtualCallExpr();
-CalleePtr = CGM.getCXXABI().getVirtualFunctionPointer(
-*this, Callee.getVirtualMethodDecl(), Callee.getThisAddress(),
-Callee.getFunctionType(), CE ? CE->getLocStart() : SourceLocation());
-  } else
-CalleePtr = Callee.getFunctionPointer();
+  const CGCallee &ConcreteCallee = Callee.prepareConcreteCallee(*this);
+  llvm::Value *CalleePtr = ConcreteCallee.getFunctionPointer();
 
   // If we're using inalloca, set up that argument.
   if (ArgMemory.isValid()) {
@@ -4412,6 +4406,17 @@ RValue CodeGenFunction::EmitCall(const C
   return Ret;
 }
 
+CGCallee CGCallee::prepareConcreteCallee(CodeGenFunction &CGF) const {
+  if (isVirtual()) {
+const CallExpr *CE = getVirtualCallExpr();
+return CGF.CGM.getCXXABI().getVirtualFunctionPointer(
+CGF, getVirtualMethodDecl(), getThisAddress(),
+getFunctionType(), CE ? CE->getLocStart() : SourceLocation());
+  }
+
+  return *this;
+}
+
 /* VarArg handling */
 
 Address CodeGenFunction::EmitVAArg(VAArgExpr *VE, Address &VAListAddr) {

Modified: cfe/trunk/lib/CodeGen/CGCall.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.h?rev=324377&r1=324376&r2=324377&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.h (original)
+++ cfe/trunk/lib/CodeGen/CGCall.h Tue Feb  6 10:52:44 2018
@@ -206,6 +206,10 @@ public:
   return cast(
   getFunctionPointer()->getType()->getPointerElementType());
 }
+
+/// If this is a delayed callee computation of some sort, prepare
+/// a concrete callee.
+CGCallee prepareConcreteCallee(CodeGenFunction &CGF) const;
   };
 
   struct CallArg {

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=324377&r1=324376&r2=324377&view=diff
==
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Tue Feb  6 10:52:44 2018
@@ -280,9 +280,9 @@ public:
   llvm::GlobalVariable *getAddrOfVTable(const CXXRecordDecl *RD,
 CharUnits VPtrOffset) override;
 
-  llvm::Value *getVirtualFunctionPointer(CodeGenFunction &CGF, GlobalDecl GD,
- Address This, llvm::Type *Ty,
- SourceLocation Loc) override;
+  CGCallee getVirtualFunctionPointer(CodeGenFunction &CGF, GlobalDecl GD,
+ Address This, llvm::Type *Ty,
+ SourceLocation Loc) override;
 
   llvm::Value *EmitVirtualDestructorCall(CodeGenFunction &CGF,
  

r304951 - When determining the target function of an explicit instantiation, make

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Jun  7 18:00:05 2017
New Revision: 304951

URL: http://llvm.org/viewvc/llvm-project?rev=304951&view=rev
Log:
When determining the target function of an explicit instantiation, make
sure that non-template functions don't end up in the candidate set.

Fixes PR14211.

Patch by Don Hinton!

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p5.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=304951&r1=304950&r2=304951&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jun  7 18:00:05 2017
@@ -8957,7 +8957,8 @@ DeclResult Sema::ActOnExplicitInstantiat
   //   A member function [...] of a class template can be explicitly
   //  instantiated from the member definition associated with its class
   //  template.
-  UnresolvedSet<8> Matches;
+  UnresolvedSet<8> TemplateMatches;
+  FunctionDecl *NonTemplateMatch = nullptr;
   AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
   TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc());
   for (LookupResult::iterator P = Previous.begin(), PEnd = Previous.end();
@@ -8968,11 +8969,13 @@ DeclResult Sema::ActOnExplicitInstantiat
 QualType Adjusted = adjustCCAndNoReturn(R, Method->getType(),
 /*AdjustExceptionSpec*/true);
 if (Context.hasSameUnqualifiedType(Method->getType(), Adjusted)) {
-  Matches.clear();
-
-  Matches.addDecl(Method, P.getAccess());
-  if (Method->getTemplateSpecializationKind() == TSK_Undeclared)
-break;
+  if (Method->getPrimaryTemplate()) {
+TemplateMatches.addDecl(Method, P.getAccess());
+  } else {
+// FIXME: Can this assert ever happen?  Needs a test.
+assert(!NonTemplateMatch && "Multiple NonTemplateMatches");
+NonTemplateMatch = Method;
+  }
 }
   }
 }
@@ -9011,22 +9014,25 @@ DeclResult Sema::ActOnExplicitInstantiat
   continue;
 }
 
-Matches.addDecl(Specialization, P.getAccess());
+TemplateMatches.addDecl(Specialization, P.getAccess());
   }
 
-  // Find the most specialized function template specialization.
-  UnresolvedSetIterator Result = getMostSpecialized(
-  Matches.begin(), Matches.end(), FailedCandidates,
-  D.getIdentifierLoc(),
-  PDiag(diag::err_explicit_instantiation_not_known) << Name,
-  PDiag(diag::err_explicit_instantiation_ambiguous) << Name,
-  PDiag(diag::note_explicit_instantiation_candidate));
+  FunctionDecl *Specialization = NonTemplateMatch;
+  if (!Specialization) {
+// Find the most specialized function template specialization.
+UnresolvedSetIterator Result = getMostSpecialized(
+TemplateMatches.begin(), TemplateMatches.end(), FailedCandidates,
+D.getIdentifierLoc(),
+PDiag(diag::err_explicit_instantiation_not_known) << Name,
+PDiag(diag::err_explicit_instantiation_ambiguous) << Name,
+PDiag(diag::note_explicit_instantiation_candidate));
 
-  if (Result == Matches.end())
-return true;
+if (Result == TemplateMatches.end())
+  return true;
 
-  // Ignore access control bits, we don't need them for redeclaration checking.
-  FunctionDecl *Specialization = cast(*Result);
+// Ignore access control bits, we don't need them for redeclaration 
checking.
+Specialization = cast(*Result);
+  }
 
   // C++11 [except.spec]p4
   // In an explicit instantiation an exception-specification may be specified,

Modified: cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p5.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p5.cpp?rev=304951&r1=304950&r2=304951&view=diff
==
--- cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p5.cpp (original)
+++ cfe/trunk/test/CXX/temp/temp.decls/temp.mem/p5.cpp Wed Jun  7 18:00:05 2017
@@ -77,3 +77,16 @@ void test_X0(X0 x0, const X0 &x0c) {
   x0.operator float *();
   x0c.operator const char*();
 }
+
+namespace PR14211 {
+template  struct X {
+  void foo(U){}
+  template  void foo(T){}
+
+  template  void bar(T){}
+  void bar(U){}
+};
+
+template void X::foo(int);
+template void X::bar(int);
+}


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

Re: [PATCH] D17143: [Sema] PR25156 Crash when parsing dtor call on incomplete type

[John McCall via cfe-commits]

On Sun, Jun 11, 2017 at 9:26 AM, don hinton  wrote:

> On Sat, Jun 10, 2017 at 8:55 PM John McCall via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> rjmccall accepted this revision.
>> rjmccall added a comment.
>> This revision is now accepted and ready to land.
>>
>> Thanks, looks great.
>
>
> Great, thanks.
>
>>
>>
>> If you're going to be submitting multiple patches, you should really ask
>> for commit access; it's not an arduous process.
>
>
> Sure, I'll look into it this week.
>
> In the meantime, would you mind commiting this one for me?
>

Sure thing, r305169.

John.


>
> Thanks again...
> don
>
>
>>
>>
>> https://reviews.llvm.org/D17143
>>
>>
>>
>>


-- 
I suppose you'd like my real thoughts as well.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r305169 - Don't crash when forming a destructor name on an incomplete type.

[John McCall via cfe-commits]

Author: rjmccall
Date: Sun Jun 11 15:33:00 2017
New Revision: 305169

URL: http://llvm.org/viewvc/llvm-project?rev=305169&view=rev
Log:
Don't crash when forming a destructor name on an incomplete type.

Fixes PR25156.

Patch by Don Hinton!

Modified:
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/nested-name-spec.cpp

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=305169&r1=305168&r2=305169&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Sun Jun 11 15:33:00 2017
@@ -189,12 +189,15 @@ ParsedType Sema::getDestructorName(Sourc
 // have one) and, if that fails to find a match, in the scope (if
 // we're allowed to look there).
 Found.clear();
-if (Step == 0 && LookupCtx)
+if (Step == 0 && LookupCtx) {
+  if (RequireCompleteDeclContext(SS, LookupCtx))
+return nullptr;
   LookupQualifiedName(Found, LookupCtx);
-else if (Step == 1 && LookInScope && S)
+} else if (Step == 1 && LookInScope && S) {
   LookupName(Found, S);
-else
+} else {
   continue;
+}
 
 // FIXME: Should we be suppressing ambiguities here?
 if (Found.isAmbiguous())

Modified: cfe/trunk/test/SemaCXX/nested-name-spec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nested-name-spec.cpp?rev=305169&r1=305168&r2=305169&view=diff
==
--- cfe/trunk/test/SemaCXX/nested-name-spec.cpp (original)
+++ cfe/trunk/test/SemaCXX/nested-name-spec.cpp Sun Jun 11 15:33:00 2017
@@ -169,6 +169,13 @@ void N::f() { } // okay
 struct Y;  // expected-note{{forward declaration of 'Y'}}
 Y::foo y; // expected-error{{incomplete type 'Y' named in nested name 
specifier}}
 
+namespace PR25156 {
+struct Y;  // expected-note{{forward declaration of 'PR25156::Y'}}
+void foo() {
+  Y::~Y(); // expected-error{{incomplete type 'PR25156::Y' named in nested 
name specifier}}
+}
+}
+
 X::X() : a(5) { } // expected-error{{use of undeclared identifier 'X'}}
 
 struct foo_S {


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

r320721 - In an ARC lambda-to-block conversion thunk, reclaim the return value of

[John McCall via cfe-commits]

Author: rjmccall
Date: Thu Dec 14 10:21:14 2017
New Revision: 320721

URL: http://llvm.org/viewvc/llvm-project?rev=320721&view=rev
Log:
In an ARC lambda-to-block conversion thunk, reclaim the return value of
the lambda so that we don't over-release it.

Patch by Dan Zimmerman!

Added:
cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=320721&r1=320720&r2=320721&view=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Dec 14 10:21:14 2017
@@ -2777,9 +2777,12 @@ void CodeGenFunction::EmitForwardingCall
   RValue RV = EmitCall(calleeFnInfo, callee, returnSlot, callArgs);
 
   // If necessary, copy the returned value into the slot.
-  if (!resultType->isVoidType() && returnSlot.isNull())
+  if (!resultType->isVoidType() && returnSlot.isNull()) {
+if (getLangOpts().ObjCAutoRefCount && resultType->isObjCRetainableType()) {
+  RV = 
RValue::get(EmitARCRetainAutoreleasedReturnValue(RV.getScalarVal()));
+}
 EmitReturnOfRValue(RV, resultType);
-  else
+  } else
 EmitBranchThroughCleanup(ReturnBlock);
 }
 

Added: cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm?rev=320721&view=auto
==
--- cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm (added)
+++ cfe/trunk/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm Thu Dec 14 
10:21:14 2017
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -emit-llvm 
-disable-llvm-passes -O3 -fblocks -fobjc-arc -fobjc-runtime-has-weak -std=c++11 
-o - %s | FileCheck %s
+
+void test0(id x) {
+  extern void test0_helper(id (^)(void));
+  test0_helper([=]() { return x; });
+  // CHECK-LABEL: define internal i8* @___Z5test0P11objc_object_block_invoke
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test0P11objc_objectENK3$_0clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}
+
+id test1_rv;
+
+void test1() {
+  extern void test1_helper(id (*)(void));
+  test1_helper([](){ return test1_rv; });
+  // CHECK-LABEL: define internal i8* @"_ZZ5test1vEN3$_18__invokeEv"
+  // CHECK: [[T0:%.*]] = call i8* @"_ZZ5test1vENK3$_1clEv"
+  // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* 
[[T0]])
+  // CHECK-NEXT: [[T2:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* 
[[T1]])
+  // CHECK-NEXT: ret i8* [[T2]]
+}


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

trivial_abi

[John McCall via cfe-commits]

Hey, Richard et al.  Akira and I were talking about the right ABI rule for 
deciding can-pass-in-registers-ness for structs in the presence of trivial_abi, 
and I think I like Akira's approach but wanted to get your input.

The current definition in Itanium is:

  non-trivial for the purposes of calls <>
 <>
A type is considered non-trivial for the purposes of calls if:

it has a non-trivial copy constructor, move constructor, or destructor, or
all of its copy and move constructors are deleted.
 <>

I'd suggest modifying this to:

A type is considered non-trivial for the purposes of calls if:
- if has a copy constructor, move constructor, or destructor 
which is non-trivial for the purposes of calls, or
- all of its copy and move constructors are deleted and it does 
not have the trivial_abi attribute.

A copy/move constructor is considered trivial for the purposes of calls 
if:
- it is user-provided and
- the class has the trivial_abi attribute and
- a defaulted definition of the constructor would be 
trivial for the purposes of calls; or
- it is not user-provided and
- the class has no virtual functions and no virtual 
base classes, and
- the constructor used to copy/move each direct base 
class subobject is trivial for the purposes of calls, and
- for each non-static data member that is of class type 
(or array thereof), the constructor selected to copy/move that member is 
trivial for the purposes of calls.

A destructor is considered trivial for the purposes of calls if:
- it is not user-provided or the class has the trivial_abi 
attribute, and
- the destructor is not virtual, and
- all of the direct base classes of its class have destructors 
that are trivial for the purposes of calls, and
- for all of the non-static data members of its class that are 
of class type (or array thereof), each such class is trivial for the purposes 
of calls.

These definitions are intended to follow [class.copy.ctor]p11 and 
[class.dtor]p6 except for the special rules applicable to trivial_abi classes.

I'm not sure about the "defaulted definition" rule for copy/move constructors 
in trivial_abi classes.  The intent is to allow class temploids with 
trivial_abi that are instantiated to contain non-trivial classes to just 
silently become non-trivial.  I was thinking at first that it would be nice to 
have a general rule that trivial_abi classes only contain trivial_abi 
subobjects, but unfortunately that's not consistent with the standard 
triviality rule in some silly corner cases: a trivially-copyable class can have 
a non-trivially-copyable subobject if it happens to copy that subobject with a 
trivial copy constructor.  I couldn't think of a better way of capturing this 
than the "defaulted definition" rule.  I considered using the actual 
initializers used by the constructor, but that would introduce a lot of new 
complexity: suddenly we'd be asking about triviality for an arbitrary 
constructor, and copy/move elision make the question somewhat ambiguous anyway.

I'm also not sure about the right rules about virtual methods.  Should we allow 
polymorphic classes to be made trivial by application of the attribute?

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

Re: trivial_abi

[John McCall via cfe-commits]

> On Jan 2, 2018, at 9:15 PM, Akira Hatanaka  wrote:
> 
> 
> 
>> On Jan 2, 2018, at 4:56 PM, Richard Smith via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>> On 2 January 2018 at 15:33, John McCall via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> Hey, Richard et al.  Akira and I were talking about the right ABI rule for 
>> deciding can-pass-in-registers-ness for structs in the presence of 
>> trivial_abi, and I think I like Akira's approach but wanted to get your 
>> input.
>> 
>> The current definition in Itanium is:
>> 
>>   non-trivial for the purposes of calls <>
>>  <>
>> A type is considered non-trivial for the purposes of calls if:
>> 
>> it has a non-trivial copy constructor, move constructor, or destructor, or
>>  <>
>> I'm assuming we're implicitly excluding deleted functions here. (I'd prefer 
>> to make that explicit; this has been the source of a number of ABI 
>> mismatches.)
>> all of its copy and move constructors are deleted.
>>  <>
>> 
>> I'd suggest modifying this to:
>> 
>>  A type is considered non-trivial for the purposes of calls if:
>>  - if has a copy constructor, move constructor, or destructor 
>> which is non-trivial for the purposes of calls, or
>>  - all of its copy and move constructors are deleted and it does 
>> not have the trivial_abi attribute.
>> 
>>  A copy/move constructor is considered trivial for the purposes of calls 
>> if:
>>  - it is user-provided and
>>  - the class has the trivial_abi attribute and
>>  - a defaulted definition of the constructor would be 
>> trivial for the purposes of calls; or
>> 
>> We'd need to say what happens if the function in question cannot validly be 
>> defaulted for any of the reasons in [dcl.fct.def.default]. Do we try to 
>> infer whether it's a copy or move constructor, and use the rules for a 
>> defaulted copy or move constructor? Or do we just say that's never trivial 
>> for the purposes of calls? Or something else? Eg:
>> 
>> struct [[clang::trivial_abi]] A {
>>   A(A && = make());
>> };
>> 
>> Here, A::A(A&&) cannot validly be defaulted. Is A trivial for the purpose of 
>> calls? Likewise:
>> 
>> struct [[clang::trivial_abi]] B {
>>   B(...);
>> };
>> struct C {
>>   volatile B b;
>> };
>> 
>> Here, C's copy constructor calls B::B(...). Is C trivial for the purpose of 
>> calls? (OK, Clang crashes on that example today. But still...)
>> 
>> I'd be uncomfortable making the rules in [dcl.fct.def.default] part of the 
>> ABI; they seem to be changing relatively frequently. Perhaps we could say 
>> "if the function is a copy constructor ([class.copy.ctor]/1), then consider 
>> what an implicitly-declared defaulted copy constructor would do; if it's a 
>> move constructor ([class.copy.ctor]/2), then consider what an 
>> implicitly-declared defaulted move constructor would do; otherwise, it's not 
>> trivial for the purpose of calls". That'd mean A is trivial for the purpose 
>> of calls and C is not, which I think is probably the right answer.
>> 
>>  - it is not user-provided and
>>  - the class has no virtual functions and no virtual 
>> base classes, and
>>  - the constructor used to copy/move each direct base 
>> class subobject is trivial for the purposes of calls, and
>>  - for each non-static data member that is of class type 
>> (or array thereof), the constructor selected to copy/move that member is 
>> trivial for the purposes of calls.
>> 
>>  A destructor is considered trivial for the purposes of calls if:
>>  - it is not user-provided or the class has the trivial_abi 
>> attribute, and
>>  - the destructor is not virtual, and
>>  - all of the direct base classes of its class have destructors 
>> that are trivial for the purposes of calls, and
>>  - for all of the non-static data members of its class that are 
>> of class type (or array thereof), each such class is trivial for the 
>> purposes of calls.
>> 
>>  These definitions are intended to follow [class.copy.ctor]p11 and 
>> [class.dtor]p6 except for the special rules applicable to trivial_abi 
>> classes.
>> 
>> If 

Re: trivial_abi

[John McCall via cfe-commits]

> On Jan 2, 2018, at 10:43 PM, Richard Smith  wrote:
> 
> On 2 January 2018 at 19:02, John McCall via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> 
>> On Jan 2, 2018, at 9:15 PM, Akira Hatanaka > <mailto:ahatan...@apple.com>> wrote:
>> 
>> 
>> 
>>> On Jan 2, 2018, at 4:56 PM, Richard Smith via cfe-commits 
>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>> 
>>> On 2 January 2018 at 15:33, John McCall via cfe-commits 
>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>> Hey, Richard et al.  Akira and I were talking about the right ABI rule for 
>>> deciding can-pass-in-registers-ness for structs in the presence of 
>>> trivial_abi, and I think I like Akira's approach but wanted to get your 
>>> input.
>>> 
>>> The current definition in Itanium is:
>>> 
>>>   non-trivial for the purposes of calls <>
>>>  <>
>>> A type is considered non-trivial for the purposes of calls if:
>>> 
>>> it has a non-trivial copy constructor, move constructor, or destructor, or
>>>  <>
>>> I'm assuming we're implicitly excluding deleted functions here. (I'd prefer 
>>> to make that explicit; this has been the source of a number of ABI 
>>> mismatches.)
>>> all of its copy and move constructors are deleted.
>>>  <>
>>> 
>>> I'd suggest modifying this to:
>>> 
>>> A type is considered non-trivial for the purposes of calls if:
>>> - if has a copy constructor, move constructor, or destructor 
>>> which is non-trivial for the purposes of calls, or
>>> - all of its copy and move constructors are deleted and it does 
>>> not have the trivial_abi attribute.
>>> 
>>> A copy/move constructor is considered trivial for the purposes of calls 
>>> if:
>>> - it is user-provided and
>>> - the class has the trivial_abi attribute and
>>> - a defaulted definition of the constructor would be 
>>> trivial for the purposes of calls; or
>>> 
>>> We'd need to say what happens if the function in question cannot validly be 
>>> defaulted for any of the reasons in [dcl.fct.def.default]. Do we try to 
>>> infer whether it's a copy or move constructor, and use the rules for a 
>>> defaulted copy or move constructor? Or do we just say that's never trivial 
>>> for the purposes of calls? Or something else? Eg:
>>> 
>>> struct [[clang::trivial_abi]] A {
>>>   A(A && = make());
>>> };
>>> 
>>> Here, A::A(A&&) cannot validly be defaulted. Is A trivial for the purpose 
>>> of calls? Likewise:
>>> 
>>> struct [[clang::trivial_abi]] B {
>>>   B(...);
>>> };
>>> struct C {
>>>   volatile B b;
>>> };
>>> 
>>> Here, C's copy constructor calls B::B(...). Is C trivial for the purpose of 
>>> calls? (OK, Clang crashes on that example today. But still...)
>>> 
>>> I'd be uncomfortable making the rules in [dcl.fct.def.default] part of the 
>>> ABI; they seem to be changing relatively frequently. Perhaps we could say 
>>> "if the function is a copy constructor ([class.copy.ctor]/1), then consider 
>>> what an implicitly-declared defaulted copy constructor would do; if it's a 
>>> move constructor ([class.copy.ctor]/2), then consider what an 
>>> implicitly-declared defaulted move constructor would do; otherwise, it's 
>>> not trivial for the purpose of calls". That'd mean A is trivial for the 
>>> purpose of calls and C is not, which I think is probably the right answer.
>>> 
>>> - it is not user-provided and
>>> - the class has no virtual functions and no virtual 
>>> base classes, and
>>> - the constructor used to copy/move each direct base 
>>> class subobject is trivial for the purposes of calls, and
>>> - for each non-static data member that is of class type 
>>> (or array thereof), the constructor selected to copy/move that member is 
>>> trivial for the purposes of calls.
>>> 
>>> A destructor is considered trivial for the purposes of calls if:
>>> - it is not user-provided or the class has the trivial_abi 
>>> attribute, and
>>> - the destructor is n

Re: [PATCH] D41039: Add support for attribute "trivial_abi"

[John McCall via cfe-commits]

On Wed, Jan 3, 2018 at 12:24 PM, Akira Hatanaka  wrote:

> On Jan 2, 2018, at 9:42 AM, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>
>
> On Tue, Dec 19, 2017 at 9:43 PM Akira Hatanaka  wrote:
>
>> On Tue, Dec 12, 2017 at 12:12 PM, John McCall  wrote:
>>
>>> On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie  w
>>> rote:
>>>
 On Mon, Dec 11, 2017 at 5:38 PM John McCall  wrote:

> On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie  w
> rote:
>
>> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> rjmccall added a comment.
>>>
>>> In https://reviews.llvm.org/D41039#951648, @ahatanak wrote:
>>>
>>> > I had a discussion with Duncan today and he pointed out that
>>> perhaps we shouldn't allow users to annotate a struct with 
>>> "trivial_abi" if
>>> one of its subobjects is non-trivial and is not annotated with
>>> "trivial_abi" since that gives users too much power.
>>> >
>>> > Should we error out or drop "trivial_abi" from struct Outer when
>>> the following code is compiled?
>>> >
>>> >   struct Inner1 {
>>> > ~Inner1(); // non-trivial
>>> > int x;
>>> >   };
>>> >
>>> >   struct __attribute__((trivial_abi)) Outer {
>>> > ~Outer();
>>> > Inner1 x;
>>> >   };
>>> >
>>> >
>>> > The current patch doesn't error out or drop the attribute, but the
>>> patch would probably be much simpler if we didn't allow it.
>>>
>>>
>>> I think it makes sense to emit an error if there is provably a
>>> non-trivial-ABI component.  However, for class temploids I think that
>>> diagnostic should only fire on the definition, not on instantiations; 
>>> for
>>> example:
>>>
>>>   template  struct __attribute__((trivial_abi)) holder {
>>>  T value;
>>>  ~holder() {}
>>>   };
>>>   holder hs; // this instantiation should be legal
>>> despite the fact that holder cannot be trivial-ABI.
>>>
>>> But we should still be able to emit the diagnostic in template
>>> definitions, e.g.:
>>>
>>>   template  struct __attribute__((trivial_abi))
>>> named_holder {
>>>  std::string name; // there are no instantiations of this
>>> template that could ever be trivial-ABI
>>>  T value;
>>>  ~named_holder() {}
>>>   };
>>>
>>> The wording should be something akin to the standard template rule
>>> that a template is ill-formed if it has no valid instantiations, no
>>> diagnostic required.
>>>
>>> I would definitely like to open the conversation about the name of
>>> the attribute.  I don't think we've used "abi" in an existing attribute
>>> name; usually it's more descriptive.  And "trivial" is a weighty word in
>>> the standard.  I'm not sure I have a great counter-proposal off the top 
>>> of
>>> my head, though.
>>>
>>
>> Agreed on both counts (would love a better name, don't have any
>> stand-out candidates off the top of my head).
>>
>> I feel like a more descriptive term about the property of the object
>> would make me happier - something like "address_independent_identity"
>> (s/identity/value/?) though, yeah, that's not spectacular by any stretch.
>>
>
> Incidentally, your comments are not showing up on Phabricator for some
> reason.
>

 Yeah, Phab doesn't do a great job looking on the mailing list for
 interesting replies - I forget, maybe it only catches bottom post or top
 post but not infix replies or something...

>>>
>>> Well, fortunately the mailing list is also archived. :)
>>>
>>>
 The term "trivially movable" suggests itself, with two caveats:
>   - What we're talking about is trivial *destructive* movability,
> i.e. that the combination of moving the value to a new object and not
> destroying the old object can be done trivially, which is not quite the
> same as trivial movability in the normal C++ sense, which I guess could be
> a property that someone theoretically might care about (if the type is
> trivially destructed, but it isn't copyable for semantic reasons?).
>   - Trivial destructive movability is a really common property, and
> it's something that a compiler would really like to optimize based on even
> in cases where trivial_abi can't be adopted for binary-compatibility
> reasons.  Stealing the term for the stronger property that the type is
> trivially destructively movable *and its ABI should reflect that in a
> specific way* would be unfortunate.
>

 Fair point.


> "trivially_movable" is a long attribute anyway, and
> "trivially_destructively_movable" is even worse.
>
> Maybe that second point is telling us that this isn't purely
> descriptive — it's inherently talking about the

Re: trivial_abi

[John McCall via cfe-commits]

> On Jan 3, 2018, at 5:12 PM, Richard Smith  wrote:
> 
> On 2 January 2018 at 20:55, John McCall via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
>> On Jan 2, 2018, at 10:43 PM, Richard Smith > <mailto:rich...@metafoo.co.uk>> wrote:
>> 
>> On 2 January 2018 at 19:02, John McCall via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>>> On Jan 2, 2018, at 9:15 PM, Akira Hatanaka >> <mailto:ahatan...@apple.com>> wrote:
>>> 
>>> 
>>> 
>>>> On Jan 2, 2018, at 4:56 PM, Richard Smith via cfe-commits 
>>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>> 
>>>> On 2 January 2018 at 15:33, John McCall via cfe-commits 
>>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>> Hey, Richard et al.  Akira and I were talking about the right ABI rule for 
>>>> deciding can-pass-in-registers-ness for structs in the presence of 
>>>> trivial_abi, and I think I like Akira's approach but wanted to get your 
>>>> input.
>>>> 
>>>> The current definition in Itanium is:
>>>> 
>>>>   non-trivial for the purposes of calls <>
>>>>  <>
>>>> A type is considered non-trivial for the purposes of calls if:
>>>> 
>>>> it has a non-trivial copy constructor, move constructor, or destructor, or
>>>>  <>
>>>> I'm assuming we're implicitly excluding deleted functions here. (I'd 
>>>> prefer to make that explicit; this has been the source of a number of ABI 
>>>> mismatches.)
>>>> all of its copy and move constructors are deleted.
>>>>  <>
>>>> 
>>>> I'd suggest modifying this to:
>>>> 
>>>>A type is considered non-trivial for the purposes of calls if:
>>>>- if has a copy constructor, move constructor, or destructor 
>>>> which is non-trivial for the purposes of calls, or
>>>>- all of its copy and move constructors are deleted and it does 
>>>> not have the trivial_abi attribute.
>>>> 
>>>>A copy/move constructor is considered trivial for the purposes of calls 
>>>> if:
>>>>- it is user-provided and
>>>>- the class has the trivial_abi attribute and
>>>>- a defaulted definition of the constructor would be 
>>>> trivial for the purposes of calls; or
>>>> 
>>>> We'd need to say what happens if the function in question cannot validly 
>>>> be defaulted for any of the reasons in [dcl.fct.def.default]. Do we try to 
>>>> infer whether it's a copy or move constructor, and use the rules for a 
>>>> defaulted copy or move constructor? Or do we just say that's never trivial 
>>>> for the purposes of calls? Or something else? Eg:
>>>> 
>>>> struct [[clang::trivial_abi]] A {
>>>>   A(A && = make());
>>>> };
>>>> 
>>>> Here, A::A(A&&) cannot validly be defaulted. Is A trivial for the purpose 
>>>> of calls? Likewise:
>>>> 
>>>> struct [[clang::trivial_abi]] B {
>>>>   B(...);
>>>> };
>>>> struct C {
>>>>   volatile B b;
>>>> };
>>>> 
>>>> Here, C's copy constructor calls B::B(...). Is C trivial for the purpose 
>>>> of calls? (OK, Clang crashes on that example today. But still...)
>>>> 
>>>> I'd be uncomfortable making the rules in [dcl.fct.def.default] part of the 
>>>> ABI; they seem to be changing relatively frequently. Perhaps we could say 
>>>> "if the function is a copy constructor ([class.copy.ctor]/1), then 
>>>> consider what an implicitly-declared defaulted copy constructor would do; 
>>>> if it's a move constructor ([class.copy.ctor]/2), then consider what an 
>>>> implicitly-declared defaulted move constructor would do; otherwise, it's 
>>>> not trivial for the purpose of calls". That'd mean A is trivial for the 
>>>> purpose of calls and C is not, which I think is probably the right answer.
>>>> 
>>>>- it is not user-provided and
>>>>- the class has no virtual functions and no virtual 
>>>> base classes, and
>>>>- the constructor used to copy/move eac

Re: trivial_abi

[John McCall via cfe-commits]

> On Jan 3, 2018, at 5:53 PM, Richard Smith  wrote:
> On 3 January 2018 at 14:29, John McCall via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> 
>> On Jan 3, 2018, at 5:12 PM, Richard Smith > <mailto:rich...@metafoo.co.uk>> wrote:
>> 
>> On 2 January 2018 at 20:55, John McCall via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>> On Jan 2, 2018, at 10:43 PM, Richard Smith >> <mailto:rich...@metafoo.co.uk>> wrote:
>>> 
>>> On 2 January 2018 at 19:02, John McCall via cfe-commits 
>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>> 
>>>> On Jan 2, 2018, at 9:15 PM, Akira Hatanaka >>> <mailto:ahatan...@apple.com>> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Jan 2, 2018, at 4:56 PM, Richard Smith via cfe-commits 
>>>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>>> 
>>>>> On 2 January 2018 at 15:33, John McCall via cfe-commits 
>>>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>>> Hey, Richard et al.  Akira and I were talking about the right ABI rule 
>>>>> for deciding can-pass-in-registers-ness for structs in the presence of 
>>>>> trivial_abi, and I think I like Akira's approach but wanted to get your 
>>>>> input.
>>>>> 
>>>>> The current definition in Itanium is:
>>>>> 
>>>>>   non-trivial for the purposes of calls <>
>>>>>  <>
>>>>> A type is considered non-trivial for the purposes of calls if:
>>>>> 
>>>>> it has a non-trivial copy constructor, move constructor, or destructor, or
>>>>>  <>
>>>>> I'm assuming we're implicitly excluding deleted functions here. (I'd 
>>>>> prefer to make that explicit; this has been the source of a number of ABI 
>>>>> mismatches.)
>>>>> all of its copy and move constructors are deleted.
>>>>>  <>
>>>>> 
>>>>> I'd suggest modifying this to:
>>>>> 
>>>>>   A type is considered non-trivial for the purposes of calls if:
>>>>>   - if has a copy constructor, move constructor, or destructor 
>>>>> which is non-trivial for the purposes of calls, or
>>>>>   - all of its copy and move constructors are deleted and it does 
>>>>> not have the trivial_abi attribute.
>>>>> 
>>>>>   A copy/move constructor is considered trivial for the purposes of calls 
>>>>> if:
>>>>>   - it is user-provided and
>>>>>   - the class has the trivial_abi attribute and
>>>>>   - a defaulted definition of the constructor would be 
>>>>> trivial for the purposes of calls; or
>>>>> 
>>>>> We'd need to say what happens if the function in question cannot validly 
>>>>> be defaulted for any of the reasons in [dcl.fct.def.default]. Do we try 
>>>>> to infer whether it's a copy or move constructor, and use the rules for a 
>>>>> defaulted copy or move constructor? Or do we just say that's never 
>>>>> trivial for the purposes of calls? Or something else? Eg:
>>>>> 
>>>>> struct [[clang::trivial_abi]] A {
>>>>>   A(A && = make());
>>>>> };
>>>>> 
>>>>> Here, A::A(A&&) cannot validly be defaulted. Is A trivial for the purpose 
>>>>> of calls? Likewise:
>>>>> 
>>>>> struct [[clang::trivial_abi]] B {
>>>>>   B(...);
>>>>> };
>>>>> struct C {
>>>>>   volatile B b;
>>>>> };
>>>>> 
>>>>> Here, C's copy constructor calls B::B(...). Is C trivial for the purpose 
>>>>> of calls? (OK, Clang crashes on that example today. But still...)
>>>>> 
>>>>> I'd be uncomfortable making the rules in [dcl.fct.def.default] part of 
>>>>> the ABI; they seem to be changing relatively frequently. Perhaps we could 
>>>>> say "if the function is a copy constructor ([class.copy.ctor]/1), then 
>>>>> consider what an implicitly-declared defaulted copy constructor would do; 
>>>>> if it's a move constructor ([class.copy.ctor]/2), then consider what an 
>>>>> implicit

Re: [PATCH] D41039: Add support for attribute "trivial_abi"

[John McCall via cfe-commits]

On Wed, Jan 3, 2018 at 2:07 PM, Akira Hatanaka  wrote:

> On Jan 3, 2018, at 10:25 AM, John McCall  wrote:
>
> On Wed, Jan 3, 2018 at 12:24 PM, Akira Hatanaka 
> wrote:
>
>> On Jan 2, 2018, at 9:42 AM, David Blaikie via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>
>>
>> On Tue, Dec 19, 2017 at 9:43 PM Akira Hatanaka 
>> wrote:
>>
>>> On Tue, Dec 12, 2017 at 12:12 PM, John McCall  wr
>>> ote:
>>>
 On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie  w
 rote:

> On Mon, Dec 11, 2017 at 5:38 PM John McCall 
> wrote:
>
>> On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie  w
>> rote:
>>
>>> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>
 rjmccall added a comment.

 In https://reviews.llvm.org/D41039#951648, @ahatanak wrote:

 > I had a discussion with Duncan today and he pointed out that
 perhaps we shouldn't allow users to annotate a struct with 
 "trivial_abi" if
 one of its subobjects is non-trivial and is not annotated with
 "trivial_abi" since that gives users too much power.
 >
 > Should we error out or drop "trivial_abi" from struct Outer when
 the following code is compiled?
 >
 >   struct Inner1 {
 > ~Inner1(); // non-trivial
 > int x;
 >   };
 >
 >   struct __attribute__((trivial_abi)) Outer {
 > ~Outer();
 > Inner1 x;
 >   };
 >
 >
 > The current patch doesn't error out or drop the attribute, but
 the patch would probably be much simpler if we didn't allow it.


 I think it makes sense to emit an error if there is provably a
 non-trivial-ABI component.  However, for class temploids I think that
 diagnostic should only fire on the definition, not on instantiations; 
 for
 example:

   template  struct __attribute__((trivial_abi)) holder {
  T value;
  ~holder() {}
   };
   holder hs; // this instantiation should be legal
 despite the fact that holder cannot be trivial-ABI.

 But we should still be able to emit the diagnostic in template
 definitions, e.g.:

   template  struct __attribute__((trivial_abi))
 named_holder {
  std::string name; // there are no instantiations of this
 template that could ever be trivial-ABI
  T value;
  ~named_holder() {}
   };

 The wording should be something akin to the standard template rule
 that a template is ill-formed if it has no valid instantiations, no
 diagnostic required.

 I would definitely like to open the conversation about the name of
 the attribute.  I don't think we've used "abi" in an existing attribute
 name; usually it's more descriptive.  And "trivial" is a weighty word 
 in
 the standard.  I'm not sure I have a great counter-proposal off the 
 top of
 my head, though.

>>>
>>> Agreed on both counts (would love a better name, don't have any
>>> stand-out candidates off the top of my head).
>>>
>>> I feel like a more descriptive term about the property of the object
>>> would make me happier - something like "address_independent_identity"
>>> (s/identity/value/?) though, yeah, that's not spectacular by any 
>>> stretch.
>>>
>>
>> Incidentally, your comments are not showing up on Phabricator for
>> some reason.
>>
>
> Yeah, Phab doesn't do a great job looking on the mailing list for
> interesting replies - I forget, maybe it only catches bottom post or top
> post but not infix replies or something...
>

 Well, fortunately the mailing list is also archived. :)


> The term "trivially movable" suggests itself, with two caveats:
>>   - What we're talking about is trivial *destructive* movability,
>> i.e. that the combination of moving the value to a new object and not
>> destroying the old object can be done trivially, which is not quite the
>> same as trivial movability in the normal C++ sense, which I guess could 
>> be
>> a property that someone theoretically might care about (if the type is
>> trivially destructed, but it isn't copyable for semantic reasons?).
>>   - Trivial destructive movability is a really common property, and
>> it's something that a compiler would really like to optimize based on 
>> even
>> in cases where trivial_abi can't be adopted for binary-compatibility
>> reasons.  Stealing the term for the stronger property that the type is
>> trivially destructively movable *and its ABI should reflect that in a
>> specific way* would be unfortunat

r321957 - Simplify the internal API for checking whether swiftcall passes a type indirectly and expose that API externally.

[John McCall via cfe-commits]

Author: rjmccall
Date: Sat Jan  6 22:28:49 2018
New Revision: 321957

URL: http://llvm.org/viewvc/llvm-project?rev=321957&view=rev
Log:
Simplify the internal API for checking whether swiftcall passes a type 
indirectly and expose that API externally.

Modified:
cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h
cfe/trunk/lib/CodeGen/ABIInfo.h
cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h?rev=321957&r1=321956&r2=321957&view=diff
==
--- cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h (original)
+++ cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h Sat Jan  6 22:28:49 2018
@@ -116,6 +116,12 @@ private:
   void splitVectorEntry(unsigned index);
 };
 
+/// Should an aggregate which expands to the given type sequence
+/// be passed/returned indirectly under swiftcall?
+bool shouldPassIndirectly(CodeGenModule &CGM,
+  ArrayRef types,
+  bool asReturnValue);
+
 /// Return the maximum voluntary integer size for the current target.
 CharUnits getMaximumVoluntaryIntegerSize(CodeGenModule &CGM);
 

Modified: cfe/trunk/lib/CodeGen/ABIInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ABIInfo.h?rev=321957&r1=321956&r2=321957&view=diff
==
--- cfe/trunk/lib/CodeGen/ABIInfo.h (original)
+++ cfe/trunk/lib/CodeGen/ABIInfo.h Sat Jan  6 22:28:49 2018
@@ -137,8 +137,7 @@ namespace swiftcall {
 
 bool supportsSwift() const final override { return true; }
 
-virtual bool shouldPassIndirectlyForSwift(CharUnits totalSize,
-  ArrayRef types,
+virtual bool shouldPassIndirectlyForSwift(ArrayRef types,
   bool asReturnValue) const = 0;
 
 virtual bool isLegalVectorTypeForSwift(CharUnits totalSize,

Modified: cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp?rev=321957&r1=321956&r2=321957&view=diff
==
--- cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp (original)
+++ cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp Sat Jan  6 22:28:49 2018
@@ -579,11 +579,9 @@ bool SwiftAggLowering::shouldPassIndirec
   // Empty types don't need to be passed indirectly.
   if (Entries.empty()) return false;
 
-  CharUnits totalSize = Entries.back().End;
-
   // Avoid copying the array of types when there's just a single element.
   if (Entries.size() == 1) {
-return getSwiftABIInfo(CGM).shouldPassIndirectlyForSwift(totalSize,
+return getSwiftABIInfo(CGM).shouldPassIndirectlyForSwift(
Entries.back().Type,
  asReturnValue);   
 
   }
@@ -593,8 +591,14 @@ bool SwiftAggLowering::shouldPassIndirec
   for (auto &entry : Entries) {
 componentTys.push_back(entry.Type);
   }
-  return getSwiftABIInfo(CGM).shouldPassIndirectlyForSwift(totalSize,
-   componentTys,
+  return getSwiftABIInfo(CGM).shouldPassIndirectlyForSwift(componentTys,
+   asReturnValue);
+}
+
+bool swiftcall::shouldPassIndirectly(CodeGenModule &CGM,
+ ArrayRef componentTys,
+ bool asReturnValue) {
+  return getSwiftABIInfo(CGM).shouldPassIndirectlyForSwift(componentTys,
asReturnValue);
 }
 

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=321957&r1=321956&r2=321957&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Sat Jan  6 22:28:49 2018
@@ -1028,8 +1028,7 @@ public:
   IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
   DefaultNumRegisterParameters(NumRegisterParameters) {}
 
-  bool shouldPassIndirectlyForSwift(CharUnits totalSize,
-ArrayRef scalars,
+  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
 bool asReturnValue) const override {
 // LLVM's x86-32 lowering currently only assigns up to three
 // integer registers and three fp registers.  Oddly, it'll use up to
@@ -2168,8 +2167,7 @@ public:
 return Has64BitPointers;
   }
 
-  bool shouldPassIndirectlyForSwift(CharUnits totalSize,
-ArrayRef scalars,
+  bool shouldP

Re: trivial_abi

[John McCall via cfe-commits]

> On Jan 8, 2018, at 10:57 AM, David Blaikie  wrote:
> (just a side note: perhaps this conversation would've been more suited to 
> cfe-dev? I sort of missed it because I only check commits once a week, unless 
> I'm specifically cc'd on something. All good though :))

I suppose it's more design-and-development than just code review, so yes, 
perhaps cfe-dev would have been more appropriate.  Regardless I should've made 
a point of CC'ing you, since you'd been part of the ongoing conversation.  
Apologies.

John.

> 
> On Wed, Jan 3, 2018 at 4:06 PM Richard Smith via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> On 3 January 2018 at 15:24, John McCall via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
>> On Jan 3, 2018, at 5:53 PM, Richard Smith > <mailto:rich...@metafoo.co.uk>> wrote:
>> On 3 January 2018 at 14:29, John McCall via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>>> On Jan 3, 2018, at 5:12 PM, Richard Smith >> <mailto:rich...@metafoo.co.uk>> wrote:
>>> 
>>> On 2 January 2018 at 20:55, John McCall via cfe-commits 
>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>> On Jan 2, 2018, at 10:43 PM, Richard Smith >>> <mailto:rich...@metafoo.co.uk>> wrote:
>>>> 
>>>> On 2 January 2018 at 19:02, John McCall via cfe-commits 
>>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>> 
>>>>> On Jan 2, 2018, at 9:15 PM, Akira Hatanaka >>>> <mailto:ahatan...@apple.com>> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Jan 2, 2018, at 4:56 PM, Richard Smith via cfe-commits 
>>>>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>>>> 
>>>>>> On 2 January 2018 at 15:33, John McCall via cfe-commits 
>>>>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>>>>> Hey, Richard et al.  Akira and I were talking about the right ABI rule 
>>>>>> for deciding can-pass-in-registers-ness for structs in the presence of 
>>>>>> trivial_abi, and I think I like Akira's approach but wanted to get your 
>>>>>> input.
>>>>>> 
>>>>>> The current definition in Itanium is:
>>>>>> 
>>>>>>   non-trivial for the purposes of calls <>
>>>>>>  <>
>>>>>> A type is considered non-trivial for the purposes of calls if:
>>>>>> 
>>>>>> it has a non-trivial copy constructor, move constructor, or destructor, 
>>>>>> or
>>>>>>  <>
>>>>>> I'm assuming we're implicitly excluding deleted functions here. (I'd 
>>>>>> prefer to make that explicit; this has been the source of a number of 
>>>>>> ABI mismatches.)
>>>>>> all of its copy and move constructors are deleted.
>>>>>>  <>
>>>>>> 
>>>>>> I'd suggest modifying this to:
>>>>>> 
>>>>>>  A type is considered non-trivial for the purposes of calls if:
>>>>>>  - if has a copy constructor, move constructor, or destructor 
>>>>>> which is non-trivial for the purposes of calls, or
>>>>>>  - all of its copy and move constructors are deleted and it does 
>>>>>> not have the trivial_abi attribute.
>>>>>> 
>>>>>>  A copy/move constructor is considered trivial for the purposes of calls 
>>>>>> if:
>>>>>>  - it is user-provided and
>>>>>>  - the class has the trivial_abi attribute and
>>>>>>  - a defaulted definition of the constructor would be 
>>>>>> trivial for the purposes of calls; or
>>>>>> 
>>>>>> We'd need to say what happens if the function in question cannot validly 
>>>>>> be defaulted for any of the reasons in [dcl.fct.def.default]. Do we try 
>>>>>> to infer whether it's a copy or move constructor, and use the rules for 
>>>>>> a defaulted copy or move constructor? Or do we just say that's never 
>>>>>> trivial for the purposes of calls? Or something else? Eg:
>>>>>> 
>>>>>> struct [[clang::trivial_abi]] A {
>>>>>>   A(A && = make());
>>>>>>

Re: r326946 - CodeGen: Fix address space of indirect function argument

[John McCall via cfe-commits]

> On Mar 9, 2018, at 8:51 PM, Richard Smith  wrote:
> 
> Hi,
> 
> This change increases the size of a CallArg, and thus that of a CallArgList, 
> dramatically (an LValue is *much* larger than an RValue, and unlike an 
> RValue, does not appear to be at all optimized for size). This results in 
> CallArgList (which contains inline storage for 16 CallArgs) ballooning in 
> size from ~500 bytes to 2KB, resulting in stack overflows on programs with 
> deep ASTs.
> 
> Given that this introduces a regression (due to stack overflow), I've 
> reverted in r327195. Sorry about that.

Seems reasonable.

The right short-term fix is probably a combination of the following:
  - put a little bit of effort into packing LValue (using fewer than 64 bits 
for the alignment and packing the bit-fields, I think)
  - drop the inline argument count to something like 8, which should still 
capture the vast majority of calls.

There are quite a few other space optimizations we could do the LValue 
afterwards.   I think we could eliminate BaseIVarExp completely by storing some 
sort of ObjC GC classification and then just evaluating the base expression 
normally.

> The program it broke looked something like this:
> 
> struct VectorBuilder {
>   VectorBuilder &operator,(int);
> };
> void f() {
>   VectorBuilder(),
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,
>   
> 1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0;
> }
> 
> using Eigen's somewhat-ridiculous comma initializer technique 
> (https://eigen.tuxfamily.org/dox/group__TutorialAdvancedInitialization.html 
> ) 
> to build an approx 40 x 40 array.

> An AST 1600 levels deep is somewhat extreme, but it still seems like 
> something we should be able to handle within our default 8MB stack limit.

I mean, at some point this really is not supportable, even if it happens to 
build on current compilers.  If we actually documented and enforced our 
implementation limits like we're supposed to, I do not think we would allow 
this much call nesting.

John.

> 
> On 7 March 2018 at 13:45, Yaxun Liu via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: yaxunl
> Date: Wed Mar  7 13:45:40 2018
> New Revision: 326946
> 
> URL: http://llvm

r333791 - Cap "voluntary" vector alignment at 16 for all Darwin platforms.

[John McCall via cfe-commits]

Author: rjmccall
Date: Fri Jun  1 14:34:26 2018
New Revision: 333791

URL: http://llvm.org/viewvc/llvm-project?rev=333791&view=rev
Log:
Cap "voluntary" vector alignment at 16 for all Darwin platforms.

This fixes two major problems:
- We were not capping vector alignment as desired on 32-bit ARM.
- We were using different alignments based on the AVX settings on
  Intel, so we did not have a consistent ABI.

This is an ABI break, but we think we can get away with it because
vectors tend to be used mostly in inline code (which is why not having
a consistent ABI has not proven disastrous on Intel).

Intel's AVX types are specified as having 32-byte / 64-byte alignment,
so align them explicitly instead of relying on the base ABI rule.
Note that this sort of attribute is stripped from template arguments
in template substitution, so there's a possibility that code templated
over vectors will produce inadequately-aligned objects.  The right
long-term solution for this is for alignment attributes to be
interpreted as true qualifiers and thus preserved in the canonical type.

Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avxintrin.h
cfe/trunk/test/CodeGen/arm-swiftcall.c
cfe/trunk/test/CodeGen/vector-alignment.c
cfe/trunk/test/CodeGenCXX/align-avx-complete-objects.cpp

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=333791&r1=333790&r2=333791&view=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Fri Jun  1 14:34:26 2018
@@ -113,6 +113,9 @@ public:
 }
 
 this->MCountName = "\01mcount";
+
+// Cap vector alignment at 16 bytes for all Darwin platforms.
+this->MaxVectorAlign = 128;
   }
 
   std::string isValidSectionSpecifier(StringRef SR) const override {

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=333791&r1=333790&r2=333791&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Fri Jun  1 14:34:26 2018
@@ -421,7 +421,6 @@ public:
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 SuitableAlign = 128;
-MaxVectorAlign = 256;
 // The watchOS simulator uses the builtin bool type for Objective-C.
 llvm::Triple T = llvm::Triple(Triple);
 if (T.isWatchOS())
@@ -437,9 +436,6 @@ public:
 if (!DarwinTargetInfo::handleTargetFeatures(Features,
   Diags))
   return false;
-// We now know the features we have: we can decide how to align vectors.
-MaxVectorAlign =
-hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
 return true;
   }
 };
@@ -802,9 +798,6 @@ public:
 if (!DarwinTargetInfo::handleTargetFeatures(Features,
   Diags))
   return false;
-// We now know the features we have: we can decide how to align vectors.
-MaxVectorAlign =
-hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
 return true;
   }
 };

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=333791&r1=333790&r2=333791&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Jun  1 14:34:26 2018
@@ -8918,18 +8918,20 @@ Value *CodeGenFunction::EmitX86BuiltinEx
   case X86::BI__builtin_ia32_movdqa64store128_mask:
   case X86::BI__builtin_ia32_storeaps128_mask:
   case X86::BI__builtin_ia32_storeapd128_mask:
+return EmitX86MaskedStore(*this, Ops, 16);
+
   case X86::BI__builtin_ia32_movdqa32store256_mask:
   case X86::BI__builtin_ia32_movdqa64store256_mask:
   case X86::BI__builtin_ia32_storeaps256_mask:
   case X86::BI__builtin_ia32_storeapd256_mask:
+return EmitX86MaskedStore(*this, Ops, 32);
+
   case X86::BI__builtin_ia32_movdqa32store512_mask:
   case X86::BI__builtin_ia32_movdqa64store512_mask:
   case X86::BI__builtin_ia32_storeaps512_mask:
-  case X86::BI__builtin_ia32_storeapd512_mask: {
-unsigned Align =
-  getContext().getTypeAlignInChars(E->getArg(1)->getType()).getQuantity();
-return EmitX86MaskedStore(*this, Ops, Align);
-  }
+  case X86::BI__builtin_ia32_storeapd512_mask:
+return EmitX86MaskedStore(*this, Ops, 64);
+
   case X86::BI__builtin_ia32_loadups128_mask:
   case X86::BI__builtin_ia32_loadups256_mask:
   case X86::BI__builtin_ia32_loadups512_mask:
@@ -8950,26 +8952,25 @@ Value *CodeGenFunction::EmitX

r345536 - In swiftcall, don't merge FP/vector types within a chunk.

[John McCall via cfe-commits]

Author: rjmccall
Date: Mon Oct 29 13:32:36 2018
New Revision: 345536

URL: http://llvm.org/viewvc/llvm-project?rev=345536&view=rev
Log:
In swiftcall, don't merge FP/vector types within a chunk.

Modified:
cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h
cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
cfe/trunk/test/CodeGen/64bit-swiftcall.c
cfe/trunk/test/CodeGen/windows-swiftcall.c

Modified: cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h?rev=345536&r1=345535&r2=345536&view=diff
==
--- cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h (original)
+++ cfe/trunk/include/clang/CodeGen/SwiftCallingConv.h Mon Oct 29 13:32:36 2018
@@ -114,6 +114,9 @@ private:
   void addLegalTypedData(llvm::Type *type, CharUnits begin, CharUnits end);
   void addEntry(llvm::Type *type, CharUnits begin, CharUnits end);
   void splitVectorEntry(unsigned index);
+  static bool shouldMergeEntries(const StorageEntry &first,
+ const StorageEntry &second,
+ CharUnits chunkSize);
 };
 
 /// Should an aggregate which expands to the given type sequence

Modified: cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp?rev=345536&r1=345535&r2=345536&view=diff
==
--- cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp (original)
+++ cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp Mon Oct 29 13:32:36 2018
@@ -415,6 +415,40 @@ static bool areBytesInSameUnit(CharUnits
   == getOffsetAtStartOfUnit(second, chunkSize);
 }
 
+static bool isMergeableEntryType(llvm::Type *type) {
+  // Opaquely-typed memory is always mergeable.
+  if (type == nullptr) return true;
+
+  // Pointers and integers are always mergeable.  In theory we should not
+  // merge pointers, but (1) it doesn't currently matter in practice because
+  // the chunk size is never greater than the size of a pointer and (2)
+  // Swift IRGen uses integer types for a lot of things that are "really"
+  // just storing pointers (like Optional).  If we ever have a
+  // target that would otherwise combine pointers, we should put some effort
+  // into fixing those cases in Swift IRGen and then call out pointer types
+  // here.
+
+  // Floating-point and vector types should never be merged.
+  // Most such types are too large and highly-aligned to ever trigger merging
+  // in practice, but it's important for the rule to cover at least 'half'
+  // and 'float', as well as things like small vectors of 'i1' or 'i8'.
+  return (!type->isFloatingPointTy() && !type->isVectorTy());
+}
+
+bool SwiftAggLowering::shouldMergeEntries(const StorageEntry &first,
+  const StorageEntry &second,
+  CharUnits chunkSize) {
+  // Only merge entries that overlap the same chunk.  We test this first
+  // despite being a bit more expensive because this is the condition that
+  // tends to prevent merging.
+  if (!areBytesInSameUnit(first.End - CharUnits::One(), second.Begin,
+  chunkSize))
+return false;
+
+  return (isMergeableEntryType(first.Type) &&
+  isMergeableEntryType(second.Type));
+}
+
 void SwiftAggLowering::finish() {
   if (Entries.empty()) {
 Finished = true;
@@ -425,12 +459,12 @@ void SwiftAggLowering::finish() {
   // which is generally the size of a pointer.
   const CharUnits chunkSize = getMaximumVoluntaryIntegerSize(CGM);
 
-  // First pass: if two entries share a chunk, make them both opaque
+  // First pass: if two entries should be merged, make them both opaque
   // and stretch one to meet the next.
+  // Also, remember if there are any opaque entries.
   bool hasOpaqueEntries = (Entries[0].Type == nullptr);
   for (size_t i = 1, e = Entries.size(); i != e; ++i) {
-if (areBytesInSameUnit(Entries[i - 1].End - CharUnits::One(),
-   Entries[i].Begin, chunkSize)) {
+if (shouldMergeEntries(Entries[i - 1], Entries[i], chunkSize)) {
   Entries[i - 1].Type = nullptr;
   Entries[i].Type = nullptr;
   Entries[i - 1].End = Entries[i].Begin;

Modified: cfe/trunk/test/CodeGen/64bit-swiftcall.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/64bit-swiftcall.c?rev=345536&r1=345535&r2=345536&view=diff
==
--- cfe/trunk/test/CodeGen/64bit-swiftcall.c (original)
+++ cfe/trunk/test/CodeGen/64bit-swiftcall.c Mon Oct 29 13:32:36 2018
@@ -10,7 +10,7 @@
 #define ERROR __attribute__((swift_error_result))
 #define CONTEXT __attribute__((swift_context))
 
-// CHECK: [[STRUCT2_RESULT:@.*]] = private {{.*}} constant 
[[STRUCT2_TYPE:%.*]] { i32 0, i8 0, i8 undef, i8 0, float 0.00

Re: r350920 - [Sema] Make canPassInRegisters return true if the CXXRecordDecl passed

[John McCall via cfe-commits]

On 16 Jan 2019, at 9:13, Aaron Ballman wrote:

On Wed, Jan 16, 2019 at 1:57 AM Akira Hatanaka  
wrote:


Yes, the behavior of the compiler doesn’t match what’s explained 
in the documentation anymore.


Please take a look at the attached patch, which updates the 
documentation.


Patch mostly LGTM, but I did have one wording suggestion.

diff --git a/include/clang/Basic/AttrDocs.td 
b/include/clang/Basic/AttrDocs.td

index 5773a92c9c..ca3cfcf9b2 100644
--- a/include/clang/Basic/AttrDocs.td
+++ b/include/clang/Basic/AttrDocs.td
@@ -2478,15 +2478,20 @@ def TrivialABIDocs : Documentation {
   let Category = DocCatVariable;
   let Content = [{
 The ``trivial_abi`` attribute can be applied to a C++ class, struct, 
or union.
-It instructs the compiler to pass and return the type using the C 
ABI for the

+``trivial_abi`` has the following effects:
+
+- It instructs the compiler to pass and return the type using the C 
ABI for the
 underlying type when the type would otherwise be considered 
non-trivial for the

 purpose of calls.
-A class annotated with `trivial_abi` can have non-trivial 
destructors or copy/move constructors without automatically becoming 
non-trivial for the purposes of calls. For example:
+- It makes the destructor and copy and move constructors of the 
class trivial
+that would otherwise be considered non-trivial under the C++ ABI 
rules.


How about: It makes the destructor, copy constructors, and move
constructors of the class trivial even if they would otherwise be
non-trivial under the C++ ABI rules.


Let's not say that it makes them trivial, because it doesn't.  It causes
their immediate non-triviality to be ignored for the purposes of 
deciding

whether the type can be passed in registers.

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

Re: r350920 - [Sema] Make canPassInRegisters return true if the CXXRecordDecl passed

[John McCall via cfe-commits]

On 16 Jan 2019, at 18:32, Richard Smith wrote:

> On Wed, 16 Jan 2019 at 09:10, John McCall via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On 16 Jan 2019, at 9:13, Aaron Ballman wrote:
>>
>>> On Wed, Jan 16, 2019 at 1:57 AM Akira Hatanaka 
>>> wrote:
>>>>
>>>> Yes, the behavior of the compiler doesn’t match what’s explained
>>>> in the documentation anymore.
>>>>
>>>> Please take a look at the attached patch, which updates the
>>>> documentation.
>>>
>>> Patch mostly LGTM, but I did have one wording suggestion.
>>>
>>>> diff --git a/include/clang/Basic/AttrDocs.td
>>>> b/include/clang/Basic/AttrDocs.td
>>>> index 5773a92c9c..ca3cfcf9b2 100644
>>>> --- a/include/clang/Basic/AttrDocs.td
>>>> +++ b/include/clang/Basic/AttrDocs.td
>>>> @@ -2478,15 +2478,20 @@ def TrivialABIDocs : Documentation {
>>>>let Category = DocCatVariable;
>>>>let Content = [{
>>>>  The ``trivial_abi`` attribute can be applied to a C++ class, struct,
>>>> or union.
>>>> -It instructs the compiler to pass and return the type using the C
>>>> ABI for the
>>>> +``trivial_abi`` has the following effects:
>>>> +
>>>> +- It instructs the compiler to pass and return the type using the C
>>>> ABI for the
>>>>  underlying type when the type would otherwise be considered
>>>> non-trivial for the
>>>>  purpose of calls.
>>>> -A class annotated with `trivial_abi` can have non-trivial
>>>> destructors or copy/move constructors without automatically becoming
>>>> non-trivial for the purposes of calls. For example:
>>>> +- It makes the destructor and copy and move constructors of the
>>>> class trivial
>>>> +that would otherwise be considered non-trivial under the C++ ABI
>>>> rules.
>>>
>>> How about: It makes the destructor, copy constructors, and move
>>> constructors of the class trivial even if they would otherwise be
>>> non-trivial under the C++ ABI rules.
>>
>> Let's not say that it makes them trivial, because it doesn't.  It causes
>> their immediate non-triviality to be ignored for the purposes of
>> deciding
>> whether the type can be passed in registers.
>>
>
> Given the attribute now forces the type to be passed in registers, I think
> it'd be more to the point to say that it makes the triviality of those
> special members be ignored when deciding whether to pass a type with a
> subobject of this type in registers.

Wait, it forces it to be passed in registers?  I thought the design
was that it didn't override the non-trivial-abi-ness of subobjects;
see all the discussion of trivially_relocatable.

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

Re: r350920 - [Sema] Make canPassInRegisters return true if the CXXRecordDecl passed

[John McCall via cfe-commits]

On 16 Jan 2019, at 20:03, Richard Smith wrote:

> On Wed, 16 Jan 2019 at 16:20, John McCall via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On 16 Jan 2019, at 18:32, Richard Smith wrote:
>>
>>> On Wed, 16 Jan 2019 at 09:10, John McCall via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
>>>> On 16 Jan 2019, at 9:13, Aaron Ballman wrote:
>>>>
>>>>> On Wed, Jan 16, 2019 at 1:57 AM Akira Hatanaka 
>>>>> wrote:
>>>>>>
>>>>>> Yes, the behavior of the compiler doesn’t match what’s explained
>>>>>> in the documentation anymore.
>>>>>>
>>>>>> Please take a look at the attached patch, which updates the
>>>>>> documentation.
>>>>>
>>>>> Patch mostly LGTM, but I did have one wording suggestion.
>>>>>
>>>>>> diff --git a/include/clang/Basic/AttrDocs.td
>>>>>> b/include/clang/Basic/AttrDocs.td
>>>>>> index 5773a92c9c..ca3cfcf9b2 100644
>>>>>> --- a/include/clang/Basic/AttrDocs.td
>>>>>> +++ b/include/clang/Basic/AttrDocs.td
>>>>>> @@ -2478,15 +2478,20 @@ def TrivialABIDocs : Documentation {
>>>>>>let Category = DocCatVariable;
>>>>>>let Content = [{
>>>>>>  The ``trivial_abi`` attribute can be applied to a C++ class, struct,
>>>>>> or union.
>>>>>> -It instructs the compiler to pass and return the type using the C
>>>>>> ABI for the
>>>>>> +``trivial_abi`` has the following effects:
>>>>>> +
>>>>>> +- It instructs the compiler to pass and return the type using the C
>>>>>> ABI for the
>>>>>>  underlying type when the type would otherwise be considered
>>>>>> non-trivial for the
>>>>>>  purpose of calls.
>>>>>> -A class annotated with `trivial_abi` can have non-trivial
>>>>>> destructors or copy/move constructors without automatically becoming
>>>>>> non-trivial for the purposes of calls. For example:
>>>>>> +- It makes the destructor and copy and move constructors of the
>>>>>> class trivial
>>>>>> +that would otherwise be considered non-trivial under the C++ ABI
>>>>>> rules.
>>>>>
>>>>> How about: It makes the destructor, copy constructors, and move
>>>>> constructors of the class trivial even if they would otherwise be
>>>>> non-trivial under the C++ ABI rules.
>>>>
>>>> Let's not say that it makes them trivial, because it doesn't.  It causes
>>>> their immediate non-triviality to be ignored for the purposes of
>>>> deciding
>>>> whether the type can be passed in registers.
>>>>
>>>
>>> Given the attribute now forces the type to be passed in registers, I
>> think
>>> it'd be more to the point to say that it makes the triviality of those
>>> special members be ignored when deciding whether to pass a type with a
>>> subobject of this type in registers.
>>
>> Wait, it forces it to be passed in registers?  I thought the design
>> was that it didn't override the non-trivial-abi-ness of subobjects;
>> see all the discussion of trivially_relocatable.
>>
>
> The attribute is ill-formed if applied to a class that has a subobject that
> can't be passed in registers (or that has a vptr). And then as a weird
> special case we don't instantiate the attribute when instantiating a class
> if it would be ill-formed (well, we instantiate it and then remove it
> again, but the effect is the same).
>
> The commit at the start of this email chain switches us from the "just
> override the trivialness for the purposes of the ABI" model to /also/
> forcing the type to be passed in registers (the type would otherwise not be
> passed in registers in some corner cases, such as if all its copy/move
> special members are deleted).

I see.  Alright, I accept your wording, then.

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

Re: r311065 - Further refactoring of the constant emitter. NFC.

[John McCall via cfe-commits]

On 18 Mar 2019, at 14:39, Don Hinton wrote:

It looks like this change introduced a small bug;  Specifically, the
following cast test:

-  if (auto PT = dyn_cast(DestTy)) {
...
+  // If we're producing a pointer, this is easy.
+  if (auto destPtrTy = cast(destTy)) {

Since the cast can fail, shouldn't you prefer dyn_cast<>(), which can
return nullptr, over cast<>(), which will assert?


Yes, although if it hasn't caused a problem in the last year and a half, 
maybe we should just change the code to be non-conditional.


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

Re: r311065 - Further refactoring of the constant emitter. NFC.

[John McCall via cfe-commits]

On 18 Mar 2019, at 15:38, Don Hinton wrote:

Hi John:

I found this investigating the cast assert noted here:
http://lists.llvm.org/pipermail/cfe-dev/2019-March/061685.html

I subsequently did quick grep and found a number of cases in 
clang+llvm

(didn't find any in other projects) .  I'm happy to fix these in mass,
i.e., s/cast/dyn_cast/, but as you mentioned, some might require 
additional

refactoring, e.g., removal of the conditional.


They probably all ought to be considered individually.  Tagging Richard 
in case he has opinions about the AST ones.



In any case, here's what I've found -- perhaps a new llvm clang-tidy
checker would be useful:


Yeah, it would be great if something like this got run automatically.

John.

$ find ./clang ./llvm -type f \( -name "*.h" -o -name "*.cpp" \) -exec 
grep

-Hn "if *(auto.* *= *cast *<" \{\} \;
./clang/lib/Sema/SemaOpenMP.cpp:10904:  else if (auto *DRD =
cast(D))
./clang/lib/CodeGen/CGExprConstant.cpp:1701:  if (auto destPtrTy =
cast(destTy)) {
./clang/lib/CodeGen/MicrosoftCXXABI.cpp:738:  if (auto *Fn =
cast(Throw.getCallee()))
./clang/lib/AST/ASTImporter.cpp:8463:  if (auto *FromDC =
cast(From)) {
./clang/lib/AST/DeclBase.cpp:1182:if (auto *Def =
cast(this)->getDefinition())
./clang/lib/AST/DeclBase.cpp:1187:if (auto *Def =
cast(this)->getDefinition())
./llvm/tools/llvm-objdump/llvm-objdump.cpp:802:  if (auto *Elf64BEObj 
=

cast(Obj))
./llvm/tools/llvm-objdump/llvm-objdump.cpp:846:  else if (auto 
*Elf64BEObj

= cast(Obj))
./llvm/lib/Target/X86/AsmParser/X86Operand.h:102:if (auto Imm 
=

cast(Val)->getValue())
./llvm/lib/Transforms/IPO/CalledValuePropagation.cpp:139:  } else 
if

(auto *F = cast(Key.getPointer()))
./llvm/lib/Transforms/Coroutines/CoroEarly.cpp:183:if (auto 
*CII =

cast(&I)) {

thanks...
don

On Mon, Mar 18, 2019 at 12:07 PM John McCall  
wrote:



On 18 Mar 2019, at 14:39, Don Hinton wrote:

It looks like this change introduced a small bug;  Specifically, the
following cast test:

-  if (auto PT = dyn_cast(DestTy)) {
...
+  // If we're producing a pointer, this is easy.
+  if (auto destPtrTy = cast(destTy)) {

Since the cast can fail, shouldn't you prefer dyn_cast<>(), which 
can

return nullptr, over cast<>(), which will assert?


Yes, although if it hasn't caused a problem in the last year and a 
half,

maybe we should just change the code to be non-conditional.

John.




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

Re: r311065 - Further refactoring of the constant emitter. NFC.

[John McCall via cfe-commits]

On 18 Mar 2019, at 18:05, John McCall wrote:


On 18 Mar 2019, at 15:38, Don Hinton wrote:

Hi John:

I found this investigating the cast assert noted here:
http://lists.llvm.org/pipermail/cfe-dev/2019-March/061685.html

I subsequently did quick grep and found a number of cases in 
clang+llvm
(didn't find any in other projects) .  I'm happy to fix these in 
mass,
i.e., s/cast/dyn_cast/, but as you mentioned, some might require 
additional

refactoring, e.g., removal of the conditional.


They probably all ought to be considered individually.  Tagging 
Richard in case he has opinions about the AST ones.


To get just the Clang ones:

./clang/lib/CodeGen/CGExprConstant.cpp:1701:  if (auto destPtrTy = 
cast(destTy)) {


As discussed, I think this should just be a `cast<>` and
we should drop the dead code following the `if`.

./clang/lib/CodeGen/MicrosoftCXXABI.cpp:738:  if (auto *Fn = 
cast(Throw.getCallee()))


This should be a `dyn_cast` in case there's an existing declaration.
Well, really `CreateRuntimeFunction` should take a CC as an optional
argument, but that's not something I can ask you to do.

./clang/lib/Sema/SemaOpenMP.cpp:10904:  else if (auto *DRD = 
cast(D))


This should be a `dyn_cast`, I think, although again arguably there
should be a more intrusive change to this code that would ensure that
the lookup can only contain these declarations.

./clang/lib/AST/ASTImporter.cpp:8463:  if (auto *FromDC = 
cast(From)) {


It does actually seem to be a general rule that this is only used with
declarations that are DCs, so this can be a `cast<>`, and honestly maybe
it could be updated so that the caller has to pass in a `DeclContext*`.

./clang/lib/AST/DeclBase.cpp:1182:if (auto *Def = 
cast(this)->getDefinition())
./clang/lib/AST/DeclBase.cpp:1187:if (auto *Def = 
cast(this)->getDefinition())


I think `getPrimaryContext` is probably only ever used on declarations
that have members, but `dyn_cast` would be the safe thing to do for
both of these.

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

r358104 - Don't emit an unreachable return block.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104&view=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104&r1=358103&r2=358104&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();
 
-// If that was the only use of the return value, nuke it as well now.
-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = dyn_cast(returnValueInst)) {
-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104&r1=358103&r2=358104&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions called by this
   //function.
   CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = dyn_cast(ReturnValue.getPointer());
+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }
 
 /// ShouldInstrumentFunction - Return true if the current function should be

Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104&view=auto
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp?rev=358104&r1=358103&r2=358104&view=diff
==
--- cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp Wed Apr 10 10:03:09 
2019
@@ -622,7 +622,7 @@ int main() {
 
 // CHECK-NOT: call i32 @__kmpc_reduce
 
-// CHECK: ret void
+// CHECK: }
 
 // CHECK: define {{.*}} i{{[0-9]+}} [[TMAIN_INT]]()
 // CHECK: [[TEST:%.+]] = alloca [[S_INT_TY]],


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

r358115 - Fix an off-by-one mistake in IRGen's copy-construction

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 11:07:18 2019
New Revision: 358115

URL: http://llvm.org/viewvc/llvm-project?rev=358115&view=rev
Log:
Fix an off-by-one mistake in IRGen's copy-construction
special cases in the presence of zero-length arrays.

Patch by Joran Bigalet!

Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=358115&r1=358114&r2=358115&view=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Wed Apr 10 11:07:18 2019
@@ -1008,7 +1008,7 @@ namespace {
   if (FOffset < FirstFieldOffset) {
 FirstField = F;
 FirstFieldOffset = FOffset;
-  } else if (FOffset > LastFieldOffset) {
+  } else if (FOffset >= LastFieldOffset) {
 LastField = F;
 LastFieldOffset = FOffset;
   }

Modified: cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp?rev=358115&r1=358114&r2=358115&view=diff
==
--- cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp Wed Apr 10 11:07:18 2019
@@ -45,6 +45,13 @@ struct ArrayMember {
   int w, x, y, z;
 };
 
+struct ZeroLengthArrayMember {
+NonPOD np;
+int a;
+int b[0];
+int c;
+};
+
 struct VolatileMember {
   int a, b, c, d;
   volatile int v;
@@ -109,6 +116,7 @@ CALL_AO(Basic)
 CALL_AO(PODMember)
 CALL_AO(PODLikeMember)
 CALL_AO(ArrayMember)
+CALL_AO(ZeroLengthArrayMember)
 CALL_AO(VolatileMember)
 CALL_AO(BitfieldMember)
 CALL_AO(InnerClassMember)
@@ -142,6 +150,12 @@ CALL_AO(PackedMembers)
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 64, i1 {{.*}})
 // CHECK: ret %struct.ArrayMember*
 
+// ZeroLengthArrayMember copy-assignment:
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) 
%struct.ZeroLengthArrayMember* 
@_ZN21ZeroLengthArrayMemberaSERKS_(%struct.ZeroLengthArrayMember* %this, 
%struct.ZeroLengthArrayMember* dereferenceable({{[0-9]+}}))
+// CHECK: call dereferenceable({{[0-9]+}}) %struct.NonPOD* @_ZN6NonPODaSERKS_
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 8, i1 {{.*}})
+// CHECK: ret %struct.ZeroLengthArrayMember*
+
 // VolatileMember copy-assignment:
 // CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) 
%struct.VolatileMember* @_ZN14VolatileMemberaSERKS_(%struct.VolatileMember* 
%this, %struct.VolatileMember* dereferenceable({{[0-9]+}}))
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 16, i1 {{.*}})


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

r358125 - Fix for different build configurations.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 12:11:32 2019
New Revision: 358125

URL: http://llvm.org/viewvc/llvm-project?rev=358125&view=rev
Log:
Fix for different build configurations.

Modified:
cfe/trunk/test/CodeGen/unreachable-ret.c

Modified: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358125&r1=358124&r2=358125&view=diff
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (original)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 12:11:32 2019
@@ -5,7 +5,7 @@ extern void abort() __attribute__((noret
 void f1() {
   abort();
 }
-// CHECK-LABEL: define void @f1()
+// CHECK-LABEL: define {{.*}}void @f1()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:   call void @abort()
 // CHECK-NEXT:   unreachable
@@ -15,7 +15,7 @@ void *f2() {
   abort();
   return 0;
 }
-// CHECK-LABEL: define i8* @f2()
+// CHECK-LABEL: define {{.*}}i8* @f2()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:   call void @abort()
 // CHECK-NEXT:   unreachable


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

r358132 - Add IRGen APIs to fetch ctor/dtor helper functions for non-trivial structs.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 12:57:20 2019
New Revision: 358132

URL: http://llvm.org/viewvc/llvm-project?rev=358132&view=rev
Log:
Add IRGen APIs to fetch ctor/dtor helper functions for non-trivial structs.

Patch by Tony Allevato!

Modified:
cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp

Modified: cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h?rev=358132&r1=358131&r2=358132&view=diff
==
--- cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h (original)
+++ cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h Wed Apr 10 12:57:20 2019
@@ -30,6 +30,7 @@
 namespace llvm {
   class DataLayout;
   class Module;
+  class Function;
   class FunctionType;
   class Type;
 }
@@ -83,6 +84,59 @@ llvm::Type *convertTypeForMemory(CodeGen
 unsigned getLLVMFieldNumber(CodeGenModule &CGM,
 const RecordDecl *RD, const FieldDecl *FD);
 
+/// Returns the default constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes a single argument that is a
+/// pointer to the address of the struct.
+llvm::Function *getNonTrivialCStructDefaultConstructor(CodeGenModule &GCM,
+   CharUnits DstAlignment,
+   bool IsVolatile,
+   QualType QT);
+
+/// Returns the copy constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes two arguments: pointers to the
+/// addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructCopyConstructor(CodeGenModule &CGM,
+CharUnits DstAlignment,
+CharUnits SrcAlignment,
+bool IsVolatile,
+QualType QT);
+
+/// Returns the move constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes two arguments: pointers to the
+/// addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructMoveConstructor(CodeGenModule &CGM,
+CharUnits DstAlignment,
+CharUnits SrcAlignment,
+bool IsVolatile,
+QualType QT);
+
+/// Returns the copy assignment operator for a C struct with non-trivially
+/// copyable fields, generating it if necessary. The returned function uses the
+/// `cdecl` calling convention, returns void, and takes two arguments: pointers
+/// to the addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructCopyAssignmentOperator(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT);
+
+/// Return the move assignment operator for a C struct with non-trivially
+/// copyable fields, generating it if necessary. The returned function uses the
+/// `cdecl` calling convention, returns void, and takes two arguments: pointers
+/// to the addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructMoveAssignmentOperator(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT);
+
+/// Returns the destructor for a C struct with non-trivially copyable fields,
+/// generating it if necessary. The returned function uses the `cdecl` calling
+/// convention, returns void, and takes a single argument that is a pointer to
+/// the address of the struct.
+llvm::Function *getNonTrivialCStructDestructor(CodeGenModule &CGM,
+   CharUnits DstAlignment,
+   bool IsVolatile, QualType QT);
+
 }  // end namespace CodeGen
 }  // end namespace clang
 

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=358132&r1=358131&r2=358132&view=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Wed Apr 10 12:57:20 2019
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h

Re: r358104 - Don't emit an unreachable return block.

[John McCall via cfe-commits]

On 10 Apr 2019, at 21:50, wolfgang.p...@sony.com wrote:


Hi,
This commit seems to be causing a test failure on several buildbots 
(e.g. 
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/26305). 
instrprof-shared-gcov-flush.test is failing because of differences in 
profile info. The test passes when I revert your change, so perhaps 
it's just a case of updating the test.

Could you please take a look?


Adjusted the test in r358149 as a hopeful fix.  CC'ing Marco since I 
believe this was originally his test.


John.



Thanks,
Wolfgang Pieb

-Original Message-
From: cfe-commits  On Behalf Of 
John McCall via cfe-commits

Sent: Wednesday, April 10, 2019 10:03 AM
To: cfe-commits@lists.llvm.org
Subject: r358104 - Don't emit an unreachable return block.

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104&view=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104&r1=358103&r2=358104&view=diff

==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();

-// If that was the only use of the return value, nuke it as 
well now.

-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = 
dyn_cast(returnValueInst)) {

-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104&r1=358103&r2=358104&view=diff

==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions 
called by this

   //function.
   CurFn->addFnAttr("min-legal-vector-width", 
llvm::utostr(LargestVectorWidth));

+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = 
dyn_cast(ReturnValue.getPointer());

+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }

 /// ShouldInstrumentFunction - Return true if the current function 
should be


Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104&view=auto

==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://ll

Re: [PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.

[John McCall via cfe-commits]

On Sun, Sep 23, 2018 at 5:39 AM David Chisnall via Phabricator <
revi...@reviews.llvm.org> wrote:

> theraven added a comment.
>
> In https://reviews.llvm.org/D52344#1242530, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D52344#1242451, @kristina wrote:
> >
> > > Would you be okay with me renaming `cfstring` to `.cfstring` for ELF
> so it's in line with ELF section naming conventions (I'm not entirely sure
> if that could cause issues with ObjC stuff)? And yes I think it's best to
> avoid that code-path altogether if it turns out to be a constant as that's
> likely from the declaration of the type, I'll write a FileCheck test in a
> moment and check that I can apply the same logic to ELF aside from the DLL
> stuff as CoreFoundation needs to export the symbol somehow while preserving
> the implicit extern attribute for every other TU that comes from the
> builtin that `CFSTR` expands to. Is what I'm suggesting making sense? If
> not, let me know, I may be misunderstanding what's happening here.
> >
> >
> > Following the ELF conventions seems like the right thing to do, assuming
> it doesn't cause compatibility problems.  David Chisnall might know best
> here.
>
>
> I don't believe it will have any compatibility issues.  We expose builtins
> for creating CF- and NSString objects from C code.  For Apple targets,
> these are equivalent.  For targets that don't care about Objective-C
> interop, the CF version is sensible because it doesn't introduce any
> dependencies on an Objective-C runtime.  For code targeting a non-Apple
> runtime and wanting CF / Foundation interop, the `CFSTR` macro expands to
> the NS version.
>
> As others have pointed out, the section must be a valid C identifier for
> the magic `__start_` and `__stop_` symbols to be created.  I don't really
> like this limitation and it would be nice if we could improve lld (and
> encourage binutils to do the same) so that `__start_.cfstring` and
> `__stop_.cfstring` could be used to create symbols pointing to the start
> and end of a section because it's useful to name things in such a way that
> they can't conflict with any valid C identifier.  With PE/COFF, we have a
> mechanism for more precise control and can give sections any names that we
> want (and also arrange for start and end symbols for subsections).
>

Okay.  I can respect wanting to change the rules on ELF, but it sounds like
we do need to stick with the current section names.  Absent an ABI break to
stop looking for the existing section names, CF will misbehave if they
exist even if we change the compiler output, so effectively those
identifiers are claimed in a way that cannot be incrementally changed.
Wishing otherwise won't make it so.

Also, it's probably correct for CF on non-Darwin OSes to stick to
non-system namespaces anyway.

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

Re: r350920 - [Sema] Make canPassInRegisters return true if the CXXRecordDecl passed

[John McCall via cfe-commits]

On 31 Jan 2019, at 16:57, Akira Hatanaka wrote:

Would it be better if we disallowed trivial_abi if the class’ copy 
and move destructors were all deleted (and revert r350920)? I think 
that would make it easier to reason about when you are allowed to use 
trivial_abi and what effect the attribute has (which is to override 
the trivialness for the purpose of calls).


Sorry for my late reply. It took a while to understand that the patch 
I committed might not be the right way to fix the problem.


I'd be fine with that.  If nothing else, we can generalize it later if 
we decide that's an important use-case.


John.



On Jan 16, 2019, at 8:37 PM, John McCall via cfe-commits 
 wrote:


On 16 Jan 2019, at 20:03, Richard Smith wrote:


On Wed, 16 Jan 2019 at 16:20, John McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:


On 16 Jan 2019, at 18:32, Richard Smith wrote:


On Wed, 16 Jan 2019 at 09:10, John McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:


On 16 Jan 2019, at 9:13, Aaron Ballman wrote:

On Wed, Jan 16, 2019 at 1:57 AM Akira Hatanaka 


wrote:


Yes, the behavior of the compiler doesn’t match what’s 
explained

in the documentation anymore.

Please take a look at the attached patch, which updates the
documentation.


Patch mostly LGTM, but I did have one wording suggestion.


diff --git a/include/clang/Basic/AttrDocs.td
b/include/clang/Basic/AttrDocs.td
index 5773a92c9c..ca3cfcf9b2 100644
--- a/include/clang/Basic/AttrDocs.td
+++ b/include/clang/Basic/AttrDocs.td
@@ -2478,15 +2478,20 @@ def TrivialABIDocs : Documentation {
  let Category = DocCatVariable;
  let Content = [{
The ``trivial_abi`` attribute can be applied to a C++ class, 
struct,

or union.
-It instructs the compiler to pass and return the type using 
the C

ABI for the
+``trivial_abi`` has the following effects:
+
+- It instructs the compiler to pass and return the type using 
the C

ABI for the
underlying type when the type would otherwise be considered
non-trivial for the
purpose of calls.
-A class annotated with `trivial_abi` can have non-trivial
destructors or copy/move constructors without automatically 
becoming

non-trivial for the purposes of calls. For example:
+- It makes the destructor and copy and move constructors of 
the

class trivial
+that would otherwise be considered non-trivial under the C++ 
ABI

rules.


How about: It makes the destructor, copy constructors, and move
constructors of the class trivial even if they would otherwise 
be

non-trivial under the C++ ABI rules.


Let's not say that it makes them trivial, because it doesn't.  It 
causes

their immediate non-triviality to be ignored for the purposes of
deciding
whether the type can be passed in registers.



Given the attribute now forces the type to be passed in registers, 
I

think
it'd be more to the point to say that it makes the triviality of 
those
special members be ignored when deciding whether to pass a type 
with a

subobject of this type in registers.


Wait, it forces it to be passed in registers?  I thought the design
was that it didn't override the non-trivial-abi-ness of subobjects;
see all the discussion of trivially_relocatable.



The attribute is ill-formed if applied to a class that has a 
subobject that
can't be passed in registers (or that has a vptr). And then as a 
weird
special case we don't instantiate the attribute when instantiating a 
class
if it would be ill-formed (well, we instantiate it and then remove 
it

again, but the effect is the same).

The commit at the start of this email chain switches us from the 
"just
override the trivialness for the purposes of the ABI" model to 
/also/
forcing the type to be passed in registers (the type would otherwise 
not be
passed in registers in some corner cases, such as if all its 
copy/move

special members are deleted).


I see.  Alright, I accept your wording, then.

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



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

r362054 - Add the `objc_class_stub` attribute.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed May 29 21:09:01 2019
New Revision: 362054

URL: http://llvm.org/viewvc/llvm-project?rev=362054&view=rev
Log:
Add the `objc_class_stub` attribute.

Swift requires certain classes to be not just initialized lazily on first
use, but actually allocated lazily using information that is only available
at runtime.  This is incompatible with ObjC class initialization, or at least
not efficiently compatible, because there is no meaningful class symbol
that can be put in a class-ref variable at load time.  This leaves ObjC
code unable to access such classes, which is undesirable.

objc_class_stub says that class references should be resolved by calling
a new ObjC runtime function with a pointer to a new "class stub" structure.
Non-ObjC compilers (like Swift) can simply emit this structure when ObjC
interop is required for a class that cannot be statically allocated,
then apply this attribute to the `@interface` in the generated ObjC header
for the class.

This attribute can be thought of as a generalization of the existing
`objc_runtime_visible` attribute which permits more efficient class
resolution as well as supporting the additon of categories to the class.
Subclassing these classes from ObjC is currently not allowed.

Patch by Slava Pestov!

Added:
cfe/trunk/test/CodeGenObjC/class-stubs.m
cfe/trunk/test/SemaObjC/class-stub-attr-unsupported.m
cfe/trunk/test/SemaObjC/class-stub-attr.m
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/ObjCRuntime.h
cfe/trunk/lib/CodeGen/CGObjCMac.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=362054&r1=362053&r2=362054&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed May 29 21:09:01 2019
@@ -284,20 +284,25 @@ class SubjectList subj
   string CustomDiag = customDiag;
 }
 
-class LangOpt {
+class LangOpt {
   string Name = name;
-  bit Negated = negated;
+
+  // A custom predicate, written as an expression evaluated in a context with
+  // "LangOpts" bound.
+  code CustomCode = customCode;
 }
 def MicrosoftExt : LangOpt<"MicrosoftExt">;
 def Borland : LangOpt<"Borland">;
 def CUDA : LangOpt<"CUDA">;
-def COnly : LangOpt<"CPlusPlus", 1>;
+def COnly : LangOpt<"COnly", "!LangOpts.CPlusPlus">;
 def CPlusPlus : LangOpt<"CPlusPlus">;
 def OpenCL : LangOpt<"OpenCL">;
 def RenderScript : LangOpt<"RenderScript">;
 def ObjC : LangOpt<"ObjC">;
 def BlocksSupported : LangOpt<"Blocks">;
 def ObjCAutoRefCount : LangOpt<"ObjCAutoRefCount">;
+def ObjCNonFragileRuntime : LangOpt<"ObjCNonFragileRuntime",
+"LangOpts.ObjCRuntime.allowsClassStubs()">;
 
 // Language option for CMSE extensions
 def Cmse : LangOpt<"Cmse">;
@@ -1806,6 +1811,13 @@ def ObjCRuntimeVisible : Attr {
   let Documentation = [ObjCRuntimeVisibleDocs];
 }
 
+def ObjCClassStub : Attr {
+  let Spellings = [Clang<"objc_class_stub">];
+  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let Documentation = [ObjCClassStubDocs];
+  let LangOpts = [ObjCNonFragileRuntime];
+}
+
 def ObjCBoxable : Attr {
   let Spellings = [Clang<"objc_boxable">];
   let Subjects = SubjectList<[Record], ErrorDiag>;

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=362054&r1=362053&r2=362054&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed May 29 21:09:01 2019
@@ -1085,6 +1085,25 @@ them.
 }];
 }
 
+def ObjCClassStubDocs : Documentation {
+let Category = DocCatType;
+let Content = [{
+This attribute specifies that the Objective-C class to which it applies is
+instantiated at runtime.
+
+Unlike ``__attribute__((objc_runtime_visible))``, a class having this attribute
+still has a "class stub" that is visible to the linker. This allows categories
+to be defined. Static message sends with the class as a receiver use a special
+access pattern to ensure the class is lazily instantiated from the class stub.
+
+Classes annotated with this attribute cannot be subclassed and cannot have
+implementations defined for them. This attribute is intended for use in
+Swift-generated headers for classes defined in Swift.
+
+Adding or removing this attribute to a class is an ABI-breaking change.
+}];
+}
+
 def ObjCBoxableDocs : Documentation {
 let Category = DocCatDecl;
 let 

r362183 - Fix the predefined exponent limit macros for the 16-bit IEEE format.

[John McCall via cfe-commits]

Author: rjmccall
Date: Thu May 30 18:21:36 2019
New Revision: 362183

URL: http://llvm.org/viewvc/llvm-project?rev=362183&view=rev
Log:
Fix the predefined exponent limit macros for the 16-bit IEEE format.

The magnitude range of normalized _Float16 is 2^-14 (~6e-5) to
(2-2^-10)*2^15 (65504).  You might think, then, that the code is
correct to defne FLT16_MIN_EXP and FLT16_MAX_EXP to be -14 and 15
respectively.  However, for some reason the C specification actually
specifies a bias for these macros:

C11 5.2.4.2.2:

  - minimum negative integer such that FLT_RADIX raised to one less than
that power is a normalized floating-point number, e_min:
  FLT_MIN_EXP
  DBL_MIN_EXP
  LDBL_MIN_EXP

  - maximum integer such that FLT_RADIX raised to one less than that
power is a representable finite floating-point number, e_max:
  FLT_MAX_EXP
  DBL_MAX_EXP
  LDBL_MAX_EXP

FLT16_MIN_EXP and FLT16_MAX_EXP should clearly be biased the same way,
and other compilers do in fact do so, as do our OpenCL headers for `half`.

Additionally, FLT16_MIN_10_EXP is just wrong.

Modified:
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/test/Headers/float16.c
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=362183&r1=362182&r2=362183&view=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Thu May 30 18:21:36 2019
@@ -122,10 +122,10 @@ static void DefineFloatMacros(MacroBuild
"4.94065645841246544176568792868221e-324",
"1.92592994438723585305597794258492732e-34");
   int MantissaDigits = PickFP(Sem, 11, 24, 53, 64, 106, 113);
-  int Min10Exp = PickFP(Sem, -13, -37, -307, -4931, -291, -4931);
+  int Min10Exp = PickFP(Sem, -4, -37, -307, -4931, -291, -4931);
   int Max10Exp = PickFP(Sem, 4, 38, 308, 4932, 308, 4932);
-  int MinExp = PickFP(Sem, -14, -125, -1021, -16381, -968, -16381);
-  int MaxExp = PickFP(Sem, 15, 128, 1024, 16384, 1024, 16384);
+  int MinExp = PickFP(Sem, -13, -125, -1021, -16381, -968, -16381);
+  int MaxExp = PickFP(Sem, 16, 128, 1024, 16384, 1024, 16384);
   Min = PickFP(Sem, "6.103515625e-5", "1.17549435e-38", 
"2.2250738585072014e-308",
"3.36210314311209350626e-4932",
"2.00416836000897277799610805135016e-292",

Modified: cfe/trunk/test/Headers/float16.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/float16.c?rev=362183&r1=362182&r2=362183&view=diff
==
--- cfe/trunk/test/Headers/float16.c (original)
+++ cfe/trunk/test/Headers/float16.c Thu May 30 18:21:36 2019
@@ -13,7 +13,7 @@
 
 #ifndef FLT16_MIN_10_EXP
 #error "Macro FLT16_MIN_10_EXP is missing."
-#elif   FLT16_MIN_10_EXP > -13
+#elif   FLT16_MIN_10_EXP > -4
 #error "Macro FLT16_MIN_10_EXP is invalid."
 #endif
 
@@ -21,7 +21,7 @@ _Static_assert(FLT16_MIN_10_EXP == __FLT
 
 #ifndef FLT16_MIN_EXP
 #error "Macro FLT16_MIN_EXP is missing."
-#elif   FLT16_MIN_EXP > -14
+#elif   FLT16_MIN_EXP > -13
 #error "Macro FLT16_MIN_EXP is invalid."
 #endif
 
@@ -37,7 +37,7 @@ _Static_assert(FLT16_MAX_10_EXP == __FLT
 
 #ifndef FLT16_MAX_EXP
 #error "Macro FLT16_MAX_EXP is missing."
-#elif   FLT16_MAX_EXP < 15
+#elif   FLT16_MAX_EXP < 16
 #error "Macro FLT16_MAX_EXP is invalid."
 #endif
 

Modified: cfe/trunk/test/Preprocessor/init.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=362183&r1=362182&r2=362183&view=diff
==
--- cfe/trunk/test/Preprocessor/init.c (original)
+++ cfe/trunk/test/Preprocessor/init.c Thu May 30 18:21:36 2019
@@ -310,10 +310,10 @@
 // AARCH64:#define __FLT16_HAS_QUIET_NAN__ 1
 // AARCH64:#define __FLT16_MANT_DIG__ 11
 // AARCH64:#define __FLT16_MAX_10_EXP__ 4
-// AARCH64:#define __FLT16_MAX_EXP__ 15
+// AARCH64:#define __FLT16_MAX_EXP__ 16
 // AARCH64:#define __FLT16_MAX__ 6.5504e+4F16
-// AARCH64:#define __FLT16_MIN_10_EXP__ (-13)
-// AARCH64:#define __FLT16_MIN_EXP__ (-14)
+// AARCH64:#define __FLT16_MIN_10_EXP__ (-4)
+// AARCH64:#define __FLT16_MIN_EXP__ (-13)
 // AARCH64:#define __FLT16_MIN__ 6.103515625e-5F16
 // AARCH64:#define __FLT_DENORM_MIN__ 1.40129846e-45F
 // AARCH64:#define __FLT_DIG__ 6


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

r368796 - Remove unreachable blocks before splitting a coroutine.

[John McCall via cfe-commits]

Author: rjmccall
Date: Tue Aug 13 20:54:13 2019
New Revision: 368796

URL: http://llvm.org/viewvc/llvm-project?rev=368796&view=rev
Log:
Remove unreachable blocks before splitting a coroutine.

The suspend-crossing algorithm is not correct in the presence of uses
that cannot be reached on some successor path from their defs.

Added:
cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll

Added: cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll?rev=368796&view=auto
==
--- cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll (added)
+++ cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll Tue Aug 13 
20:54:13 2019
@@ -0,0 +1,46 @@
+; RUN: opt < %s -coro-early -coro-split -S | FileCheck %s
+target datalayout = "E-p:64:64"
+
+%swift.type = type { i64 }
+%swift.opaque = type opaque
+%T4red215EmptyCollectionV = type opaque
+%TSi = type <{ i64 }>
+
+define hidden swiftcc { i8*, %swift.opaque* } @no_suspends(i8* %buffer, i64 
%arg) #1 {
+  %id = call token @llvm.coro.id.retcon.once(i32 32, i32 8, i8* %buffer, i8* 
bitcast (void (i8*, i1)* @prototype to i8*), i8* bitcast (i8* (i64)* @malloc to 
i8*), i8* bitcast (void (i8*)* @free to i8*))
+  %begin = call i8* @llvm.coro.begin(token %id, i8* null)
+  call void @print(i64 %arg)
+  call void @llvm.trap()
+  unreachable
+
+bb1:
+  call void @print(i64 %arg)
+  call i1 @llvm.coro.end(i8* %begin, i1 false)
+  unreachable
+}
+; CHECK-LABEL: define hidden swiftcc { i8*, %swift.opaque* } @no_suspends(
+; CHECK: call token @llvm.coro.id.retcon.once
+; CHECK-NEXT:call void @print(i64 %arg)
+; CHECK-NEXT:call void @llvm.trap()
+; CHECK-NEXT:unreachable
+
+declare swiftcc void @prototype(i8* noalias dereferenceable(32), i1)
+declare void @print(i64)
+
+declare noalias i8* @malloc(i64) #5
+declare void @free(i8* nocapture) #5
+
+declare token @llvm.coro.id.retcon.once(i32, i32, i8*, i8*, i8*, i8*) #5
+declare i8* @llvm.coro.begin(token, i8* writeonly) #5
+declare token @llvm.coro.alloca.alloc.i64(i64, i32) #5
+declare i8* @llvm.coro.alloca.get(token) #5
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) #6
+declare i1 @llvm.coro.suspend.retcon.i1(...) #5
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) #6
+declare void @llvm.coro.alloca.free(token) #5
+declare i1 @llvm.coro.end(i8*, i1) #5
+
+declare void @llvm.trap()
+
+attributes #1 = { noreturn nounwind }
+attributes #5 = { nounwind }


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

r373407 - Emit TypeNodes.def with tblgen.

[John McCall via cfe-commits]

Author: rjmccall
Date: Tue Oct  1 16:13:03 2019
New Revision: 373407

URL: http://llvm.org/viewvc/llvm-project?rev=373407&view=rev
Log:
Emit TypeNodes.def with tblgen.

The primary goal here is to make the type node hierarchy available to
other tblgen backends, although it should also make it easier to generate
more selective x-macros in the future.

Because tblgen doesn't seem to allow backends to preserve the source
order of defs, this is not NFC because it significantly re-orders IDs.
I've fixed the one (fortunately obvious) place where we relied on
the old order.  Unfortunately, I wasn't able to share code with the
existing AST-node x-macro generators because the x-macro schema we use
for types is different in a number of ways.  The main loss is that
subclasses aren't ordered together, which doesn't seem important for
types because the hierarchy is generally very shallow with little
clustering.

Added:
cfe/trunk/include/clang/Basic/TypeNodes.td
cfe/trunk/utils/TableGen/ClangTypeNodesEmitter.cpp
Removed:
cfe/trunk/include/clang/AST/TypeNodes.def
Modified:
cfe/trunk/include/clang/AST/CMakeLists.txt
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Modified: cfe/trunk/include/clang/AST/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CMakeLists.txt?rev=373407&r1=373406&r2=373407&view=diff
==
--- cfe/trunk/include/clang/AST/CMakeLists.txt (original)
+++ cfe/trunk/include/clang/AST/CMakeLists.txt Tue Oct  1 16:13:03 2019
@@ -31,6 +31,10 @@ clang_tablegen(DeclNodes.inc -gen-clang-
   SOURCE ../Basic/DeclNodes.td
   TARGET ClangDeclNodes)
 
+clang_tablegen(TypeNodes.def -gen-clang-type-nodes
+  SOURCE ../Basic/TypeNodes.td
+  TARGET ClangTypeNodes)
+
 clang_tablegen(CommentNodes.inc -gen-clang-comment-nodes
   SOURCE ../Basic/CommentNodes.td
   TARGET ClangCommentNodes)

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=373407&r1=373406&r2=373407&view=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Tue Oct  1 16:13:03 2019
@@ -1437,10 +1437,9 @@ class alignas(8) Type : public ExtQualsT
 public:
   enum TypeClass {
 #define TYPE(Class, Base) Class,
-#define LAST_TYPE(Class) TypeLast = Class,
+#define LAST_TYPE(Class) TypeLast = Class
 #define ABSTRACT_TYPE(Class, Base)
 #include "clang/AST/TypeNodes.def"
-TagFirst = Record, TagLast = Enum
   };
 
 private:
@@ -4436,7 +4435,7 @@ public:
   bool isBeingDefined() const;
 
   static bool classof(const Type *T) {
-return T->getTypeClass() >= TagFirst && T->getTypeClass() <= TagLast;
+return T->getTypeClass() == Enum || T->getTypeClass() == Record;
   }
 };
 

Removed: cfe/trunk/include/clang/AST/TypeNodes.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TypeNodes.def?rev=373406&view=auto
==
--- cfe/trunk/include/clang/AST/TypeNodes.def (original)
+++ cfe/trunk/include/clang/AST/TypeNodes.def (removed)
@@ -1,135 +0,0 @@
-//===-- TypeNodes.def - Metadata about Type AST nodes ---*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-//
-//  This file defines the AST type info database. Each type node is
-//  enumerated by providing its name (e.g., "Builtin" or "Enum") and
-//  base class (e.g., "Type" or "TagType"). Depending on where in the
-//  abstract syntax tree the type will show up, the enumeration uses
-//  one of five different macros:
-//
-//TYPE(Class, Base) - A type that can show up anywhere in the AST,
-//and might be dependent, canonical, or non-canonical. All clients
-//will need to understand these types.
-//
-//ABSTRACT_TYPE(Class, Base) - An abstract class that shows up in
-//the type hierarchy but has no concrete instances.
-//
-//NON_CANONICAL_TYPE(Class, Base) - A type that can show up
-//anywhere in the AST but will never be a part of a canonical
-//type. Clients that only need to deal with canonical types
-//(ignoring, e.g., typedefs and other type aliases used for
-//pretty-printing) can ignore these types.
-//
-//DEPENDENT_TYPE(Class, Base) - A type that will only show up
-//within a C++ template that has not been instantiated, e.g., a
-//type that is always dependent. Clients that do not need to deal
-//with uninstantiated C++ templates can ignore these types.
-//
-//

r373406 - Use scope qualifiers in Clang's tblgen backends to get useful

[John McCall via cfe-commits]

Author: rjmccall
Date: Tue Oct  1 16:12:57 2019
New Revision: 373406

URL: http://llvm.org/viewvc/llvm-project?rev=373406&view=rev
Log:
Use scope qualifiers in Clang's tblgen backends to get useful
redeclaration checking.  NFC.

Modified:
cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp
cfe/trunk/utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
cfe/trunk/utils/TableGen/ClangDataCollectorsEmitter.cpp
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
cfe/trunk/utils/TableGen/ClangOpcodesEmitter.cpp
cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
cfe/trunk/utils/TableGen/ClangOptionDocEmitter.cpp
cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp
cfe/trunk/utils/TableGen/NeonEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp?rev=373406&r1=373405&r2=373406&view=diff
==
--- cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp Tue Oct  1 16:12:57 2019
@@ -10,6 +10,8 @@
 //
 
//===--===//
 
+#include "TableGenBackends.h"
+
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
@@ -173,15 +175,14 @@ void ClangASTNodesEmitter::run(raw_ostre
   OS << "#undef ABSTRACT_" << macroName(Root.getName()) << "\n";
 }
 
-namespace clang {
-void EmitClangASTNodes(RecordKeeper &RK, raw_ostream &OS,
-   const std::string &N, const std::string &S) {
+void clang::EmitClangASTNodes(RecordKeeper &RK, raw_ostream &OS,
+  const std::string &N, const std::string &S) {
   ClangASTNodesEmitter(RK, N, S).run(OS);
 }
 
 // Emits and addendum to a .inc file to enumerate the clang declaration
 // contexts.
-void EmitClangDeclContext(RecordKeeper &Records, raw_ostream &OS) {
+void clang::EmitClangDeclContext(RecordKeeper &Records, raw_ostream &OS) {
   // FIXME: Find a .td file format to allow for this to be represented better.
 
   emitSourceFileHeader("List of AST Decl nodes", OS);
@@ -225,4 +226,3 @@ void EmitClangDeclContext(RecordKeeper &
   OS << "#undef DECL_CONTEXT\n";
   OS << "#undef DECL_CONTEXT_BASE\n";
 }
-} // end namespace clang

Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=373406&r1=373405&r2=373406&view=diff
==
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Tue Oct  1 16:12:57 2019
@@ -10,6 +10,8 @@
 //
 
//===--===//
 
+#include "TableGenBackends.h"
+
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -2219,10 +2221,8 @@ static void emitClangAttrThisIsaIdentifi
   OS << "#endif // CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST\n\n";
 }
 
-namespace clang {
-
 // Emits the class definitions for attributes.
-void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) {
+void clang::EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) {
   emitSourceFileHeader("Attribute classes' definitions", OS);
 
   OS << "#ifndef LLVM_CLANG_ATTR_CLASSES_INC\n";
@@ -2491,7 +2491,7 @@ void EmitClangAttrClass(RecordKeeper &Re
 }
 
 // Emits the class method definitions for attributes.
-void EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
+void clang::EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
   emitSourceFileHeader("Attribute classes' member function definitions", OS);
 
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
@@ -2556,8 +2556,6 @@ void EmitClangAttrImpl(RecordKeeper &Rec
   EmitFunc("printPretty(OS, Policy)");
 }
 
-} // end namespace clang
-
 static void emitAttrList(raw_ostream &OS, StringRef Class,
  const std::vector &AttrList) {
   for (auto Cur : AttrList) {

Modified: cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp?rev=373406&r1=373405&r2=373406&view=diff
==
--- cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp Tue Oct  1 
16:12:57 2019
@@ -11,6 +11,8 @@
 //
 
//===--===//
 
+#include "TableGenBackends.h"
+
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/StringMatcher.h"
 #include "

r373416 - Remove TypeNodes.def from the modulemap.

[John McCall via cfe-commits]

Author: rjmccall
Date: Tue Oct  1 18:02:27 2019
New Revision: 373416

URL: http://llvm.org/viewvc/llvm-project?rev=373416&view=rev
Log:
Remove TypeNodes.def from the modulemap.

We currently just look for files named in the modulemap in its
associated source directory.  This means that we can't name
generated files, like TypeNodes.def now is, which means we can't
explicitly mark it as textual.  But fortunately that's okay
because (as I understand it) the most important purpose of naming
the header in the modulemap is to ensure that it's not treated as
public, and the search for public headers also only considers
files in the associated source directory.  This isn't an elegant
solution, since among other things it means that a build which
wrote the generated files directly into the source directory would
result in something that wouldn't build as a module, but that's
a problem for all our other generated files as well.

Modified:
cfe/trunk/include/clang/module.modulemap

Modified: cfe/trunk/include/clang/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/module.modulemap?rev=373416&r1=373415&r2=373416&view=diff
==
--- cfe/trunk/include/clang/module.modulemap (original)
+++ cfe/trunk/include/clang/module.modulemap Tue Oct  1 18:02:27 2019
@@ -20,7 +20,6 @@ module Clang_AST {
   textual header "AST/BuiltinTypes.def"
   textual header "AST/OperationKinds.def"
   textual header "AST/TypeLocNodes.def"
-  textual header "AST/TypeNodes.def"
 
   module * { export * }
 }


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

Re: r373407 - Emit TypeNodes.def with tblgen.

[John McCall via cfe-commits]

On 1 Oct 2019, at 21:20, Nico Weber wrote:
All other tablegen outputs are called .inc instead of .def. Any reason 
this

one's different?


This was an existing file; it’s just generated now instead of written
in source.  I was deliberately not changing anything about the existing
use sites.  That said, I could certainly go rename it as a follow-up
just to re-establish that consistent naming convention.

John.



On Tue, Oct 1, 2019 at 7:10 PM John McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:


Author: rjmccall
Date: Tue Oct  1 16:13:03 2019
New Revision: 373407

URL: http://llvm.org/viewvc/llvm-project?rev=373407&view=rev
Log:
Emit TypeNodes.def with tblgen.

The primary goal here is to make the type node hierarchy available to
other tblgen backends, although it should also make it easier to 
generate

more selective x-macros in the future.

Because tblgen doesn't seem to allow backends to preserve the source
order of defs, this is not NFC because it significantly re-orders 
IDs.

I've fixed the one (fortunately obvious) place where we relied on
the old order.  Unfortunately, I wasn't able to share code with the
existing AST-node x-macro generators because the x-macro schema we 
use

for types is different in a number of ways.  The main loss is that
subclasses aren't ordered together, which doesn't seem important for
types because the hierarchy is generally very shallow with little
clustering.

Added:
cfe/trunk/include/clang/Basic/TypeNodes.td
cfe/trunk/utils/TableGen/ClangTypeNodesEmitter.cpp
Removed:
cfe/trunk/include/clang/AST/TypeNodes.def
Modified:
cfe/trunk/include/clang/AST/CMakeLists.txt
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Modified: cfe/trunk/include/clang/AST/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CMakeLists.txt?rev=373407&r1=373406&r2=373407&view=diff

==
--- cfe/trunk/include/clang/AST/CMakeLists.txt (original)
+++ cfe/trunk/include/clang/AST/CMakeLists.txt Tue Oct  1 16:13:03 
2019

@@ -31,6 +31,10 @@ clang_tablegen(DeclNodes.inc -gen-clang-
   SOURCE ../Basic/DeclNodes.td
   TARGET ClangDeclNodes)

+clang_tablegen(TypeNodes.def -gen-clang-type-nodes
+  SOURCE ../Basic/TypeNodes.td
+  TARGET ClangTypeNodes)
+
 clang_tablegen(CommentNodes.inc -gen-clang-comment-nodes
   SOURCE ../Basic/CommentNodes.td
   TARGET ClangCommentNodes)

Modified: cfe/trunk/include/clang/AST/Type.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=373407&r1=373406&r2=373407&view=diff

==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Tue Oct  1 16:13:03 2019
@@ -1437,10 +1437,9 @@ class alignas(8) Type : public ExtQualsT
 public:
   enum TypeClass {
 #define TYPE(Class, Base) Class,
-#define LAST_TYPE(Class) TypeLast = Class,
+#define LAST_TYPE(Class) TypeLast = Class
 #define ABSTRACT_TYPE(Class, Base)
 #include "clang/AST/TypeNodes.def"
-TagFirst = Record, TagLast = Enum
   };

 private:
@@ -4436,7 +4435,7 @@ public:
   bool isBeingDefined() const;

   static bool classof(const Type *T) {
-return T->getTypeClass() >= TagFirst && T->getTypeClass() <= 
TagLast;

+return T->getTypeClass() == Enum || T->getTypeClass() == Record;
   }
 };


Removed: cfe/trunk/include/clang/AST/TypeNodes.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TypeNodes.def?rev=373406&view=auto

==
--- cfe/trunk/include/clang/AST/TypeNodes.def (original)
+++ cfe/trunk/include/clang/AST/TypeNodes.def (removed)
@@ -1,135 +0,0 @@
-//===-- TypeNodes.def - Metadata about Type AST nodes ---*- 
C++

-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//

-//===--===//
-//
-//  This file defines the AST type info database. Each type node is
-//  enumerated by providing its name (e.g., "Builtin" or "Enum") and
-//  base class (e.g., "Type" or "TagType"). Depending on where in 
the

-//  abstract syntax tree the type will show up, the enumeration uses
-//  one of five different macros:
-//
-//TYPE(Class, Base) - A type that can show up anywhere in the 
AST,
-//and might be dependent, canonical, or non-canonical. All 
clients

-//will need to understand these types.
-//
-//ABSTRACT_TYPE(Class, Base) - An abstract class 

r373425 - Rename TypeNodes.def to TypeNodes.inc for consistency across all

[John McCall via cfe-commits]

Author: rjmccall
Date: Tue Oct  1 23:35:23 2019
New Revision: 373425

URL: http://llvm.org/viewvc/llvm-project?rev=373425&view=rev
Log:
Rename TypeNodes.def to TypeNodes.inc for consistency across all
our autogenerated files.  NFC.

As requested by Nico Weber.

Modified:
cfe/trunk/include/clang/AST/ASTFwd.h
cfe/trunk/include/clang/AST/ASTTypeTraits.h
cfe/trunk/include/clang/AST/CMakeLists.txt
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/AST/TypeLoc.h
cfe/trunk/include/clang/AST/TypeLocNodes.def
cfe/trunk/include/clang/AST/TypeVisitor.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ASTDiagnostic.cpp
cfe/trunk/lib/AST/ASTTypeTraits.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypePrinter.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/AST/ASTFwd.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTFwd.h?rev=373425&r1=373424&r2=373425&view=diff
==
--- cfe/trunk/include/clang/AST/ASTFwd.h (original)
+++ cfe/trunk/include/clang/AST/ASTFwd.h Tue Oct  1 23:35:23 2019
@@ -24,7 +24,7 @@ class Stmt;
 #include "clang/AST/StmtNodes.inc"
 class Type;
 #define TYPE(DERIVED, BASE) class DERIVED##Type;
-#include "clang/AST/TypeNodes.def"
+#include "clang/AST/TypeNodes.inc"
 class CXXCtorInitializer;
 
 } // end namespace clang

Modified: cfe/trunk/include/clang/AST/ASTTypeTraits.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTTypeTraits.h?rev=373425&r1=373424&r2=373425&view=diff
==
--- cfe/trunk/include/clang/AST/ASTTypeTraits.h (original)
+++ cfe/trunk/include/clang/AST/ASTTypeTraits.h Tue Oct  1 23:35:23 2019
@@ -148,7 +148,7 @@ private:
 #include "clang/AST/StmtNodes.inc"
 NKI_Type,
 #define TYPE(DERIVED, BASE) NKI_##DERIVED##Type,
-#include "clang/AST/TypeNodes.def"
+#include "clang/AST/TypeNodes.inc"
 NKI_OMPClause,
 #define OPENMP_CLAUSE(TextualSpelling, Class) NKI_##Class,
 #include "clang/Basic/OpenMPKinds.def"
@@ -205,7 +205,7 @@ KIND_TO_KIND_ID(OMPClause)
 #define STMT(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED)
 #include "clang/AST/StmtNodes.inc"
 #define TYPE(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Type)
-#include "clang/AST/TypeNodes.def"
+#include "clang/AST/TypeNodes.inc"
 #define OPENMP_CLAUSE(TextualSpelling, Class) KIND_TO_KIND_ID(Class)
 #include "clang/Basic/OpenMPKinds.def"
 #undef KIND_TO_KIND_ID

Modified: cfe/trunk/include/clang/AST/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CMakeLists.txt?rev=373425&r1=373424&r2=373425&view=diff
==
--- cfe/trunk/include/clang/AST/CMakeLists.txt (original)
+++ cfe/trunk/include/clang/AST/CMakeLists.txt Tue Oct  1 23:35:23 2019
@@ -31,7 +31,7 @@ clang_tablegen(DeclNodes.inc -gen-clang-
   SOURCE ../Basic/DeclNodes.td
   TARGET ClangDeclNodes)
 
-clang_tablegen(TypeNodes.def -gen-clang-type-nodes
+clang_tablegen(TypeNodes.inc -gen-clang-type-nodes
   SOURCE ../Basic/TypeNodes.td
   TARGET ClangTypeNodes)
 

Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=373425&r1=373424&r2=373425&view=diff
==
--- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
+++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue Oct  1 23:35:23 2019
@@ -431,7 +431,7 @@ public:
 // Declare Traverse*() for all concrete Type classes.
 #define ABSTRACT_TYPE(CLASS, BASE)
 #define TYPE(CLASS, BASE) bool Traverse##CLASS##Type(CLASS##Type *T);
-#include "clang/AST/TypeNodes.def"
+#include "clang/AST/TypeNodes.inc"
   // The above header #undefs ABSTRACT_TYPE and TYPE upon exit.
 
   // Define WalkUpFrom*() and empty Visit*() for all Type classes.
@@ -444,7 +444,7 @@ public:
 return true;   
\
   }
\
   bool Visit##CLASS##Type(CLASS##Type *T) { return true; }
-#include "clang/AST/TypeNodes.def"
+#include "clang/AST/TypeNodes.inc"
 
 //  Methods on TypeLocs 
 // FIXME: this currently just calls the matching Type methods
@@ -460,7 +460,7 @@ 

Re: r373407 - Emit TypeNodes.def with tblgen.

[John McCall via cfe-commits]

On 1 Oct 2019, at 21:31, Nico Weber wrote:
I think that'd be nice; I believe I renamed one .def output with the 
same

history for the same reason a while ago.


r373425

John.



On Tue, Oct 1, 2019 at 9:25 PM John McCall  wrote:


On 1 Oct 2019, at 21:20, Nico Weber wrote:
All other tablegen outputs are called .inc instead of .def. Any 
reason

this
one's different?


This was an existing file; it’s just generated now instead of 
written
in source.  I was deliberately not changing anything about the 
existing

use sites.  That said, I could certainly go rename it as a follow-up
just to re-establish that consistent naming convention.

John.



On Tue, Oct 1, 2019 at 7:10 PM John McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:


Author: rjmccall
Date: Tue Oct  1 16:13:03 2019
New Revision: 373407

URL: http://llvm.org/viewvc/llvm-project?rev=373407&view=rev
Log:
Emit TypeNodes.def with tblgen.

The primary goal here is to make the type node hierarchy available 
to

other tblgen backends, although it should also make it easier to
generate
more selective x-macros in the future.

Because tblgen doesn't seem to allow backends to preserve the 
source

order of defs, this is not NFC because it significantly re-orders
IDs.
I've fixed the one (fortunately obvious) place where we relied on
the old order.  Unfortunately, I wasn't able to share code with the
existing AST-node x-macro generators because the x-macro schema we
use
for types is different in a number of ways.  The main loss is that
subclasses aren't ordered together, which doesn't seem important 
for

types because the hierarchy is generally very shallow with little
clustering.

Added:
cfe/trunk/include/clang/Basic/TypeNodes.td
cfe/trunk/utils/TableGen/ClangTypeNodesEmitter.cpp
Removed:
cfe/trunk/include/clang/AST/TypeNodes.def
Modified:
cfe/trunk/include/clang/AST/CMakeLists.txt
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Modified: cfe/trunk/include/clang/AST/CMakeLists.txt
URL:


http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CMakeLists.txt?rev=373407&r1=373406&r2=373407&view=diff




==

--- cfe/trunk/include/clang/AST/CMakeLists.txt (original)
+++ cfe/trunk/include/clang/AST/CMakeLists.txt Tue Oct  1 16:13:03
2019
@@ -31,6 +31,10 @@ clang_tablegen(DeclNodes.inc -gen-clang-
   SOURCE ../Basic/DeclNodes.td
   TARGET ClangDeclNodes)

+clang_tablegen(TypeNodes.def -gen-clang-type-nodes
+  SOURCE ../Basic/TypeNodes.td
+  TARGET ClangTypeNodes)
+
 clang_tablegen(CommentNodes.inc -gen-clang-comment-nodes
   SOURCE ../Basic/CommentNodes.td
   TARGET ClangCommentNodes)

Modified: cfe/trunk/include/clang/AST/Type.h
URL:


http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=373407&r1=373406&r2=373407&view=diff




==

--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Tue Oct  1 16:13:03 2019
@@ -1437,10 +1437,9 @@ class alignas(8) Type : public ExtQualsT
 public:
   enum TypeClass {
 #define TYPE(Class, Base) Class,
-#define LAST_TYPE(Class) TypeLast = Class,
+#define LAST_TYPE(Class) TypeLast = Class
 #define ABSTRACT_TYPE(Class, Base)
 #include "clang/AST/TypeNodes.def"
-TagFirst = Record, TagLast = Enum
   };

 private:
@@ -4436,7 +4435,7 @@ public:
   bool isBeingDefined() const;

   static bool classof(const Type *T) {
-return T->getTypeClass() >= TagFirst && T->getTypeClass() <=
TagLast;
+return T->getTypeClass() == Enum || T->getTypeClass() == 
Record;

   }
 };


Removed: cfe/trunk/include/clang/AST/TypeNodes.def
URL:


http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TypeNodes.def?rev=373406&view=auto




==

--- cfe/trunk/include/clang/AST/TypeNodes.def (original)
+++ cfe/trunk/include/clang/AST/TypeNodes.def (removed)
@@ -1,135 +0,0 @@
-//===-- TypeNodes.def - Metadata about Type AST nodes 
---*-

C++
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with 
LLVM

Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//



-//===--===//

-//
-//  This file defines the AST type info database. Each type node 
is
-//  enumerated by providing its name (e.g., "Builtin" or "Enum") 
and

-//  base class (e.g., "Type" or "TagType"). Depending on where in
the
-//  abstract syntax tree the type will show up, the enumeration 
uses

-//  one of five differen

Re: r373406 - Use scope qualifiers in Clang's tblgen backends to get useful

[John McCall via cfe-commits]

On 3 Oct 2019, at 17:22, David Blaikie wrote:
(mostly joking) any interest in seeing what it'd take to make 
Clang/LLVM
-Wmissing-prototype clean? (which would also catch the same sort of 
issues,

I think?)


Sounds like a nice project for someone else. :)

John.



On Tue, Oct 1, 2019 at 4:10 PM John McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:


Author: rjmccall
Date: Tue Oct  1 16:12:57 2019
New Revision: 373406

URL: http://llvm.org/viewvc/llvm-project?rev=373406&view=rev
Log:
Use scope qualifiers in Clang's tblgen backends to get useful
redeclaration checking.  NFC.

Modified:
cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp

cfe/trunk/utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
cfe/trunk/utils/TableGen/ClangDataCollectorsEmitter.cpp
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
cfe/trunk/utils/TableGen/ClangOpcodesEmitter.cpp
cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
cfe/trunk/utils/TableGen/ClangOptionDocEmitter.cpp
cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp
cfe/trunk/utils/TableGen/NeonEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp?rev=373406&r1=373405&r2=373406&view=diff

==
--- cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangASTNodesEmitter.cpp Tue Oct  1 
16:12:57

2019
@@ -10,6 +10,8 @@
 //

 
//===--===//

+#include "TableGenBackends.h"
+
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
@@ -173,15 +175,14 @@ void ClangASTNodesEmitter::run(raw_ostre
   OS << "#undef ABSTRACT_" << macroName(Root.getName()) << "\n";
 }

-namespace clang {
-void EmitClangASTNodes(RecordKeeper &RK, raw_ostream &OS,
-   const std::string &N, const std::string &S) {
+void clang::EmitClangASTNodes(RecordKeeper &RK, raw_ostream &OS,
+  const std::string &N, const 
std::string &S)

{
   ClangASTNodesEmitter(RK, N, S).run(OS);
 }

 // Emits and addendum to a .inc file to enumerate the clang 
declaration

 // contexts.
-void EmitClangDeclContext(RecordKeeper &Records, raw_ostream &OS) {
+void clang::EmitClangDeclContext(RecordKeeper &Records, raw_ostream 
&OS) {
   // FIXME: Find a .td file format to allow for this to be 
represented

better.

   emitSourceFileHeader("List of AST Decl nodes", OS);
@@ -225,4 +226,3 @@ void EmitClangDeclContext(RecordKeeper &
   OS << "#undef DECL_CONTEXT\n";
   OS << "#undef DECL_CONTEXT_BASE\n";
 }
-} // end namespace clang

Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=373406&r1=373405&r2=373406&view=diff

==
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Tue Oct  1 16:12:57 
2019

@@ -10,6 +10,8 @@
 //

 
//===--===//

+#include "TableGenBackends.h"
+
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -2219,10 +2221,8 @@ static void emitClangAttrThisIsaIdentifi
   OS << "#endif // CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST\n\n";
 }

-namespace clang {
-
 // Emits the class definitions for attributes.
-void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) {
+void clang::EmitClangAttrClass(RecordKeeper &Records, raw_ostream 
&OS) {

   emitSourceFileHeader("Attribute classes' definitions", OS);

   OS << "#ifndef LLVM_CLANG_ATTR_CLASSES_INC\n";
@@ -2491,7 +2491,7 @@ void EmitClangAttrClass(RecordKeeper &Re
 }

 // Emits the class method definitions for attributes.
-void EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
+void clang::EmitClangAttrImpl(RecordKeeper &Records, raw_ostream 
&OS) {
   emitSourceFileHeader("Attribute classes' member function 
definitions",

OS);

   std::vector Attrs = 
Records.getAllDerivedDefinitions("Attr");

@@ -2556,8 +2556,6 @@ void EmitClangAttrImpl(RecordKeeper &Rec
   EmitFunc("printPretty(OS, Policy)");
 }

-} // end namespace clang
-
 static void emitAttrList(raw_ostream &OS, StringRef Class,
  

Re: [PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

[John McCall via cfe-commits]

(Sorry for the delay in responding — I'm actually on vacation.)

On Tue, Apr 10, 2018 at 1:52 PM, David Blaikie  wrote:

> On Tue, Apr 10, 2018 at 10:20 AM John McCall  wrote:
>
>> Do you think they’re bad precedent?
>
>
> Somewhat, yes - though -Wparens is perhaps conflating a few cases too. I
> think the argument for the -Wparens for precedence is probably pretty good.
>

True.  I agree that -Wparentheses is really at least three and probably
four separate warnings, all bundled into one flag.  I was really only
thinking about the = for == warning, which is an important warning that
ends up being highly opinionated about how you write your code.

Do you have a replacement for that approach to warning about those
>> problems?
>
>
> If we were looking at a green field today (code that'd never seen the
> warning before), I don't think -Wparens for assignment in conditional would
> pass the bar (or at least the bar as Clang seemed to have 6-7 years ago
> when I would talk to Doug about warning ideas, as best as I understood the
> perspective being espoused back then). I suspect maybe in that world we
> would've found other warnings with lower-false-positive that might've been
> able to diagnose assignment-in-conditional a bit more precisely than "all
> assignments are so suspect you have to manually go and look at them to
> decide".
>

I think you might be misunderstanding -Wparentheses as primarily a warning
about confusing precedence rather than primarily a warning about
accidentally substituting = for ==.  This of course interacts with
precedence for the conditional operator because the assignment is no longer
parsed within the condition.   Admittedly, GCC's documentation doesn't
explain this well.


> Because they certainly were precedent for -Wfallthrough, and they
>> certainly catch a class of bugs at least as large and important as that
>> warning, and they certainly have exactly the same false-positive
>> characteristics as it does. I am really struggling to see a difference in
>> kind or even degree here.
>>
>> -Wself-assign is a much less important warning but it’s also far less
>> likely to see false positives, except in this pattern of unit tests for
>> data structures, which are not commonly-written code. As is, in fact,
>> evidenced by Google, a company full of programmers whom I think I can
>> fairly but affectionately describe as loving to reinvent data structures,
>> only seeing this warning eight times.
>>
>
> I think the 8 cases were in Chromium - at Google so far (& I'm not sure if
> that's across all of Google's codebase, or some subset) something in the
> 100-200 cases?
>

I see.  8 did seem rather low for all of Google.  And all 100-200 are false
positives?  Or have only the Chromium cases been investigated yet?

Nico's point that, so far, the only evidence we have is that this warning
> added false positives and didn't catch any real world bugs. Yeah, a small
> number/easy enough to cleanup, but all we have is the cost and no idea of
> the value. (usually when we roll out warnings - even the -Wparens and
> -Wfallthrough, we find checked in code that has those problems (& the false
> positive cases) & so we evaluate cost/benefit with that data)
>

I understand.

I still believe strongly that we should be warning here, but I'm certainly
open to allowing it to be disabled.  We did actually talk about this during
the review.  There are three general options here, not necessary exclusive:
1. We move it to a separate warning group.  I would argue that (a) this
should at least be a sub-group so that -Wself-assign turns it on by
default, and that (b) trivial self-assignments should still be triggered
under -Wself-assign, for C/C++ consistency if nothing else.
2. We find some way to turn it off by recognizing something about the code
that suggests that we're in test code.
3. We add a general -wtest (capitalization intentional) that says "this is
test code, please don't warn about code patterns that would be unusual in
normal code but might make sense in tests".  I'm positive I've seen other
examples of that, e.g. for some of our warnings around misuse of library
functions.  Many build systems make it fairly early to use different build
settings when building tests — IIRC that's true of Bazel.

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

Re: r328680 - [ObjC] Make C++ triviality type traits available to non-trivial C

[John McCall via cfe-commits]

> On Apr 9, 2018, at 3:47 PM, Akira Hatanaka  wrote:
> 
> 
>> On Apr 5, 2018, at 1:25 PM, John McCall > > wrote:
>> 
>> 
>> 
>>> On Apr 5, 2018, at 3:54 PM, Akira Hatanaka >> > wrote:
>>> 
>>> 
 On Apr 5, 2018, at 12:39 PM, John McCall >>> > wrote:
 
 
 
> On Apr 4, 2018, at 7:37 PM, Akira Hatanaka  > wrote:
> 
> 
> 
>> On Apr 4, 2018, at 4:24 PM, Akira Hatanaka via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>>> 
>>> On Apr 4, 2018, at 3:38 PM, Richard Smith >> > wrote:
>>> 
>>> Hi Akira,
>>> 
>>> This change has broken the C++ versions of these type traits for 
>>> classes with volatile members. Such classes are required to claim to be 
>>> trivial per C++ DR 2094 
>>> (http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2094 
>>> ) 
>>> but return false from isNonTrivialToPrimitiveCopy().
>>> 
>> 
>> Oh that’s right. The function returns false when 
>> isNonTrivialToPrimitiveCopy() returns PCK_VolatileTrivial. That is wrong.
>> 
>>> Also, exposing these __has_* traits more widely seems like a backwards 
>>> step to me: these traits are deprecated, near-useless, and we're trying 
>>> to remove them. No code should be using them under any circumstances; 
>>> the __is_* traits should be used instead.
>>> 
>> 
>> The __is_* traits (is_trivially_copy_constructible, etc.) are templates 
>> defined in libcxx, so it seems that we can’t use them when compiling in 
>> C mode. Is it OK to add their definitions to TokenKinds.def as non-C++ 
>> keywords?
>> 
> 
> Or perhaps redefine the six __has_* traits used here as non-C++ (KEYNOCXX 
> or ‘KEYC99 | KEYC11') keywords?
 
 I think Richard is talking about e.g. the __is_trivially_destructible 
 intrinsic type trait function rather than std::is_trivially_destructible.
 
 Do we have a concrete need to expose these type traits to C?
 
>>> 
>>> No, no one has asked any of these type traits to be exposed to C yet. Do 
>>> you think we should just revert the patch and wait until someone asks for 
>>> those type traits to be available in C?
>> 
>> I think that would be fine.  Although it would be nice if we could save the 
>> part of the patch that makes the trait logic sensitive to the type queries 
>> instead of needing to include duplicated checks for __strong, __weak, and 
>> whatever other non-trivial C features we eventually add.
>> 
> 
> Reverted r329289 in r329608. I wasn’t sure which part of the patch should be 
> saved. I don’t think we need to check the properties of the types calling the 
> QualType::isNonTrivialTo* functions since we aren’t exposing the type traits 
> to C anymore?

I mean that it would be nice if the queries could just consider the primitive 
type properties in the non-record case instead of having explicit extra checks 
for __strong/__weak and so on.

John.

> 
>> John.
>> 
>>> 
 John.
 
> 
>>> On 27 March 2018 at 17:12, Akira Hatanaka via cfe-commits 
>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>> Author: ahatanak
>>> Date: Tue Mar 27 17:12:08 2018
>>> New Revision: 328680
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=328680&view=rev 
>>> 
>>> Log:
>>> [ObjC] Make C++ triviality type traits available to non-trivial C
>>> structs.
>>> 
>>> r326307 and r327870 made changes that allowed using non-trivial C
>>> structs with fields qualified with __strong or __weak. This commit makes
>>> the following C++ triviality type traits available to non-trivial C
>>> structs:
>>> 
>>> __has_trivial_assign
>>> __has_trivial_move_assign
>>> __has_trivial_copy
>>> __has_trivial_move_constructor
>>> __has_trivial_constructor
>>> __has_trivial_destructor
>>> 
>>> rdar://problem/33599681 
>>> 
>>> Differential Revision: https://reviews.llvm.org/D44913 
>>> 
>>> 
>>> Added:
>>> cfe/trunk/test/SemaObjC/non-trivial-struct-traits.m
>>> Modified:
>>> cfe/trunk/include/clang/Basic/TokenKinds.def
>>> cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>> 
>>> Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=328680&r1=328679&r2=328680&view=diff
>>>  
>>> 
>>> ==
>>> --- cfe/t

Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

[John McCall via cfe-commits]

Let me try to summarize where I think we are.

1. I think it’s now generally agreed that this is a useful warning —
certainly for field assignments, but we also have (at least?) one example
with a local variable.

2. It’s also generally agreed that this warning has a problem with unit
tests and that we should provide a compiler option to suppress.

3. There isn’t really a middle-point between case-by-case suppression (by
rewriting the RHS to avoid the warning) and file-by-file (compiler option)
suppression. We don’t know how to distinguish the unit-test false positives
from the local-variable true positive.

4. Whatever the file-by-file suppression is, it would be best for build
systems to target it narrowly at their unit-test code; but we also have to
acknowledge that that’s not always straightforward, and so the suppression
should be narrowly-targeted on the compiler side as well, just in case the
suppression does have to be more global.

5. Roman and I are suggesting that the file-by-file suppression should not
be specific to this warning but instead should be a more generic way of
disabling warnings that are known to be problematic in unit tests. We could
then recommend that option to project owners as a single mitigation rather
than forcing them to maintain a growing list of test-directed warning
suppressions.

6. Using a more general mechanism seems advisable because we are already
considering extending some other warnings to cover user-defined operators,
and all such warnings have the potential of running afoul of unit tests.
(This is something that will require careful thought because user-defined
operators need not have their conventional meaning. However, any
semantics-based suppression will almost certainly have different
characteristics from a test-directed suppression. For example,
test-directed suppression for self-assign need not suppress warnings about
fields, whereas a semantics-based suppression should.)

John.
On Wed, Apr 18, 2018 at 13:54 John McCall via Phabricator <
revi...@reviews.llvm.org> wrote:

> rjmccall added a comment.
>
> I'm sorry, that warning *is* in self-assign, although I'm not sure it
> should be.
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D45685
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

[John McCall via cfe-commits]

On Mon, Apr 23, 2018 at 6:32 PM, David Blaikie  wrote:

> On Mon, Apr 23, 2018 at 3:29 PM John McCall via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> rjmccall added a comment.
>>
>> In https://reviews.llvm.org/D45766#1076176, @dblaikie wrote:
>>
>> > Is there anything else in the "-w" namespace other than the literal
>> "-w" so
>> >  far?
>>
>>
>> No. This would be novel.
>>
>
> Ah, I see.
>
>
>> > I mean, I could imagine it might make more sense to default these
>> warnings
>> >  off & users can turn them on for non-test code, potentially? So
>> >  "-Wnon-test" might make sense.
>>
>> That's an interesting idea, but it's still not a warning group, because
>> you shouldn't get the self-assign warnings unless `-Wself-assign` is
>> enabled.
>>
>
> You shouldn't?
>

I wouldn't think so.  Remember that the goal of the option is to be a
single thing that users can add to their unit-test CFLAGS to disable these
noisy-in-tests cases.  So if we add an opt-in/experimental
`-Wunpredictable-foozits` warning, and it has a unit-test carve-out,
passing `-wtest -wno-test` or whatever shouldn't turn on the carved-out
special case of `-Wunpredictable-foozits`.

It's probably not the worst thing to just use a `-W` spelling anyway; not
everything in that namespace is (e.g. `-Werror`).  It could be
`-Wnoisy-in-tests` and `-Wno-noisy-in-tests`, with a documentation note
that `-Wnoisy-in-tests` is just a cancellation of `-Wno-noisy-in-tests` and
doesn't actually enable any warnings by itself.  We could have the
diagnostic printer add `-Wnoisy-in-tests` to the diagnostic-group
annotation for diagnostics that would be suppressed under
`-Wno-noisy-in-tests`, analogously to how it adds `-Werror` for diagnostics
that have been promoted to an error.

John.


> But yeah, it's tricky either way - either you get them all, then opt out
> of all the warnings for test code you don't generally want.
>
> I'll leave it to you, then - don't feel too strongly. Maybe worth seeing
> if Richard has an opinion, but up to you.
>
> - Dave
>
>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D45766
>>
>>
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

[John McCall via cfe-commits]

On Mon, Apr 23, 2018 at 8:23 PM, Richard Smith 
wrote:

> On 23 April 2018 at 16:23, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On Mon, Apr 23, 2018 at 4:12 PM John McCall  wrote:
>>
>>> On Mon, Apr 23, 2018 at 6:32 PM, David Blaikie 
>>> wrote:
>>>
 On Mon, Apr 23, 2018 at 3:29 PM John McCall via Phabricator <
 revi...@reviews.llvm.org> wrote:

> rjmccall added a comment.
>
> In https://reviews.llvm.org/D45766#1076176, @dblaikie wrote:
>
> > Is there anything else in the "-w" namespace other than the literal
> "-w" so
> >  far?
>
>
> No. This would be novel.
>

 Ah, I see.


> > I mean, I could imagine it might make more sense to default these
> warnings
> >  off & users can turn them on for non-test code, potentially? So
> >  "-Wnon-test" might make sense.
>
> That's an interesting idea, but it's still not a warning group,
> because you shouldn't get the self-assign warnings unless `-Wself-assign`
> is enabled.
>

 You shouldn't?

>>>
>>> I wouldn't think so.  Remember that the goal of the option is to be a
>>> single thing that users can add to their unit-test CFLAGS to disable these
>>> noisy-in-tests cases.  So if we add an opt-in/experimental
>>> `-Wunpredictable-foozits` warning, and it has a unit-test carve-out,
>>> passing `-wtest -wno-test` or whatever shouldn't turn on the carved-out
>>> special case of `-Wunpredictable-foozits`.
>>>
>>> It's probably not the worst thing to just use a `-W` spelling anyway;
>>> not everything in that namespace is (e.g. `-Werror`).  It could be
>>> `-Wnoisy-in-tests` and `-Wno-noisy-in-tests`, with a documentation note
>>> that `-Wnoisy-in-tests` is just a cancellation of `-Wno-noisy-in-tests` and
>>> doesn't actually enable any warnings by itself.  We could have the
>>> diagnostic printer add `-Wnoisy-in-tests` to the diagnostic-group
>>> annotation for diagnostics that would be suppressed under
>>> `-Wno-noisy-in-tests`, analogously to how it adds `-Werror` for diagnostics
>>> that have been promoted to an error.
>>>
>>
>> That sort of sounds pretty plausible to me. Poked Richard about his
>> opinion here too.
>>
>
> This is not the only warning group that has the property that only one of
> -Wfoo and -Wno-foo seems useful. There are, for instance, plenty of
> diagnostic groups that make sense to turn on, but not to turn off (eg,
> -Wno-all, -Wno-extra are largely meaningless), and some that make sense to
> turn off, but not to turn on (eg, -Wnon-gcc is only intended to be turned
> off). So I don't think that's as special a property as is being suggested.
> If someone uses -Wnoisy-in-tests and it turns on all warnings that are
> noisy in tests, I think they got what they asked for.
>

The issue there is that -Wnoisy-in-tests is likely to be useful as a
cancellation of -Wno-noisy-in-tests, which (as David suggests) might
reasonably be set by default by a build system.  That's completely defeated
if it potentially enables a bunch of unwanted warnings.

This is also not the only warning group for which we want diagnostics to be
> in this group and in some other group or groups. I think the idea to
> include -Wnoisy-in-tests in the diagnostic output is very interesting, but
> as a general feature not as a -Wnoisy-in-tests special case -- for example,
> if I use a binary literal in C++98 mode, I'd like the warning to be tagged
> with [-Wbinary-literal,-Wc++14-extensions,-Wgnu], rather than the current
> [-Wc++14-binary-literal] (an artificial warning group that only exists to
> work around the inability of our infrastructure to properly represent
> warnings that are in multiple groups at once).
>

> As a simple way to get this effect, perhaps we could allow warning groups
> to be tagged as artificial, and for such warning groups, recurse to the
> warning groups including them to find the name to use for diagnostics.
>

That's a really good idea.  I don't think it's a good match for this, but I
completely agree that there's a separate problem of presenting artificial
diagnostic groups to users.

I'm going to quote your other post into this thread in the interest of
making it easier for posterity to read a unified discussion.

I would feel a lot more comfortable adding a `-wtest` flag if we had more
> than one warning that it would control. If there's really only one warning
> out of >600 that qualifies for this treatment, I really don't think we have
> a good argument to introduce a new feature here, and it'll just be dead
> weight we're carrying forever. My inclination is to say that the single
> `-Wno-` flag is sufficient until we actually have multiple warnings that
> this would control, and this is a premature generalization until that
> point. ("We're just about to add these warnings" carries a lot more weight
> for me here than "We have ideas of warnings we might add", but if we're in
> the former situation, there

Re: [PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via cfe-commits]

I think you and Richard agreed that you weren’t going to synthesize a whole
expression tree at every use of the operator, and I agree with that
decision. That’s very different from what I’m asking you to do, which is to
synthesize in isolation a call to the copy-constructor. There are several
places in the compiler that require these implicit copies which aren’t just
normal expressions; this is the common pattern for handling them. The
synthesized expression can be emitted multiple times, and it can be freely
re-synthesized in different translation units instead of being serialized.
You’re already finding and caching a constructor; storing a
CXXConstructExpr is basically thr same thing, but in a nicer form that
handles more cases and doesn’t require as much redundant code in IRGen.

STLs *frequently* make use of default arguments on copy constructors (for
allocators). I agree that it’s unlikely that that would happen here, but
that’s precisely because it’s unlikely that this type would ever be
non-trivial.

Mostly, though, I don’t understand the point of imposing a partial set of
non-conformant restrictions on the type. It’s easy to support an arbitrary
copy constructor by synthesizing a CXXConstructExpr, and this will
magically take care of any constexpr issues, as well as removing the need
for open-coding a constructor call.

The constexpr issues are that certain references to constexpr variables of
literal type (as these types are likely to be) are required to not ODR-use
the variable but instead just directly produce the initializer as the
expression result.  That’s especially important here because (1) existing
STL binaries will not define these variables, and we shouldn’t create
artificial deployment problems for this feature, and (2) we’d really rather
not emit these expressions as loads from externally-defined variables that
the optimizer won’t be able to optimize.

John.

Eric Fiselier via Phabricator  wrote:

> EricWF added inline comments.
>
>
> 
> Comment at: lib/Sema/SemaDeclCXX.cpp:8931
> +  /*ConstArg*/ true, false, false, false, false);
> +  auto *CopyCtor = cast_or_null(SMI.getMethod());
> +
> 
> rjmccall wrote:
> > Sorry, I didn't realize you'd go off and write this code manually.
> >
> > The way we generally handle this sort of thing is just to synthesize an
> expression in Sema that performs the copy-construction.  That way, the
> stdlib can be as crazy as it wants — default arguments on the
> copy-constructor or whatever else — and it just works.  The pattern to
> follow here is the code in Sema::BuildExceptionDeclaration, except that in
> your case you can completely dispense with faking up an OpaqueValueExpr and
> instead just build a DeclRefExpr to the actual variable.  (You could even
> use ActOnIdExpression for this, although just synthesizing the DRE
> shouldn't be too hard.)  Then the ComparisonCategoryInfo can just store
> expressions for each of the three (four?) variables, and in IRGen you just
> evaluate the appropriate one into the destination.
> I think this goes against the direction Richard and I decided to go, which
> was to not synthesize any expressions.
>
> The problem is that the synthesized expressions cannot be stored in
> `ComparisonCategoryInfo`, because the data it contains is not serialized.
> So when we read the AST back, the `ComparisonCategoryInfo` will be empty.
> And for obvious reasons we can't cache the data in Sema either.
> Additionally, we probably don't want to waste space building and storing
> synthesized expressions in each AST node which needs them.
>
> Although the checking here leaves something to be desired, I suspect it's
> more than enough to handle any conforming STL implementation.
>
>
>
>
> https://reviews.llvm.org/D45476
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] baf91d0 - [NFC] Add a tablegen node for the root of the AST node hierarchies.

[John McCall via cfe-commits]

Author: John McCall
Date: 2019-10-25T16:39:21-07:00
New Revision: baf91d02da6e68c4ee6723ef68911fcd80ece6a5

URL: 
https://github.com/llvm/llvm-project/commit/baf91d02da6e68c4ee6723ef68911fcd80ece6a5
DIFF: 
https://github.com/llvm/llvm-project/commit/baf91d02da6e68c4ee6723ef68911fcd80ece6a5.diff

LOG: [NFC] Add a tablegen node for the root of the AST node hierarchies.

This is useful for the property databases we want to add for abstract
serialization, since root classes can have interesting properties.

Added: 
clang/utils/TableGen/ClangASTEmitters.h

Modified: 
clang/include/clang/Basic/CommentNodes.td
clang/include/clang/Basic/DeclNodes.td
clang/include/clang/Basic/StmtNodes.td
clang/include/clang/Basic/TypeNodes.td
clang/utils/TableGen/ClangASTNodesEmitter.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp
clang/utils/TableGen/ClangTypeNodesEmitter.cpp
clang/utils/TableGen/TableGen.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CommentNodes.td 
b/clang/include/clang/Basic/CommentNodes.td
index 7bf32b78b5b6..e58ff4c98bd5 100644
--- a/clang/include/clang/Basic/CommentNodes.td
+++ b/clang/include/clang/Basic/CommentNodes.td
@@ -1,27 +1,25 @@
-class Comment {
+class CommentNode {
+  CommentNode Base = base;
   bit Abstract = abstract;
 }
 
-class DComment : Comment {
-  Comment Base = base;
-}
-
-def InlineContentComment : Comment<1>;
-  def TextComment : DComment;
-  def InlineCommandComment : DComment;
-  def HTMLTagComment : DComment;
-def HTMLStartTagComment : DComment;
-def HTMLEndTagComment : DComment;
+def Comment : CommentNode;
+def InlineContentComment : CommentNode;
+  def TextComment : CommentNode;
+  def InlineCommandComment : CommentNode;
+  def HTMLTagComment : CommentNode;
+def HTMLStartTagComment : CommentNode;
+def HTMLEndTagComment : CommentNode;
 
-def BlockContentComment : Comment<1>;
-  def ParagraphComment : DComment;
-  def BlockCommandComment : DComment;
-def ParamCommandComment : DComment;
-def TParamCommandComment : DComment;
-def VerbatimBlockComment : DComment;
-def VerbatimLineComment : DComment;
+def BlockContentComment : CommentNode;
+  def ParagraphComment : CommentNode;
+  def BlockCommandComment : CommentNode;
+def ParamCommandComment : CommentNode;
+def TParamCommandComment : CommentNode;
+def VerbatimBlockComment : CommentNode;
+def VerbatimLineComment : CommentNode;
 
-def VerbatimBlockLineComment : Comment;
+def VerbatimBlockLineComment : CommentNode;
 
-def FullComment : Comment;
+def FullComment : CommentNode;
 

diff  --git a/clang/include/clang/Basic/DeclNodes.td 
b/clang/include/clang/Basic/DeclNodes.td
index 2d3fa6b6147f..fa4d1b4f47e7 100644
--- a/clang/include/clang/Basic/DeclNodes.td
+++ b/clang/include/clang/Basic/DeclNodes.td
@@ -1,105 +1,103 @@
 class AttrSubject;
 
-class Decl : AttrSubject {
+class DeclNode
+: AttrSubject {
+  DeclNode Base = base;
   bit Abstract = abstract;
   string DiagSpelling = diagSpelling;
 }
 
-class DDecl
-: Decl {
-  Decl Base = base;
-}
-
 class DeclContext {}
 
-def TranslationUnit : Decl, DeclContext;
-def PragmaComment : Decl;
-def PragmaDetectMismatch : Decl;
-def ExternCContext : Decl, DeclContext;
-def Named : Decl<"named declarations", 1>;
-  def Namespace : DDecl, DeclContext;
-  def UsingDirective : DDecl;
-  def NamespaceAlias : DDecl;
-  def Label : DDecl;
-  def Type : DDecl;
-def TypedefName : DDecl;
-  def Typedef : DDecl;
-  def TypeAlias : DDecl;
-  def ObjCTypeParam : DDecl;
-def UnresolvedUsingTypename : DDecl;
-def Tag : DDecl, DeclContext;
-  def Enum : DDecl;
-  def Record : DDecl;
-def CXXRecord : DDecl;
-  def ClassTemplateSpecialization : DDecl;
+def Decl : DeclNode;
+def TranslationUnit : DeclNode, DeclContext;
+def PragmaComment : DeclNode;
+def PragmaDetectMismatch : DeclNode;
+def ExternCContext : DeclNode, DeclContext;
+def Named : DeclNode;
+  def Namespace : DeclNode, DeclContext;
+  def UsingDirective : DeclNode;
+  def NamespaceAlias : DeclNode;
+  def Label : DeclNode;
+  def Type : DeclNode;
+def TypedefName : DeclNode;
+  def Typedef : DeclNode;
+  def TypeAlias : DeclNode;
+  def ObjCTypeParam : DeclNode;
+def UnresolvedUsingTypename : DeclNode;
+def Tag : DeclNode, DeclContext;
+  def Enum : DeclNode;
+  def Record : DeclNode;
+def CXXRecord : DeclNode;
+  def ClassTemplateSpecialization : DeclNode;
 def ClassTemplatePartialSpecialization
-  : DDecl;
-def TemplateTypeParm : DDecl;
-  def Value : DDecl;
-def EnumConstant : DDecl;
-def UnresolvedUsingValue : DDecl;
-def IndirectField : DDecl;
-def Binding : DDecl;
-def OMPDeclareReduction : DDecl, DeclContext;
-def OMPDeclareMapper : DDecl, DeclContext;
-def Declarator : DDecl;
-  def Field : DDecl;
-d

[clang] 8a8d703 - Fix how cc1 command line options are mapped into FP options.

[John McCall via cfe-commits]

Author: John McCall
Date: 2020-06-01T22:00:30-04:00
New Revision: 8a8d703be0986dd6785cba0b610c9c4708b83e89

URL: 
https://github.com/llvm/llvm-project/commit/8a8d703be0986dd6785cba0b610c9c4708b83e89
DIFF: 
https://github.com/llvm/llvm-project/commit/8a8d703be0986dd6785cba0b610c9c4708b83e89.diff

LOG: Fix how cc1 command line options are mapped into FP options.

Canonicalize on storing FP options in LangOptions instead of
redundantly in CodeGenOptions.  Incorporate -ffast-math directly
into the values of those LangOptions rather than considering it
separately when building FPOptions.  Build IR attributes from
those options rather than a mix of sources.

We should really simplify the driver/cc1 interaction here and have
the driver pass down options that cc1 directly honors.  That can
happen in a follow-up, though.

Patch by Michele Scandale!
https://reviews.llvm.org/D80315

Added: 
clang/test/CodeGen/fp-options-to-fast-math-flags.c

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/LangOptions.h
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/builtins-nvptx-ptx60.cu
clang/test/CodeGen/complex-math.c
clang/test/CodeGen/libcalls.c
clang/test/CodeGenCUDA/builtins-amdgcn.cu
clang/test/CodeGenCUDA/library-builtin.cu
clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 363ee1eb16ac..d20282316b64 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -150,13 +150,7 @@ CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug 
info should contain
  ///< inline line tables.
 CODEGENOPT(StackClashProtector, 1, 0) ///< Set when -fstack-clash-protection 
is enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
-CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.
-CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP 
zero
 CODEGENOPT(NullPointerIsValid , 1, 0) ///< Assume Null pointer deference is 
defined.
-CODEGENOPT(Reassociate   , 1, 0) ///< Allow reassociation of FP math ops
-CODEGENOPT(ReciprocalMath, 1, 0) ///< Allow FP divisions to be 
reassociated.
-CODEGENOPT(NoTrappingMath, 1, 0) ///< Set when -fno-trapping-math is 
enabled.
-CODEGENOPT(NoNaNsFPMath  , 1, 0) ///< Assume FP arguments, results not NaN.
 CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< 
-cl-fp32-correctly-rounded-divide-sqrt
 CODEGENOPT(UniqueInternalLinkageNames, 1, 0) ///< Internal Linkage symbols get 
unique names.
 
@@ -248,7 +242,6 @@ VALUE_CODEGENOPT(TimeTraceGranularity, 32, 500) ///< 
Minimum time granularity (i
 CODEGENOPT(UnrollLoops   , 1, 0) ///< Control whether loops are unrolled.
 CODEGENOPT(RerollLoops   , 1, 0) ///< Control whether loops are rerolled.
 CODEGENOPT(NoUseJumpTables   , 1, 0) ///< Set when -fno-jump-tables is enabled.
-CODEGENOPT(UnsafeFPMath  , 1, 0) ///< Allow unsafe floating point optzns.
 CODEGENOPT(UnwindTables  , 1, 0) ///< Emit unwind tables.
 CODEGENOPT(VectorizeLoop , 1, 0) ///< Run loop vectorizer.
 CODEGENOPT(VectorizeSLP  , 1, 0) ///< Run SLP vectorizer.

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index bee463a0f890..ef56a78b7d48 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -384,12 +384,10 @@ class FPOptions {
 fenv_access(LangOptions::FPM_Off),
 rounding(static_cast(LangOpts.getFPRoundingMode())),
 exceptions(LangOpts.getFPExceptionMode()),
-allow_reassoc(LangOpts.FastMath || LangOpts.AllowFPReassoc),
-no_nans(LangOpts.FastMath || LangOpts.NoHonorNaNs),
-no_infs(LangOpts.FastMath || LangOpts.NoHonorInfs),
-no_signed_zeros(LangOpts.FastMath || LangOpts.NoSignedZero),
-allow_reciprocal(LangOpts.FastMath || LangOpts.AllowRecip),
-approx_func(LangOpts.FastMath || LangOpts.ApproxFunc) {}
+allow_reassoc(LangOpts.AllowFPReassoc), no_nans(LangOpts.NoHonorNaNs),
+no_infs(LangOpts.NoHonorInfs), no_signed_zeros(LangOpts.NoSignedZero),
+allow_reciprocal(LangOpts.AllowRecip),
+approx_func(LangOpts.ApproxFunc) {}
   // FIXME: Use getDefaultFEnvAccessMode() when available.
 
   void setFastMath(bool B = true) {

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index dd5016333920..fdf699cc28e0 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -

[clang] 7fac1ac - Set the LLVM FP optimization flags conservatively.

[John McCall via cfe-commits]

Author: John McCall
Date: 2020-06-11T18:16:41-04:00
New Revision: 7fac1acc617113b7a3276ee0f0664bedca978292

URL: 
https://github.com/llvm/llvm-project/commit/7fac1acc617113b7a3276ee0f0664bedca978292
DIFF: 
https://github.com/llvm/llvm-project/commit/7fac1acc617113b7a3276ee0f0664bedca978292.diff

LOG: Set the LLVM FP optimization flags conservatively.

Functions can have local pragmas that override the global settings.
We set the flags eagerly based on global settings, but if we emit
an expression under the influence of a pragma, we clear the
appropriate flags from the function.

In order to avoid doing a ton of redundant work whenever we emit
an FP expression, configure the IRBuilder to default to global
settings, and only reconfigure it when we see an FP expression
that's not using the global settings.

Patch by Michele Scandale!

https://reviews.llvm.org/D80462

Added: 
clang/test/CodeGen/fp-function-attrs.cpp

Modified: 
clang/include/clang/Basic/LangOptions.h
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index ef56a78b7d48..7b92a8964862 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -529,6 +529,13 @@ class FPOptions {
   unsigned approx_func : 1;
 };
 
+inline bool operator==(FPOptions LHS, FPOptions RHS) {
+  return LHS.getAsOpaqueInt() == RHS.getAsOpaqueInt();
+}
+inline bool operator!=(FPOptions LHS, FPOptions RHS) {
+  return LHS.getAsOpaqueInt() != RHS.getAsOpaqueInt();
+}
+
 /// Describes the kind of translation unit being processed.
 enum TranslationUnitKind {
   /// The translation unit is a complete translation unit.

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index b2bc38b329ef..83614b031543 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -215,23 +215,6 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx, 
const BinOpInfo &Op) {
  (2 * Ctx.getTypeSize(RHSTy)) < PromotedSize;
 }
 
-static void setBuilderFlagsFromFPFeatures(CGBuilderTy &Builder,
-  CodeGenFunction &CGF,
-  FPOptions FPFeatures) {
-  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
-  Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
-  auto NewExceptionBehavior =
-  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
-  Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
-  CGF.SetFastMathFlags(FPFeatures);
-  assert((CGF.CurFuncDecl == nullptr || Builder.getIsFPConstrained() ||
-  isa(CGF.CurFuncDecl) ||
-  isa(CGF.CurFuncDecl) ||
-  (NewExceptionBehavior == llvm::fp::ebIgnore &&
-   NewRoundingBehavior == llvm::RoundingMode::NearestTiesToEven)) &&
- "FPConstrained should be enabled on entire function");
-}
-
 class ScalarExprEmitter
   : public StmtVisitor {
   CodeGenFunction &CGF;
@@ -764,8 +747,7 @@ class ScalarExprEmitter
 
 if (Ops.LHS->getType()->isFPOrFPVectorTy()) {
   //  Preserve the old values
-  llvm::IRBuilder<>::FastMathFlagGuard FMFG(Builder);
-  setBuilderFlagsFromFPFeatures(Builder, CGF, Ops.FPFeatures);
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Ops.FPFeatures);
   return Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul");
 }
 if (Ops.isFixedPointOp())
@@ -2769,9 +2751,8 @@ Value *ScalarExprEmitter::VisitUnaryLNot(const 
UnaryOperator *E) {
 Value *Zero = llvm::Constant::getNullValue(Oper->getType());
 Value *Result;
 if (Oper->getType()->isFPOrFPVectorTy()) {
-  llvm::IRBuilder<>::FastMathFlagGuard FMFG(Builder);
-  setBuilderFlagsFromFPFeatures(Builder, CGF,
-E->getFPFeatures(CGF.getLangOpts()));
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(
+  CGF, E->getFPFeatures(CGF.getLangOpts()));
   Result = Builder.CreateFCmp(llvm::CmpInst::FCMP_OEQ, Oper, Zero, "cmp");
 } else
   Result = Builder.CreateICmp(llvm::CmpInst::ICMP_EQ, Oper, Zero, "cmp");
@@ -3183,8 +3164,7 @@ Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) {
 
   if (Ops.LHS->getType()->isFPOrFPVectorTy()) {
 llvm::Value *Val;
-llvm::IRBuilder<>::FastMathFlagGuard FMFG(Builder);
-setBuilderFlagsFromFPFeatures(Builder, CGF, Ops.FPFeatures);
+CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Ops.FPFeatures);
 Val = Builder.CreateFDiv(Ops.LHS, Ops.RHS, "div");
 if (CGF.getLangOpts().OpenCL &&
 !CGF.CGM.getCodeGenOpts().CorrectlyRoundedDivSqrt) {
@@ -3562,8 +3542,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
 return EmitOverflowCheckedBinOp(op);
 
   if (op.LHS->getType()->isFPOrFPVectorTy()) {
-llvm::

[clang] 32870a8 - Expose IRGen API to add the default IR attributes to a function definition.

[John McCall via cfe-commits]

Author: John McCall
Date: 2020-05-16T14:44:54-04:00
New Revision: 32870a84d9a40ea682e22a24b5f0d1a218c3b062

URL: 
https://github.com/llvm/llvm-project/commit/32870a84d9a40ea682e22a24b5f0d1a218c3b062
DIFF: 
https://github.com/llvm/llvm-project/commit/32870a84d9a40ea682e22a24b5f0d1a218c3b062.diff

LOG: Expose IRGen API to add the default IR attributes to a function definition.

I've also made a stab at imposing some more order on where and how we add
attributes; this part should be NFC.  I wasn't sure whether the CUDA use
case for libdevice should propagate CPU/features attributes, so there's a
bit of unnecessary duplication.

Added: 


Modified: 
clang/include/clang/CodeGen/CodeGenABITypes.h
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CodeGenABITypes.cpp
clang/lib/CodeGen/CodeGenAction.cpp
clang/lib/CodeGen/CodeGenModule.h

Removed: 




diff  --git a/clang/include/clang/CodeGen/CodeGenABITypes.h 
b/clang/include/clang/CodeGen/CodeGenABITypes.h
index 5f4af7fd2a36..255e1603509e 100644
--- a/clang/include/clang/CodeGen/CodeGenABITypes.h
+++ b/clang/include/clang/CodeGen/CodeGenABITypes.h
@@ -28,6 +28,7 @@
 #include "clang/CodeGen/CGFunctionInfo.h"
 
 namespace llvm {
+class AttrBuilder;
 class Constant;
 class DataLayout;
 class Module;
@@ -86,6 +87,25 @@ llvm::Type *convertTypeForMemory(CodeGenModule &CGM, 
QualType T);
 unsigned getLLVMFieldNumber(CodeGenModule &CGM,
 const RecordDecl *RD, const FieldDecl *FD);
 
+/// Given the language and code-generation options that Clang was configured
+/// with, set the default LLVM IR attributes for a function definition.
+/// The attributes set here are mostly global target-configuration and
+/// pipeline-configuration options like the target CPU, variant stack
+/// rules, whether to optimize for size, and so on.  This is useful for
+/// frontends (such as Swift) that generally intend to interoperate with
+/// C code and rely on Clang's target configuration logic.
+///
+/// As a general rule, this function assumes that meaningful attributes
+/// haven't already been added to the builder.  It won't intentionally
+/// displace any existing attributes, but it also won't check to avoid
+/// overwriting them.  Callers should generally apply customizations after
+/// making this call.
+///
+/// This function assumes that the caller is not defining a function that
+/// requires special no-builtin treatment.
+void addDefaultFunctionDefinitionAttributes(CodeGenModule &CGM,
+llvm::AttrBuilder &attrs);
+
 /// Returns the default constructor for a C struct with non-trivially copyable
 /// fields, generating it if necessary. The returned function uses the `cdecl`
 /// calling convention, returns void, and takes a single argument that is a

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 068d053d17cc..b8bd8a780e50 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1700,8 +1700,9 @@ static void AddAttributesFromFunctionProtoType(ASTContext 
&Ctx,
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
 }
 
-void CodeGenModule::ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone,
-   bool AttrOnCallSite,
+void CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
+ bool HasOptnone,
+ bool AttrOnCallSite,
llvm::AttrBuilder &FuncAttrs) {
   // OptimizeNoneAttr takes precedence over -Os or -Oz. No warning needed.
   if (!HasOptnone) {
@@ -1796,6 +1797,8 @@ void CodeGenModule::ConstructDefaultFnAttrList(StringRef 
Name, bool HasOptnone,
   FuncAttrs.addAttribute("stackrealign");
 if (CodeGenOpts.Backchain)
   FuncAttrs.addAttribute("backchain");
+if (CodeGenOpts.EnableSegmentedStacks)
+  FuncAttrs.addAttribute("split-stack");
 
 if (CodeGenOpts.SpeculativeLoadHardening)
   FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening);
@@ -1822,13 +1825,21 @@ void 
CodeGenModule::ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone,
   }
 }
 
-void CodeGenModule::AddDefaultFnAttrs(llvm::Function &F) {
+void CodeGenModule::addDefaultFunctionDefinitionAttributes(llvm::Function &F) {
   llvm::AttrBuilder FuncAttrs;
-  ConstructDefaultFnAttrList(F.getName(), F.hasOptNone(),
- /* AttrOnCallSite = */ false, FuncAttrs);
+  getDefaultFunctionAttributes(F.getName(), F.hasOptNone(),
+   /* AttrOnCallSite = */ false, FuncAttrs);
+  // TODO: call GetCPUAndFeaturesAttributes?
   F.addAttributes(llvm::AttributeList::FunctionIndex, FuncAttrs);
 }
 
+void CodeGenModule::addDefaultFunctionDefinitionAttributes(
+   llvm::AttrBu

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[John McCall via cfe-commits]

rjmccall added a comment.

The formation restrictions on ARC writeback conversions probably make this 
more-or-less impossible to test, but I can try to explain when they happen.  
They happen when you have an argument of type "id __strong *" and pass it as a 
parameter of type "id __autoreleasing *".  __strong is the default for 
variables, and __autoreleasing is the default for pointer-to-id parameters, so 
basically you need something like:

  void test(int opaque) {
extern void foo(id*);
id x;
id y;
foo(opaque ? &x : &y);
  }

or any other expression that forms a sufficiently complex argument of type "id 
__strong *" prior to writeback conversion.


https://reviews.llvm.org/D24712



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

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[John McCall via cfe-commits]

rjmccall added a comment.

Actually, that should demonstrate the difference, assuming the LLVM function 
looks through selects, since IRGen should generate that as a select.


https://reviews.llvm.org/D24712



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

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[John McCall via cfe-commits]

rjmccall added a comment.

Oh.  One danger with invoking a generic LLVM routine is that they often expect 
a well-formed function, not something that is plausibly still being emitted.


https://reviews.llvm.org/D24712



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

Re: [PATCH] D24712: Replace 'isProvablyNonNull' with existing utility llvm::IsKnownNonNull.

[John McCall via cfe-commits]

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

In that case, you probably will not be able to test this difference, because 
the argument is basically required to be the address of a local variable, a 
parameter, or null (or a various things that propagate such values).  And our 
IR-generation scheme for parameters will never make them just an llvm::Argument.

LGTM, then.


https://reviews.llvm.org/D24712



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

[PATCH] D24012: Fix strict-aliasing violation in typeinfo::hash_code()

[John McCall via cfe-commits]

rjmccall added a comment.

LGTM, but I'm not a code owner here.


https://reviews.llvm.org/D24012



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

[PATCH] D9403: llvm.noalias - Clang CodeGen for local restrict-qualified pointers

[John McCall via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1291
+// where the pointer to the local variable is the key in the map.
+void CodeGenFunction::EmitAutoVarNoAlias(const AutoVarEmission &emission) {
+  assert(emission.Variable && "emission was not valid!");

This should really be a subroutine of EmitAutoVarAlloca; that will implicitly 
pick this logic up for a bunch of less standard places in IRGen that create 
local variables.  Please make it a static function (if possible) called 
EmitNoAliasScope and call it immediately after EmitVarAnnotations.



Comment at: lib/CodeGen/CGStmt.cpp:525
+void CodeGenFunction::LexicalNoAliasInfo::addNoAliasMD() {
+  if (MemoryInsts.empty() || NoAliasScopes.empty())
+return;

hfinkel wrote:
> rjmccall wrote:
> > It's much more likely that NoAliasScopes will be empty than that 
> > MemoryInsts will be empty.  You should probably fast-path using that, or 
> > better yet, with the RecordMemoryInsts bit.
> I'm not sure that's true; we only record memory-accessing instructions in the 
> first place if there are relevant restrict-qualified pointers around.
Ok, that makes sense.



Comment at: lib/CodeGen/CGStmt.cpp:537
+  llvm::LLVMContext::MD_noalias),
+  NewScopeList));
+

rjmccall wrote:
> hfinkel wrote:
> > rjmccall wrote:
> > > This is a very strange representation.  Every memory operation in the 
> > > lexical block is annotated with a list of all of the scopes that were 
> > > entered within the block, even if they were entered after the operation.  
> > > But for some reason, not with nested scopes?
> > > 
> > > What's the right patch for me to read about this representation?
> > Perhaps unfortunately, this is an artifact of the way that restrict is 
> > defined in C. It applies to all accesses in the block in which the variable 
> > is declared, even those before the declaration of the restrict-qualified 
> > local itself.
> > 
> > It should work with nested scopes, in the sense that we add these things as 
> > we complete each scope. So we add things to the inner scope, and then when 
> > we complete the outer scope, we go back over the instructions (including 
> > those in the inner scope because the scope recording recurses up the scope 
> > hierarchy), and adds the outer scopes - it concatenates them to any added 
> > by the inner (nested) scopes.
> > 
> > The noalias intrinsic's LangRef updates are in D9375.
> Ah, I see the recursion now.  Please add a comment explaining that 
> expectation here.
I would still like this comment.  It should be enough to explain how 
MemoryInsts actually gets filled: there's a callback from inserting an 
instruction which adds all memory instructions to every active lexical scope 
that's recording them.



Comment at: lib/CodeGen/CodeGenFunction.cpp:1986
+  if (I->mayReadOrWriteMemory())
+recordMemoryInstruction(I);
 }

This condition will include calls, which I assume is unnecessary (except maybe 
memory intrinsics?).  More seriously, it will also include loads and stores 
into allocas, which will sweep in all sorts of bookkeeping temporaries that 
IRGen uses in more complex code-generation situations.  You might want to 
consider ways to avoid recording this kind of instruction that are very likely 
to just get immediately optimized away by e.g. mem2reg.

Please also modify LexicalScope so that it records whether there's any active 
recording scope in the CodeGenFunction.  Lexical scopes are nested, so that 
should be as easy as saving the current state when you enter a scope and 
restoring it when you leave.  That will allow you to optimize this code by 
completely bypassing the recursion and even the check for whether the 
instruction is a memory instruction in the extremely common case of a function 
with no restrict variables.

In general, a lot of things about this approach seem to have worryingly poor 
asymptotic performance in the face of large functions or (especially) many 
restrict variables.  (You could, for example, have chosen to have each memory 
instruction link to its enclosing noalias scope, which would contain a link to 
its enclosing scope and a list of the variables it directly declares.  This 
would be a more complex representation to consume, but it would not require 
inherently quadratic frontend work building all these lists.)  The only saving 
grace is that we expect very little code to using restrict, so it becomes vital 
to ensure that your code is immediately short-circuited when nothing is using 
restrict.



Comment at: lib/CodeGen/CodeGenFunction.h:597
+// those blocks) so that we can later add the appropriate metadata. Record
+// this instruction and so the same in any parent scopes.
+void recordMemoryInstruction(llvm::Instruction *I) {
---

[PATCH] D25448: [ubsan] Disable -fsanitize=vptr checks for devirtualized calls

[John McCall via cfe-commits]

rjmccall requested changes to this revision.
rjmccall added inline comments.
This revision now requires changes to proceed.



Comment at: lib/CodeGen/CodeGenFunction.h:379
+  /// Set of object pointers which are blacklisted from the UB sanitizer.
+  llvm::SmallPtrSet SanitizerBasePointerBlacklist;
+

Please find some way to do this that doesn't require adding new tracking state 
like this.  It should be quite easy to pass down that a call was devirtualized.


https://reviews.llvm.org/D25448



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

[PATCH] D25547: [CodeGen][ObjC] Do not emit objc_storeStrong to initialize a constexpr variable

[John McCall via cfe-commits]

rjmccall added a comment.

The correct fix is to honor isInit by folding the logic for EmitScalarInit into 
this function.  That should allow you to eliminate EmitScalarInit completely, 
although it would be fine to leave it as just a call to EmitStoreThroughLValue. 
 I did a quick audit of all the calls to EmitStoreThroughLValue that might pass 
isInit=true, and it does look like none of them are relying on the current 
behavior for ARC ownership; the most suspicious are the calls from 
EmitObjCCollectionLiteral, but the l-value there is non lifetime-qualified, so 
it's fine.


https://reviews.llvm.org/D25547



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

[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

[John McCall via cfe-commits]

rjmccall added a comment.

Richard should probably weigh in about whether we should be recording potential 
captures at *all* capturing scopes.  But at the very least, I think you have a 
bug here where the variable is declared outside of the block but within the 
lambda; in that case, it is definitely not a potential capture of the lambda.


https://reviews.llvm.org/D25556



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

[PATCH] D25448: [ubsan] Disable -fsanitize=vptr checks for devirtualized calls

[John McCall via cfe-commits]

rjmccall added a comment.

Wait, can you talk me through the bug here?  Why is final-based 
devirtualization here different from, say, user-directed devirtualization via a 
qualified method name?

It sounds to me from your description that you're not sure why this is 
happening.  If this indeed only triggers in the presence of multiple 
inheritance, it might just be the case that you're doing your object-extents 
analysis starting from the wrong offset.


https://reviews.llvm.org/D25448



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

[PATCH] D25547: [CodeGen][ObjC] Do not emit objc_storeStrong to initialize a constexpr variable

[John McCall via cfe-commits]

rjmccall added a comment.

Sorry, no, just the one that takes an llvm::Value* instead of an Expr*.


https://reviews.llvm.org/D25547



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

[PATCH] D25547: [CodeGen][ObjC] Do not emit objc_storeStrong to initialize a constexpr variable

[John McCall via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:1650
+}
+
 switch (Lifetime) {

I think you can fold this a bit more. :)  You have exactly the same switch 
statement below, and several of the cases are identical; for the others, you 
can just sink the isInit check into the case.

Note that calling EmitStoreOfScalar and returning has the same behavior as 
"falling into the normal path".  isObjCWeak() / isObjCStrong() are checking for 
the GC qualifiers, which are exclusive with ARC lifetime.


https://reviews.llvm.org/D25547



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

Re: [PATCH] D41039: Add support for attribute "trivial_abi"

[John McCall via cfe-commits]

On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie  wrote:

> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> rjmccall added a comment.
>>
>> In https://reviews.llvm.org/D41039#951648, @ahatanak wrote:
>>
>> > I had a discussion with Duncan today and he pointed out that perhaps we
>> shouldn't allow users to annotate a struct with "trivial_abi" if one of its
>> subobjects is non-trivial and is not annotated with "trivial_abi" since
>> that gives users too much power.
>> >
>> > Should we error out or drop "trivial_abi" from struct Outer when the
>> following code is compiled?
>> >
>> >   struct Inner1 {
>> > ~Inner1(); // non-trivial
>> > int x;
>> >   };
>> >
>> >   struct __attribute__((trivial_abi)) Outer {
>> > ~Outer();
>> > Inner1 x;
>> >   };
>> >
>> >
>> > The current patch doesn't error out or drop the attribute, but the
>> patch would probably be much simpler if we didn't allow it.
>>
>>
>> I think it makes sense to emit an error if there is provably a
>> non-trivial-ABI component.  However, for class temploids I think that
>> diagnostic should only fire on the definition, not on instantiations; for
>> example:
>>
>>   template  struct __attribute__((trivial_abi)) holder {
>>  T value;
>>  ~holder() {}
>>   };
>>   holder hs; // this instantiation should be legal despite
>> the fact that holder cannot be trivial-ABI.
>>
>> But we should still be able to emit the diagnostic in template
>> definitions, e.g.:
>>
>>   template  struct __attribute__((trivial_abi)) named_holder {
>>  std::string name; // there are no instantiations of this template
>> that could ever be trivial-ABI
>>  T value;
>>  ~named_holder() {}
>>   };
>>
>> The wording should be something akin to the standard template rule that a
>> template is ill-formed if it has no valid instantiations, no diagnostic
>> required.
>>
>> I would definitely like to open the conversation about the name of the
>> attribute.  I don't think we've used "abi" in an existing attribute name;
>> usually it's more descriptive.  And "trivial" is a weighty word in the
>> standard.  I'm not sure I have a great counter-proposal off the top of my
>> head, though.
>>
>
> Agreed on both counts (would love a better name, don't have any stand-out
> candidates off the top of my head).
>
> I feel like a more descriptive term about the property of the object would
> make me happier - something like "address_independent_identity"
> (s/identity/value/?) though, yeah, that's not spectacular by any stretch.
>

Incidentally, your comments are not showing up on Phabricator for some
reason.

The term "trivially movable" suggests itself, with two caveats:
  - What we're talking about is trivial *destructive* movability, i.e. that
the combination of moving the value to a new object and not destroying the
old object can be done trivially, which is not quite the same as trivial
movability in the normal C++ sense, which I guess could be a property that
someone theoretically might care about (if the type is trivially
destructed, but it isn't copyable for semantic reasons?).
  - Trivial destructive movability is a really common property, and it's
something that a compiler would really like to optimize based on even in
cases where trivial_abi can't be adopted for binary-compatibility reasons.
Stealing the term for the stronger property that the type is trivially
destructively movable *and its ABI should reflect that in a specific way*
would be unfortunate.

"trivially_movable" is a long attribute anyway, and
"trivially_destructively_movable" is even worse.

Maybe that second point is telling us that this isn't purely descriptive —
it's inherently talking about the ABI, not just the semantics of the type.
I might be talking myself into accepting trivial_abi if we don't end up
with a better suggestion.

Random thing that occurred to me: is it actually reasonable to enforce
trivial_abi correctness in a non-template context?  Templates aren't the
only case where a user could reasonably want to add trivial_abi and just
have it be suppressed if it's wrong.  Imagine if some stdlib made
std::string trivial_abi; someone might reasonably want to make my
named_holder example above trivial_abi as well, with the expectation that
it would only have an effect on targets where std::string was trivial_abi.
At the very least, I'm concerned that we might be opening ourselves up to a
need to add supporting features, like a way to be conditionally trivial_abi
based on context.

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

Re: [PATCH] D41039: Add support for attribute "trivial_abi"

[John McCall via cfe-commits]

On Tue, Dec 12, 2017 at 1:45 PM, David Blaikie  wrote:

> On Mon, Dec 11, 2017 at 5:38 PM John McCall  wrote:
>
>> On Mon, Dec 11, 2017 at 6:19 PM, David Blaikie 
>> wrote:
>>
>>> On Mon, Dec 11, 2017 at 3:16 PM John McCall via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>
 rjmccall added a comment.

 In https://reviews.llvm.org/D41039#951648, @ahatanak wrote:

 > I had a discussion with Duncan today and he pointed out that perhaps
 we shouldn't allow users to annotate a struct with "trivial_abi" if one of
 its subobjects is non-trivial and is not annotated with "trivial_abi" since
 that gives users too much power.
 >
 > Should we error out or drop "trivial_abi" from struct Outer when the
 following code is compiled?
 >
 >   struct Inner1 {
 > ~Inner1(); // non-trivial
 > int x;
 >   };
 >
 >   struct __attribute__((trivial_abi)) Outer {
 > ~Outer();
 > Inner1 x;
 >   };
 >
 >
 > The current patch doesn't error out or drop the attribute, but the
 patch would probably be much simpler if we didn't allow it.


 I think it makes sense to emit an error if there is provably a
 non-trivial-ABI component.  However, for class temploids I think that
 diagnostic should only fire on the definition, not on instantiations; for
 example:

   template  struct __attribute__((trivial_abi)) holder {
  T value;
  ~holder() {}
   };
   holder hs; // this instantiation should be legal despite
 the fact that holder cannot be trivial-ABI.

 But we should still be able to emit the diagnostic in template
 definitions, e.g.:

   template  struct __attribute__((trivial_abi)) named_holder {
  std::string name; // there are no instantiations of this template
 that could ever be trivial-ABI
  T value;
  ~named_holder() {}
   };

 The wording should be something akin to the standard template rule that
 a template is ill-formed if it has no valid instantiations, no diagnostic
 required.

 I would definitely like to open the conversation about the name of the
 attribute.  I don't think we've used "abi" in an existing attribute name;
 usually it's more descriptive.  And "trivial" is a weighty word in the
 standard.  I'm not sure I have a great counter-proposal off the top of my
 head, though.

>>>
>>> Agreed on both counts (would love a better name, don't have any
>>> stand-out candidates off the top of my head).
>>>
>>> I feel like a more descriptive term about the property of the object
>>> would make me happier - something like "address_independent_identity"
>>> (s/identity/value/?) though, yeah, that's not spectacular by any stretch.
>>>
>>
>> Incidentally, your comments are not showing up on Phabricator for some
>> reason.
>>
>
> Yeah, Phab doesn't do a great job looking on the mailing list for
> interesting replies - I forget, maybe it only catches bottom post or top
> post but not infix replies or something...
>

Well, fortunately the mailing list is also archived. :)


> The term "trivially movable" suggests itself, with two caveats:
>>   - What we're talking about is trivial *destructive* movability, i.e.
>> that the combination of moving the value to a new object and not destroying
>> the old object can be done trivially, which is not quite the same as
>> trivial movability in the normal C++ sense, which I guess could be a
>> property that someone theoretically might care about (if the type is
>> trivially destructed, but it isn't copyable for semantic reasons?).
>>   - Trivial destructive movability is a really common property, and it's
>> something that a compiler would really like to optimize based on even in
>> cases where trivial_abi can't be adopted for binary-compatibility reasons.
>> Stealing the term for the stronger property that the type is trivially
>> destructively movable *and its ABI should reflect that in a specific way*
>> would be unfortunate.
>>
>
> Fair point.
>
>
>> "trivially_movable" is a long attribute anyway, and
>> "trivially_destructively_movable" is even worse.
>>
>> Maybe that second point is telling us that this isn't purely descriptive
>> — it's inherently talking about the ABI, not just the semantics of the
>> type.  I might be talking myself into accepting trivial_abi if we don't end
>> up with a better suggestion.
>>
>
> *nod* I think if you want to slice this that way (ensuring there's space
> to support describing a similar property for use only in non-ABI-breaking
> contexts as well) it seems like ABI is the term to use somewhere in the
> name.
>

Yeah.  We just had a little internal conversation about it, and the idea of
"address_invariant_abi" came up, but I'm not sure it has enough to
recommend it over "trivial_abi" to justify the longer name.


> Random thing that occurred to me: is it actually reasonable to enforc

[clang] [ObjC] Fix offsets following `[[no_unique_address]]` for `@encode()` (PR #71321)

[John McCall via cfe-commits]

https://github.com/rjmccall approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/71321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not clear FP pragma stack when instantiating functions (PR #70646)

[John McCall via cfe-commits]

rjmccall wrote:

> pragma pack uses pragma stack and it is allowed in any place: 
> https://godbolt.org/z/f8fP1vn63 . If such pragma presents in the late-parsed 
> template, and push-pop operations in it are not balanced, the late parse of 
> such function can break the pragma stack and next templates can be parsed 
> incorrectly. It is not a FP pragma, but it demonstrates that the compiler 
> must provide some Sema state isolation for late parsed templates.

Okay, so if we accept this, this is a hard constraint on our ability to do late 
parsing of function bodies in situations not blessed by the language.  We could 
decide not to support that — we probably have to support uses of `#pragma pack` 
within a function, but we could e.g. emit an error if the effect of the pragmas 
is non-local (or a warning saying that we're going to ignore it).  It looks 
like MSVC clears the stack before it does late parsing: you get a warning if 
you put your test function inside a class body.

https://github.com/llvm/llvm-project/pull/70646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not clear FP pragma stack when instantiating functions (PR #70646)

[John McCall via cfe-commits]

https://github.com/rjmccall edited 
https://github.com/llvm/llvm-project/pull/70646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not clear FP pragma stack when instantiating functions (PR #70646)

[John McCall via cfe-commits]

@@ -710,9 +710,11 @@ class Sema final {
 return result;
   }
 
+  // Saves the current floating-point pragma stack and clear it in this Sema.
   class FpPragmaStackSaveRAII {
   public:
-FpPragmaStackSaveRAII(Sema &S) : S(S), SavedStack(S.FpPragmaStack) {}
+FpPragmaStackSaveRAII(Sema &S)
+: S(S), SavedStack(std::move(S.FpPragmaStack)) {}

rjmccall wrote:

Does a move of `PragmaStack` guarantee to leave an empty stack?  I guess it's 
ultimately just a `SmallVector`, and it looks like `SmallVector` does currently 
make that guarantee.  That seems worth a comment or an assertion.

https://github.com/llvm/llvm-project/pull/70646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not clear FP pragma stack when instantiating functions (PR #70646)

[John McCall via cfe-commits]

https://github.com/rjmccall commented:

Otherwise LGTM

https://github.com/llvm/llvm-project/pull/70646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Remove fixed point arithmetic error (PR #71884)

[John McCall via cfe-commits]

rjmccall wrote:

I'm happy with the basic idea here of making fixed-point an opt-in feature.  
It'd be nice to preserve the special diagnostic that tells the user that they 
need to use `-ffixed-point`, though.  Do we have any existing parsing paths 
that recognize unrecognized identifiers that are written as apparent type 
specifiers?

https://github.com/llvm/llvm-project/pull/71884
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Ignore template parameter visibility (PR #72092)

[John McCall via cfe-commits]

rjmccall wrote:

Can we just ignore template parameters selectively when we have a visibility 
attribute, the way I suggested in https://reviews.llvm.org/D154774?  To quote:

> A visibility attribute on an explicit specialization or instantiation should 
> definitely override everything else.  A visibility attribute on a template 
> pattern should only override other visibility information that we would 
> derive from that pattern, like the visibility of the template parameters; it 
> should not override visibility from the template arguments.  You probably 
> need this function to return an enum with three cases: (1) factor in both 
> template arguments and template parameters, (2) factor in only template 
> arguments, and (3) factor in nothing.

In the absence of visibility attributes, a template that's templated over e.g. 
an `enum` with hidden visibility seems like it ought to have hidden visibility.

https://github.com/llvm/llvm-project/pull/72092
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [ARC][Documentation] Explicitly state that messaging weak objects keeps a strong reference during call lifetime (PR #72169)

[John McCall via cfe-commits]

https://github.com/rjmccall approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/72169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Ignore template parameter visibility (PR #72092)

[John McCall via cfe-commits]

rjmccall wrote:

> I cannot figure out a test case for `TemplateArgument::Expression`. I wonder 
> whether it applies to `X<&s.s> x5;` (address of static member), which Clang 
> doesn't support.

It's primarily used as a dependent template argument.  I'm not sure off-hand 
that it's *never* canonical, though, given all the ways the language has 
extended template arguments in the last decade.

> After the change, does it seem more feasible to ignore template parameters?

Hmm.  You're right that considering the type of a non-type template argument 
would be enough to give the right visibility to my example.  It would not be 
enough if the template was used as a template template argument, though.

I suspect the code just needs to be restructured a bit to get the behavior I'm 
asking for, but I'll need to think about it.

https://github.com/llvm/llvm-project/pull/72092
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Emit TBAA info for enums in C (PR #73326)

[John McCall via cfe-commits]

rjmccall wrote:

This seems conservatively correct, yeah.  My reading is that we could also use 
the underlying type as a parent type for the TBAA metadata: enums are 
compatible with their underlying type, but two enums with the same underlying 
type are not compatible with each other.  But this rule should also work.

https://github.com/llvm/llvm-project/pull/73326
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Stub out gcc_struct attribute (PR #71148)

[John McCall via cfe-commits]

rjmccall wrote:

> @rjmccall The problem will arise only if GCC implements support for MSVC C++ 
> ABI and decides that there is a better way to implement `gcc_struct`. Since, 
> AFAIC, MSVC-compatibility for GCC is not even planned, it's unlikely anybody 
> there will have strong opinions on this. Yet I posted a question on gcc 
> mailing list (https://gcc.gnu.org/pipermail/gcc/2023-December/242963.html) 
> and, unsurprisingly, got no replies in a week.

My question is actually more about supporting `ms_struct` and `gcc_struct` on 
other architectures than x86 than it's about MSVC-compatible targets.  I agree 
that it's highly unlikely that GCC would ever implement the MSVC C++ ABI, but 
e.g. MinGW on ARM doesn't seem out of the picture.

https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Stub out gcc_struct attribute (PR #71148)

[John McCall via cfe-commits]

rjmccall wrote:

Right, I'd just like to make sure that we're not deepening a divergence here.  
It would be good to get agreement from the GCC devs that they think `ms_struct` 
probably ought to do something on e.g. ARM MinGW targets and that they consider 
this a bug (in a feature that they may not really support, which is fine).  But 
if they think *we're* wrong and that this really should only have effect on 
x86, I would like to know that.

https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [FPEnv] Add strictfp in some C++ constructors lacking a FunctionDecl. (PR #74883)

[John McCall via cfe-commits]

@@ -5587,10 +5593,12 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
   !isa_and_nonnull(TargetDecl))
 EmitKCFIOperandBundle(ConcreteCallee, BundleList);
 
+#if 0 // XXX Why is this here? Duplicate!
   if (const FunctionDecl *FD = dyn_cast_or_null(CurFuncDecl))
 if (FD->hasAttr())
   // All calls within a strictfp function are marked strictfp
   Attrs = Attrs.addFnAttribute(getLLVMContext(), 
llvm::Attribute::StrictFP);
+#endif

rjmccall wrote:

Please remove this instead of `#if 0`ing it.

https://github.com/llvm/llvm-project/pull/74883
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [FPEnv] Add strictfp in some C++ constructors lacking a FunctionDecl. (PR #74883)

[John McCall via cfe-commits]

@@ -6,14 +7,80 @@ float z();
 #pragma float_control(except, on)
 class ON {
   float w = 2 + y() * z();
-  // CHECK-LABEL: define {{.*}} @_ZN2ONC2Ev{{.*}}
-  // CHECK: llvm.experimental.constrained.fmul{{.*}}tonearest{{.*}}strict
 };
 ON on;
 #pragma float_control(except, off)
 class OFF {
   float w = 2 + y() * z();
-  // CHECK-LABEL: define {{.*}} @_ZN3OFFC2Ev{{.*}}
-  // CHECK-NOT: llvm.experimental.constrained.fmul{{.*}}tonearest{{.*}}strict
 };
 OFF off;
+// CHECK-LABEL: define internal void @__cxx_global_var_init(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] section ".text.startup" {

rjmccall wrote:

1. Are all of these checks necessary?  If so, please explain why in the test so 
that maintainers will understand what you're testing for.
2. Do you need to be checking the actual content of the attributes here?

https://github.com/llvm/llvm-project/pull/74883
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [CodeGen][arm64e] Add methods and data members to Address, which are needed to authenticate signed pointers (PR #67454)

[John McCall via cfe-commits]

@@ -232,110 +279,133 @@ class CGBuilderTy : public CGBuilderBaseTy {
   /// where i64 is actually the target word size.
   Address CreateConstGEP(Address Addr, uint64_t Index,
  const llvm::Twine &Name = "") {
+llvm::Type *ElTy = Addr.getElementType();
 const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
-CharUnits EltSize =
-CharUnits::fromQuantity(DL.getTypeAllocSize(Addr.getElementType()));
+CharUnits EltSize = CharUnits::fromQuantity(DL.getTypeAllocSize(ElTy));
 
-return Address(CreateGEP(Addr.getElementType(), Addr.getPointer(),
- getSize(Index), Name),
+return Address(CreateGEP(ElTy, Addr.getBasePointer(), getSize(Index), 
Name),
Addr.getElementType(),
-   Addr.getAlignment().alignmentAtOffset(Index * EltSize),
-   NotKnownNonNull);
+   Addr.getAlignment().alignmentAtOffset(Index * EltSize));
   }
 
   /// Create GEP with single dynamic index. The address alignment is reduced
   /// according to the element size.
   using CGBuilderBaseTy::CreateGEP;
-  Address CreateGEP(Address Addr, llvm::Value *Index,
+  Address CreateGEP(CodeGenFunction &CGF, Address Addr, llvm::Value *Index,
 const llvm::Twine &Name = "") {
 const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
 CharUnits EltSize =
 CharUnits::fromQuantity(DL.getTypeAllocSize(Addr.getElementType()));
 
 return Address(
-CreateGEP(Addr.getElementType(), Addr.getPointer(), Index, Name),
+CreateGEP(Addr.getElementType(), Addr.getRawPointer(CGF), Index, Name),
 Addr.getElementType(),
-Addr.getAlignment().alignmentOfArrayElement(EltSize), NotKnownNonNull);
+Addr.getAlignment().alignmentOfArrayElement(EltSize));
   }
 
   /// Given a pointer to i8, adjust it by a given constant offset.
   Address CreateConstInBoundsByteGEP(Address Addr, CharUnits Offset,
  const llvm::Twine &Name = "") {
 assert(Addr.getElementType() == TypeCache.Int8Ty);
-return Address(CreateInBoundsGEP(Addr.getElementType(), Addr.getPointer(),
- getSize(Offset), Name),
-   Addr.getElementType(),
-   Addr.getAlignment().alignmentAtOffset(Offset),
-   Addr.isKnownNonNull());
+return Address(
+CreateInBoundsGEP(Addr.getElementType(), Addr.getBasePointer(),
+  getSize(Offset), Name),
+Addr.getElementType(), Addr.getAlignment().alignmentAtOffset(Offset),
+Addr.isKnownNonNull());
   }
+
   Address CreateConstByteGEP(Address Addr, CharUnits Offset,
  const llvm::Twine &Name = "") {
 assert(Addr.getElementType() == TypeCache.Int8Ty);
-return Address(CreateGEP(Addr.getElementType(), Addr.getPointer(),
+return Address(CreateGEP(Addr.getElementType(), Addr.getBasePointer(),
  getSize(Offset), Name),
Addr.getElementType(),
-   Addr.getAlignment().alignmentAtOffset(Offset),
-   NotKnownNonNull);
+   Addr.getAlignment().alignmentAtOffset(Offset));
   }
 
   using CGBuilderBaseTy::CreateConstInBoundsGEP2_32;
   Address CreateConstInBoundsGEP2_32(Address Addr, unsigned Idx0, unsigned 
Idx1,
  const llvm::Twine &Name = "") {
-const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+return createConstGEP2_32(Addr, Idx0, Idx1, Name);
+  }
 
-auto *GEP = cast(CreateConstInBoundsGEP2_32(
-Addr.getElementType(), Addr.getPointer(), Idx0, Idx1, Name));
-llvm::APInt Offset(
-DL.getIndexSizeInBits(Addr.getType()->getPointerAddressSpace()), 0,
-/*isSigned=*/true);
-if (!GEP->accumulateConstantOffset(DL, Offset))
-  llvm_unreachable("offset of GEP with constants is always computable");
-return Address(GEP, GEP->getResultElementType(),
-   Addr.getAlignment().alignmentAtOffset(
-   CharUnits::fromQuantity(Offset.getSExtValue())),
-   Addr.isKnownNonNull());
+  using CGBuilderBaseTy::CreateConstGEP2_32;
+  Address CreateConstGEP2_32(Address Addr, unsigned Idx0, unsigned Idx1,
+ const llvm::Twine &Name = "") {
+return createConstGEP2_32(Addr, Idx0, Idx1, Name);
+  }
+
+  Address CreateGEP(Address Addr, ArrayRef IdxList,
+llvm::Type *ElementType, CharUnits Align,
+const Twine &Name = "") {
+llvm::Value *Ptr = getRawPointerFromAddress(Addr);
+return RawAddress(CreateGEP(Addr.getElementType(), Ptr, IdxList, Name),
+  ElementType, Align);
+  }
+
+  using CGBuilderBaseTy::CreateInBoundsGEP;
+  Address CreateInBoundsGEP(Address Addr, ArrayRef IdxList,
+   

[clang] [clang] Stub out gcc_struct attribute (PR #71148)

[John McCall via cfe-commits]

@@ -7255,20 +7255,27 @@ void Sema::CheckCompletedCXXClass(Scope *S, 
CXXRecordDecl *Record) {
   CheckCompletedMemberFunction(MD);
   }
 
-  // ms_struct is a request to use the same ABI rules as MSVC.  Check
-  // whether this class uses any C++ features that are implemented
-  // completely differently in MSVC, and if so, emit a diagnostic.
-  // That diagnostic defaults to an error, but we allow projects to
-  // map it down to a warning (or ignore it).  It's a fairly common
-  // practice among users of the ms_struct pragma to mass-annotate
-  // headers, sweeping up a bunch of types that the project doesn't
-  // really rely on MSVC-compatible layout for.  We must therefore
-  // support "ms_struct except for C++ stuff" as a secondary ABI.
+  // {ms,gcc}_struct is a request to change ABI rules to either follow
+  // Microsoft or Itanium C++ ABI. However, even if these attributes are
+  // present, we do not layout classes following foreign ABI rules, but
+  // instead enter a special "compatibility mode", which only changes
+  // alignements of fundamental types and layout of bit fields.

rjmccall wrote:

```suggestion
  // alignments of fundamental types and layout of bit fields.
```

https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Stub out gcc_struct attribute (PR #71148)

[John McCall via cfe-commits]

@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
 /// This which can be turned on with an attribute, pragma, or the
 /// -mms-bitfields command-line option.
 bool RecordDecl::isMsStruct(const ASTContext &C) const {
-  return hasAttr() || C.getLangOpts().MSBitfields == 1;
+  if (hasAttr())
+return true;
+  if (hasAttr())
+return false;
+  return C.getLangOpts().MSBitfields.value_or(

rjmccall wrote:

Is there a reason we don't just set the lang opt correctly for the target?  I'm 
pretty sure language option defaults depending on target settings is 
well-established.

https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Ignore template parameter visibility (PR #72092)

[John McCall via cfe-commits]

rjmccall wrote:

The basic design idea behind Clang's visibility model is to treat hidden 
visibility as essentially a second grade of external linkage, external to the 
translation unit but internal to the linkage unit (i.e. image).  So what you 
pointed out about using an anonymous namespace is exactly the intent: just like 
using an internal-linkage entity in a declaration should force it to have 
internal linkage, using a hidden-visibility entity in a declaration should 
force it to have hidden visibility.

We then have to deal with the legacy that people often don't set template 
visibility right and try to patch it up with explicit visibility attributes on 
explicit instantiations and specializations.  We want to treat that as a "big 
hammer" that overrides all of our normal considerations, at least as far as 
that specific declaration goes.  This is not a problem with linkage because 
there's no way to override linkage this way.

https://github.com/llvm/llvm-project/pull/72092
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Stub out gcc_struct attribute (PR #71148)

[John McCall via cfe-commits]

@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
 /// This which can be turned on with an attribute, pragma, or the
 /// -mms-bitfields command-line option.
 bool RecordDecl::isMsStruct(const ASTContext &C) const {
-  return hasAttr() || C.getLangOpts().MSBitfields == 1;
+  if (hasAttr())
+return true;
+  if (hasAttr())
+return false;
+  return C.getLangOpts().MSBitfields.value_or(

rjmccall wrote:

You have a target triple, and you have the base CXXABI. What information are 
you looking to get from the `TargetInfo`?

https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Remove fixed point arithmetic error (PR #71884)

[John McCall via cfe-commits]

rjmccall wrote:

I definitely don't think we should be handling this in the lexer by trying to 
retroactively make this a keyword. I was just thinking that we might have some 
sort of parser-level recovery for e.g. `unrecognized_identifier_t x = 5;` that 
might guess that `unrecognized_identifier_t` is a type name, and it would be 
reasonable for that to special-case type names that aren't keywords in the 
current language settings.  But if we don't have that recovery already, we 
don't have it, and I'm not going to ask you to add it just for this patch.

https://github.com/llvm/llvm-project/pull/71884
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Stub out gcc_struct attribute (PR #71148)

[John McCall via cfe-commits]

@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
 /// This which can be turned on with an attribute, pragma, or the
 /// -mms-bitfields command-line option.
 bool RecordDecl::isMsStruct(const ASTContext &C) const {
-  return hasAttr() || C.getLangOpts().MSBitfields == 1;
+  if (hasAttr())
+return true;
+  if (hasAttr())
+return false;
+  return C.getLangOpts().MSBitfields.value_or(

rjmccall wrote:

I always prefer to have less complicated logic in the options types, because 
I've seen it get out of hand.  I don't feel too strongly about this, though, 
and I do see how you have uses of the components feeding into it.

https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Stub out gcc_struct attribute (PR #71148)

[John McCall via cfe-commits]

rjmccall wrote:

I agree with the Sema/AST-level LGTM (but also don't feel comfortable approving 
the driver changes)

https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

[John McCall via cfe-commits]

rjmccall wrote:

First off, the change here actually applies to all over-aligned empty structs, 
not just to those with aligned bit-fields.  Maybe we can say that aligned 
zero-width bit-fields are ill-formed, but I don't think we can say that all 
aligned empty classes are ill-formed, among other things because I'm pretty 
sure they're explicitly allowed in C++.  So let's just set the aligned 
bit-field question aside for a moment.

Second, I don't understand why the fix is to `isEmptyRecord`.  This is a very 
general routine that affects a lot of code in a lot of targets.  And for most 
of those purposes, I think an aligned empty class should still be considered an 
empty record.  I think we need to understand why this is fixing the bug.

Third, it does look like we have a real ABI bug here.  I haven't looked at 
other targets, but at least on AArch64, a 16-byte-aligned empty class should 
definitely be taking up two registers.  For some reason, we are generating IR 
to pass this class as a single byte rather than as a number of bytes equal to 
its size.

Finally, as usual, some platforms may want to opt out of this ABI break; I'll 
ask around at Apple.

https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

[John McCall via cfe-commits]

rjmccall wrote:

Oh, the [Apple AArch64 calling 
convention](https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Pass-arguments-to-functions-correctly)
 diverges from AAPCS and ignores empty classes as parameters.  We appear to 
consider this an empty class regardless of alignment, which as discussed I 
believe is correct.  So Apple does not want this change on either level: we do 
not want our ABI to change, and we should not be considering this an empty 
class.

https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

[John McCall via cfe-commits]

https://github.com/rjmccall requested changes to this pull request.

Marking as changes requested so that this doesn't get committed.

https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

[John McCall via cfe-commits]

rjmccall wrote:

> This is the code I debug located. Seem the comment is out of date?

That seems like the right place to fix the issue, specifically the place below 
where it always passes a single `i8`.  The right rule is to just fall through 
to the normal conventions for passing the argument if we don't decide to ignore 
it.

The comment isn't wrong, but it could be clearer.  Let me suggest this:

> AAPCS64 does not say that empty records are ignored as arguments,
> but other compilers do so in certain situations, and we copy that behavior.
> Those situations are in fact language-mode-specific, which seems really
> unfortunate, but it's something we just have to accept.  If this doesn't
> apply, just fall through to the standard argument-handling path.
> Darwin overrides the psABI here to ignore all empty records in all modes.

> This issue 
> [1711cc9](https://github.com/llvm/llvm-project/commit/1711cc930bda8d27e87a2092bd220c18e4600c98)
>  point that when pass the empty class the `va_list` will get out of sync.

That seems like something we should fix, yeah.  Generally `va_list` handling 
needs to do the same kind of classification that argument-passing does.

https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

[John McCall via cfe-commits]

https://github.com/rjmccall closed 
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

[John McCall via cfe-commits]

rjmccall wrote:

> The code you noted is supposed to handle two cases, neither of which are 
> relevant to your testcase:
> 
> * Darwin-specific calling convention rules.
> * GNU extensions for zero-size structs (which aren't allowed according to 
> either C or C++ standards, but GNU invented a bunch of non-standard rules for 
> them)

To be clear, this isn't just about zero-sized structs

https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

[John McCall via cfe-commits]

https://github.com/rjmccall reopened 
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Ensure fixed point conversions work in C++ (PR #68344)

[John McCall via cfe-commits]

https://github.com/rjmccall approved this pull request.

LGTM, thanks!

https://github.com/llvm/llvm-project/pull/68344
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AVR] make the AVR ABI Swift compatible (PR #72298)

[John McCall via cfe-commits]

rjmccall wrote:

Right, that would be the way to test it.

I don't know much about AVR, but you should also look at some of the parameters 
to the lowering (e.g. how many max values it's okay to break an aggregate into) 
and make sure you're happy with them.

https://github.com/llvm/llvm-project/pull/72298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AVR] make the AVR ABI Swift compatible (PR #72298)

[John McCall via cfe-commits]

rjmccall wrote:

> @efriedma-quic Cool. So it sounds like it's worth parking this for now, until 
> Kuba's work #71986 is merged?
> 
> @rjmccall I'm not 100% sure I understand? The existing code in AVR.cpp 
> handles the standard AVR ABI, which has a few simple rules based on GCC 
> behaviour. Here we are effectively just passing on the handling for that to 
> `AVRABIInfo`. The error in register thing, I just copied what ARM is doing. I 
> figure that's OK for now. If we need to do some bugfixes later to get AVR 
> working with errors then we'll take a look then probably. Is that reasonable?

`SwiftABIInfo` exists so that targets can tweak more details than just whether 
to use `swifterror`.  For example, you can change the conditions under which 
we'll try to return things in registers, and if you don't do that, IIRC Swift 
will try to return quite a few things in registers by default.  I'm just saying 
that you should make sure that that matches with your backend — either your 
backend needs to make sure that it returns a type like `{ ptr, ptr, float, 
float }` in registers for `swiftcc` or you should tweak the frontend so that it 
knows to return it directly.

https://github.com/llvm/llvm-project/pull/72298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits