[Extension --> Completeness] Relax rules for writeonce candidacy...

[lvoufo]

Summary

After D14418, Remove restriction for writeonce semantics to objects with no non-trivial methods...

[lvoufo]

Fix patch (wrong one submitted initially).

include/clang/AST/DeclCXX.h

struct DefinitionData {
     bool HasPrivateFields : 1;
 
     /// \brief True when there are protected non-static data members.
     bool HasProtectedFields : 1;
 
     /// \brief True when there are private non-static data members.
     bool HasPublicFields : 1;
 
+    /// \brief True is this class has already been checked as candidate
+    /// for writeonce semantics.
+    bool IsWriteOnceChecked : 1;
+
+    /// \brief True if instances of this class are candidates
+    /// for writeonce semantics.
+    bool IsWriteOnceCandidate : 1;
+
     /// \brief True if this class (or any subobject) has mutable fields.
     bool HasMutableFields : 1;
 
     /// \brief True if this class (or any nested anonymous struct or union)
     /// has variant members.
     bool HasVariantMembers : 1;
 
     /// \brief True if there no non-field members declared by the user.
public:
   method_iterator method_begin() const {
     return method_iterator(decls_begin());
   }
   /// \brief Method past-the-end iterator.
   method_iterator method_end() const {
     return method_iterator(decls_end());
   }
 
+  bool computeWriteOnceCandidacy() const;
+
   /// Iterator access to constructor members.
   typedef specific_decl_iterator<CXXConstructorDecl> ctor_iterator;
   typedef llvm::iterator_range<specific_decl_iterator<CXXConstructorDecl>>
     ctor_range;
 
   ctor_range ctors() const { return ctor_range(ctor_begin(), ctor_end()); }
 
   ctor_iterator ctor_begin() const {

lib/AST/DeclCXX.cpp

: UserDeclaredConstructor(false),
       Empty(true),
       Polymorphic(false),
       Abstract(false),
       IsStandardLayout(true),
       HasNoNonEmptyBases(true),
       HasPrivateFields(false),
       HasProtectedFields(false),
       HasPublicFields(false),
+      IsWriteOnceChecked(false),
+      IsWriteOnceCandidate(false),
       HasMutableFields(false),
       HasVariantMembers(false),
       HasOnlyCMembers(true),
       HasInClassInitializer(false),
       HasUninitializedReferenceMember(false),
       NeedOverloadResolutionForMoveConstructor(false),
       NeedOverloadResolutionForMoveAssignment(false),
       NeedOverloadResolutionForDestructor(false),
if (const CXXRecordDecl *RD =
             Field->getType()->getBaseElementTypeUnsafe()->getAsCXXRecordDecl())
       if (RD->isAnyDestructorNoReturn())
         return true;
 
   // All destructors are not noreturn.
   return false;
 }
 
+/// \brief A class is a candidate for 'writeonce' semantics if
+/// none of its methods writes to memory.
+bool CXXRecordDecl::computeWriteOnceCandidacy() const {
+  // The class must be defined and concrete.
+  if (!isCompleteDefinition() || isDependentType()) return false;
+
+  // If this has already been checked, then return the stored value.
+  if (data().IsWriteOnceChecked) return data().IsWriteOnceCandidate;
+
+  // Otherwise, mark that this is checked.
+  data().IsWriteOnceChecked = true;
+  assert(!data().IsWriteOnceCandidate &&
+         "The candidacy should not have been set yet.");
+
+  // Check each method, excluding constructors and destructors.
+  // NOTE: The invariant intrinsic calls are generated right after the
+  //       construction of a given const object (which must be initialized)
+  //       and right before its destruction at the end of its lifetime.
+  //       Both constructors and destructors modify the allocated memory and
+  //       and will not be called in the middle of an invariant_start/end
+  //       pair, thus may not affect the reduction of loads. So, it does not
+  //       help to handle constructors here as well.
+  for (auto *M : methods()) {
+    if (isa<CXXConstructorDecl>(M) || isa<CXXDestructorDecl>(M)) continue;
+
+    // If we already know that this method does not write to memory, skip it.
+    // NOTE: CodeGenModule::ConstructAttributeList() marks declarations with
+    //       ConstAttr and PureAttr attributes, respectively, as 'readnone' and
+    //       'readonly' (for LLVM).
+    if (M->hasAttr<ConstAttr>() || M->hasAttr<PureAttr>()) continue;
+
+    const FunctionDecl *Def = nullptr;
+    bool IsDefined = M->isDefined(Def);
+
+    // If this is not defined and not virtual, then it may write to memory.
+    // So it's not a candidate for 'writeonce' semantics.
+    // If it is purely virtual then it must be overriden and its overriding
+    // method (in this context) must be a candidate for 'writeonce' semantics.
+    // Skip it.
+    if (!IsDefined) {
+      if (M->isPure()) {
+        assert(M->isVirtual() && "Not defined and pure implies virtual.");
+        continue;
+      }
+      return false;
+    }
+
+    // If this is trivial, skip it.
+    if (Def->hasTrivialBody()) continue;
+
+    // TODO: Any other case?
+  }
+
+  // Check bases
+  for (const auto &I : bases()) {
+    const RecordType *Ty = I.getType()->getAs<RecordType>();
+    assert(Ty && "No type?");
+    CXXRecordDecl *Base =
+        cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition());
+
+    if (!Base || !Base->computeWriteOnceCandidacy()) return false;
+  }
+
+  data().IsWriteOnceCandidate = true;
+  return true;
+}
+
 void CXXRecordDecl::completeDefinition() {
   completeDefinition(nullptr);
 }
 
 void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
   RecordDecl::completeDefinition();
   
   // If the class may be abstract (but hasn't been marked as such), check for

