[CodeGen] Refine generation of TBAA info for bit-field lvalues

[kosarev]

Summary

The main change is that now we generate TBAA info before constructing the resulting lvalue instead of constructing lvalue with some default TBAA info and fixing it as necessary afterwards. We also keep the TBAA info close to lvalue base info, which is supposed to simplify their future merging.

This patch should not bring in any functional changes.

This is part of D38126 reworked to be a separate patch to simplify review.

[rjmccall]

lib/CodeGen/CGExpr.cpp

3665 ↗

(On Diff #119135)

The new name here doesn't make sense; the base is not an expression, it's an l-value. That and the change in capitalization make it hard to track down the changes you've made to each branch here. I believe all you're doing for the other path is sinking some code past the isBitField() block.

[kosarev]

• Removed renamings that complicate reviewing.

[kosarev]

Yes, some preparation code moved past the isBitField() block and generation of TBAA info moved before creating the bit-field lvalue.

Thanks for catching the BaseExpr renaming.

[rjmccall]

Okay, thanks.

cfe/trunk/lib/CodeGen/CGExpr.cpp

if (hasAnyVptr(Field->getType(), Context))
       return true;
 
   return false;
 }
 
 LValue CodeGenFunction::EmitLValueForField(LValue base,
                                            const FieldDecl *field) {
   LValueBaseInfo BaseInfo = base.getBaseInfo();
-  AlignmentSource fieldAlignSource =
-    getFieldAlignmentSource(BaseInfo.getAlignmentSource());
-  LValueBaseInfo FieldBaseInfo(fieldAlignSource, BaseInfo.getMayAlias());
-
-  QualType type = field->getType();
-  const RecordDecl *rec = field->getParent();
-  if (rec->isUnion() || rec->hasAttr<MayAliasAttr>() || type->isVectorType())
-    FieldBaseInfo.setMayAlias(true);
-  bool mayAlias = FieldBaseInfo.getMayAlias();
 
   if (field->isBitField()) {
     const CGRecordLayout &RL =
       CGM.getTypes().getCGRecordLayout(field->getParent());
     const CGBitFieldInfo &Info = RL.getBitFieldInfo(field);
     Address Addr = base.getAddress();
     unsigned Idx = RL.getLLVMFieldNo(field);
     if (Idx != 0)
       // For structs, we GEP to the field that the record layout suggests.
       Addr = Builder.CreateStructGEP(Addr, Idx, Info.StorageOffset,
                                      field->getName());
     // Get the access type.
     llvm::Type *FieldIntTy =
       llvm::Type::getIntNTy(getLLVMContext(), Info.StorageSize);
     if (Addr.getElementType() != FieldIntTy)
       Addr = Builder.CreateElementBitCast(Addr, FieldIntTy);
 
     QualType fieldType =
       field->getType().withCVRQualifiers(base.getVRQualifiers());
+    // TODO: Support TBAA for bit fields.
+    LValueBaseInfo FieldBaseInfo(BaseInfo.getAlignmentSource(), false);
     return LValue::MakeBitfield(Addr, Info, fieldType, FieldBaseInfo,
                                 TBAAAccessInfo());
   }
 
+  // Fields of may-alias structures are may-alias themselves.
+  // FIXME: this should get propagated down through anonymous structs
+  // and unions.
+  QualType FieldType = field->getType();
+  const RecordDecl *rec = field->getParent();
+  AlignmentSource BaseAlignSource = BaseInfo.getAlignmentSource();
+  LValueBaseInfo FieldBaseInfo(getFieldAlignmentSource(BaseAlignSource), false);
+  TBAAAccessInfo FieldTBAAInfo;
+  if (BaseInfo.getMayAlias() || rec->hasAttr<MayAliasAttr>() ||
+          FieldType->isVectorType()) {
+    FieldBaseInfo.setMayAlias(true);
+    FieldTBAAInfo = CGM.getTBAAMayAliasAccessInfo();
+  } else if (rec->isUnion()) {
+    // TODO: Support TBAA for unions.
+    FieldBaseInfo.setMayAlias(true);
+    FieldTBAAInfo = CGM.getTBAAMayAliasAccessInfo();
+  } else {
+    // If no base type been assigned for the base access, then try to generate
+    // one for this base lvalue.
+    FieldTBAAInfo = base.getTBAAInfo();
+    if (!FieldTBAAInfo.BaseType) {
+        FieldTBAAInfo.BaseType = CGM.getTBAABaseTypeInfo(base.getType());
+        assert(!FieldTBAAInfo.Offset &&
+               "Nonzero offset for an access with no base type!");
+    }
+
+    // Adjust offset to be relative to the base type.
+    const ASTRecordLayout &Layout =
+        getContext().getASTRecordLayout(field->getParent());
+    unsigned CharWidth = getContext().getCharWidth();
+    if (FieldTBAAInfo.BaseType)
+      FieldTBAAInfo.Offset +=
+          Layout.getFieldOffset(field->getFieldIndex()) / CharWidth;
+
+    // Update the final access type.
+    FieldTBAAInfo.AccessType = CGM.getTBAATypeInfo(FieldType);
+  }
+
   Address addr = base.getAddress();
   unsigned cvr = base.getVRQualifiers();
-  bool TBAAPath = CGM.getCodeGenOpts().StructPathTBAA;
   if (rec->isUnion()) {
     // For unions, there is no pointer adjustment.
-    assert(!type->isReferenceType() && "union has reference member");
-    // TODO: handle path-aware TBAA for union.
-    TBAAPath = false;
-
-    const auto FieldType = field->getType();
+    assert(!FieldType->isReferenceType() && "union has reference member");
     if (CGM.getCodeGenOpts().StrictVTablePointers &&
         hasAnyVptr(FieldType, getContext()))
       // Because unions can easily skip invariant.barriers, we need to add
       // a barrier every time CXXRecord field with vptr is referenced.
       addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),
                      addr.getAlignment());
   } else {
     // For structs, we GEP to the field that the record layout suggests.
     addr = emitAddrOfFieldStorage(*this, addr, field);
 
     // If this is a reference field, load the reference right now.
-    if (const ReferenceType *refType = type->getAs<ReferenceType>()) {
+    if (const ReferenceType *refType = FieldType->getAs<ReferenceType>()) {
       llvm::LoadInst *load = Builder.CreateLoad(addr, "ref");
       if (cvr & Qualifiers::Volatile) load->setVolatile(true);
 
-      // Loading the reference will disable path-aware TBAA.
-      TBAAPath = false;
-      TBAAAccessInfo TBAAInfo = mayAlias ? CGM.getTBAAMayAliasAccessInfo() :
-                                           CGM.getTBAAAccessInfo(type);
-      CGM.DecorateInstructionWithTBAA(load, TBAAInfo);
-
-      mayAlias = false;
-      type = refType->getPointeeType();
+      CGM.DecorateInstructionWithTBAA(load, FieldTBAAInfo);
 
-      CharUnits alignment =
-        getNaturalTypeAlignment(type, &FieldBaseInfo, /* TBAAInfo= */ nullptr,
+      FieldType = refType->getPointeeType();
+      CharUnits Align = getNaturalTypeAlignment(FieldType, &FieldBaseInfo,
+                                                &FieldTBAAInfo,
                                                 /* forPointeeType= */ true);
-      FieldBaseInfo.setMayAlias(false);
-      addr = Address(load, alignment);
+      addr = Address(load, Align);
 
       // Qualifiers on the struct don't apply to the referencee, and
       // we'll pick up CVR from the actual type later, so reset these
       // additional qualifiers now.
       cvr = 0;
     }
   }
 
   // Make sure that the address is pointing to the right type.  This is critical
   // for both unions and structs.  A union needs a bitcast, a struct element
   // will need a bitcast if the LLVM type laid out doesn't match the desired
   // type.
-  addr = Builder.CreateElementBitCast(addr,
-                                      CGM.getTypes().ConvertTypeForMem(type),
-                                      field->getName());
+  addr = Builder.CreateElementBitCast(
+      addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
 
   if (field->hasAttr<AnnotateAttr>())
     addr = EmitFieldAnnotations(field, addr);
 
-  LValue LV = MakeAddrLValue(addr, type, FieldBaseInfo,
-                             CGM.getTBAAAccessInfo(type));
+  LValue LV = MakeAddrLValue(addr, FieldType, FieldBaseInfo, FieldTBAAInfo);
   LV.getQuals().addCVRQualifiers(cvr);
 
-  // Fields of may_alias structs act like 'char' for TBAA purposes.
-  // FIXME: this should get propagated down through anonymous structs
-  // and unions.
-  if (mayAlias) {
-    LV.setTBAAInfo(CGM.getTBAAMayAliasAccessInfo());
-  } else if (TBAAPath) {
-    // If no base type been assigned for the base access, then try to generate
-    // one for this base lvalue.
-    TBAAAccessInfo TBAAInfo = base.getTBAAInfo();
-    if (!TBAAInfo.BaseType) {
-        TBAAInfo.BaseType = CGM.getTBAABaseTypeInfo(base.getType());
-        assert(!TBAAInfo.Offset &&
-               "Nonzero offset for an access with no base type!");
-    }
-
-    // Adjust offset to be relative to the base type.
-    const ASTRecordLayout &Layout =
-        getContext().getASTRecordLayout(field->getParent());
-    unsigned CharWidth = getContext().getCharWidth();
-    if (TBAAInfo.BaseType)
-      TBAAInfo.Offset +=
-          Layout.getFieldOffset(field->getFieldIndex()) / CharWidth;
-
-    // Update the final access type.
-    TBAAInfo.AccessType = LV.getTBAAInfo().AccessType;
-
-    LV.setTBAAInfo(TBAAInfo);
-  }
-
   // __weak attribute on a field is ignored.
   if (LV.getQuals().getObjCGCAttr() == Qualifiers::Weak)
     LV.getQuals().removeObjCGCAttr();
 
   return LV;
 }
 
 LValue