lib/AST/Type.cpp

case ObjCInterface: {
     if (Def)
       *Def = Interface;
     return !Interface->hasDefinition();
   }
   }
 }
 
 bool QualType::isWriteOnce(ASTContext &Context) {
-  return (Context.getLangOpts().CPlusPlus && isConstant(Context) &&
-          !getTypePtr()->isReferenceType());
+  // TODO: Include C objects as well?
+  if (Context.getLangOpts().CPlusPlus && isConstant(Context) &&
+      !getTypePtr()->isReferenceType()) {
+    if (const CXXRecordDecl *Record =
+            Context.getBaseElementType(*this)->getAsCXXRecordDecl())
+      return Record->computeWriteOnceCandidacy();
+    return true;
+  }
+  return false;
 }
 
 bool QualType::isPODType(ASTContext &Context) const {
   // C++11 has a more relaxed definition of POD.
   if (Context.getLangOpts().CPlusPlus11)
     return isCXX11PODType(Context);
 
   return isCXX98PODType(Context);

test/CodeGenCXX/const-invariant.cpp

 // CHECK-NL-CO-OBJ: call void @_ZN1AC{{[0-9]+}}Ei({{.*}}* @_ZL1i, {{.*}})
 // CHECK-NL-CO-INT: store {{.*}}, {{.*}}* @_ZL1i
 // CHECK-NL-CO: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL1i to i8*))
 // CHECK-NL-CO-OBJ: call void @_ZN1DC{{[0-9]+}}Ei({{.*}}* @_ZL3i_d, {{.*}})
 // CHECK-NL-CO-OBJ: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL3i_d to i8*))
 // CHECK-NL-CO-OBJ: call void @_ZN1MC{{[0-9]+}}Ei({{.*}}* @_ZL3i_m, {{.*}})
 // CHECK-NL-CO-OBJ: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL3i_m to i8*),
 // CHECK-NL-CO-OBJ: call void @_ZN1FC{{[0-9]+}}Ei({{.*}}* @_ZL3i_f, {{.*}})
-// CHECK-NL-CO-OBJ: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL3i_f to i8*))
+// CHECK-NL-CO-OBJ-NOT: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL3i_f to i8*))
 // CHECK-NL-CO-OBJ: call void @_ZN1IC{{[0-9]+}}Ei({{.*}}* @_ZL3i_i, {{.*}})
 // CHECK-NL-CO-OBJ: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL3i_i to i8*))
 // CHECK-NL-CO-OBJ: call void @_ZN2IVC{{[0-9]+}}Ei({{.*}}* @_ZL4i_iv, {{.*}})
-// CHECK-NL-CO-OBJ: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL4i_iv to i8*))
+// CHECK-NL-CO-OBJ-NOT: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL4i_iv to i8*))
 // CHECK-NL-CO-OBJ: call void @_ZN3CIVC{{[0-9]+}}Ei({{.*}}* @_ZL5i_civ, {{.*}})
 // CHECK-NL-CO-OBJ: call {{.*}}@llvm.invariant.start(i64 {{[0-9]+}}, i8* bitcast ({{.*}} @_ZL5i_civ to i8*))
 
 void foo(const Type* const);
 void bar(Type);
 void foo(const int* const);
 void bar(int);
 #if defined(OBJ)

 void ex1_f() {
 // CHECK: @_Z5ex1_fv(
 #ifdef LOCAL
   Const F i_f(one());
 #endif
   // CHECK-L-CO-OBJ: %i_f = alloca {{.*}}
 
   // CHECK-L-CO-OBJ: call void @_ZN1FC{{[0-9]+}}Ei({{.*}}* %i_f,
-  // CHECK-L-CO-OBJ: call {{.*}}@llvm.invariant.start(
+  // CHECK-L-CO-OBJ-NOT: call {{.*}}@llvm.invariant.start(
   bar_f(i_f);
   foo_f(&i_f);  // May change i_f.
   bar_f(i_f);
-  // CHECK-L-CO-OBJ: call {{.*}}@llvm.invariant.end(
+  // CHECK-L-CO-OBJ-NOT: call {{.*}}@llvm.invariant.end(
 }
 
 // Example 1 with type inheriting non-virtual base:
 //   Expect no change from Example 1.
 void ex1_i() {
 // CHECK: @_Z5ex1_iv(
 #ifdef LOCAL
   Const I i_i(one());

 void ex1_iv() {
 // CHECK: @_Z6ex1_ivv(
 #ifdef LOCAL
   Const IV i_iv(one());
 #endif
   // CHECK-L-CO-OBJ: %i_iv = alloca {{.*}}
 
   // CHECK-L-CO-OBJ: call void @_ZN2IVC{{[0-9]+}}Ei({{.*}}* %i_iv,
-  // CHECK-L-CO-OBJ: call {{.*}}@llvm.invariant.start(
+  // CHECK-L-CO-OBJ-NOT: call {{.*}}@llvm.invariant.start(
   bar_iv(i_iv);
   foo_iv(&i_iv);  // Does not change i_iv.
   bar_iv(i_iv);
-  // CHECK-L-CO-OBJ: call {{.*}}@llvm.invariant.end(
+  // CHECK-L-CO-OBJ-NOT: call {{.*}}@llvm.invariant.end(
 }
 
 // Example 1 with type inheriting virtual writeonce base:
 //   Expect no change from Example 1.
 void ex1_civ() {
 // CHECK: @_Z7ex1_civv(
 #ifdef LOCAL
   Const CIV i_civ(one());