[CodeGen] Pessimize aliasing for union members (and may-alias) objects

[kparzysz]

Summary

Use the TBAA info of the omnipotent char for these objects.

[kparzysz]

The diff is the same as before, the change is in the subscribers: from llvm-commits to cfe-commits.

[kparzysz]

Previously, the variable mayAlias in EmitLValueForField could have been false even if the base LValue had may-alias flag set. Make sure that mayAlias reflects the base aliasing.

This fixed the C++ testcase and it's now included in the patch.

[hfinkel]

test/CodeGen/union-tbaa1.c

45

Please check the path offset?

test/CodeGenCXX/union-tbaa2.cpp

45 ↗

(On Diff #99918)

Please check the path offset.

[kparzysz]

test/CodeGen/union-tbaa1.c

45

What needs to be checked exactly? The offset will be 0, do you want to check for that?

[hfinkel]

test/CodeGen/union-tbaa1.c

45

Yes, because if it's not zero, this won't have the desired effect.

[kparzysz]

Checking for offset 0 in the TBAA info.

[rjmccall]

lib/CodeGen/CGExpr.cpp

1436

Hmm. Should we be constructing a struct-path TBAA at all if the base may alias, as opposed to just using 'char'?

[kparzysz]

lib/CodeGen/CGExpr.cpp

1436

I did that and got a verifier error: "Old-style TBAA is no longer allowed, use struct-path TBAA instead. fatal error: error in backend: Broken function found, compilation aborted!"

[rjmccall]

lib/CodeGen/CGExpr.cpp

1436

Oh, I haven't been paying enough attention to LLVM's TBAA changes. It looks like they've canonicalized on always using the struct-access-style TBAA metadata, which is fine; the point is that we should be using whatever tag we would use for dereferencing a simple char*, which I believe just means passing true for ConvertTypeToTag to DecorateInstructionWithTBAA .

[kparzysz]

lib/CodeGen/CGExpr.cpp

1436

With that change I'm getting another error:

Access type node must be a valid scalar type
  store <4 x double> %call2, <4 x double>* %b, align 32, !tbaa !8
!8 = !{!9, !9, i64 0}
!9 = !{!4, !4, i64 0}
fatal error: error in backend: Broken function found, compilation aborted!

[rjmccall]

lib/CodeGen/CGExpr.cpp

1436

Sorry, I see now that what I said was unclear. If you do this, you need to pass the result of getTBAAInfo(CharTy) down as the argument to DecorateInstructionWithTBAA instead of building of a TBAA struct tag.

[kparzysz]

Pass char TBAA directly to DecorateInstructionWithTBAA and pass true for ConvertTypeToTag in such cases.

[rjmccall]

Thanks, that looks great.

lib/CodeGen/CGExpr.cpp

llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool Volatile,
 
   llvm::LoadInst *Load = Builder.CreateLoad(Addr, Volatile);
   if (isNontemporal) {
     llvm::MDNode *Node = llvm::MDNode::get(
         Load->getContext(), llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
     Load->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
   }
   if (TBAAInfo) {
+    if (BaseInfo.getMayAlias())
+      TBAAInfo = CGM.getTBAAInfo(getContext().CharTy);
     llvm::MDNode *TBAAPath = CGM.getTBAAStructTagInfo(TBAABaseType, TBAAInfo,
                                                       TBAAOffset);
     if (TBAAPath)
       CGM.DecorateInstructionWithTBAA(Load, TBAAPath,
                                       false /*ConvertTypeToTag*/);
   }
 
   if (EmitScalarRangeCheck(Load, Ty, Loc)) {
void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, Address Addr,
   llvm::StoreInst *Store = Builder.CreateStore(Value, Addr, Volatile);
   if (isNontemporal) {
     llvm::MDNode *Node =
         llvm::MDNode::get(Store->getContext(),
                           llvm::ConstantAsMetadata::get(Builder.getInt32(1)));
     Store->setMetadata(CGM.getModule().getMDKindID("nontemporal"), Node);
   }
   if (TBAAInfo) {
+    if (BaseInfo.getMayAlias())
+      TBAAInfo = CGM.getTBAAInfo(getContext().CharTy);
     llvm::MDNode *TBAAPath = CGM.getTBAAStructTagInfo(TBAABaseType, TBAAInfo,
                                                       TBAAOffset);
     if (TBAAPath)
       CGM.DecorateInstructionWithTBAA(Store, TBAAPath,
                                       false /*ConvertTypeToTag*/);
   }
 }
 

 
 LValue CodeGenFunction::EmitLValueForField(LValue base,
                                            const FieldDecl *field) {
   LValueBaseInfo BaseInfo = base.getBaseInfo();
   AlignmentSource fieldAlignSource =
     getFieldAlignmentSource(BaseInfo.getAlignmentSource());
   LValueBaseInfo FieldBaseInfo(fieldAlignSource, BaseInfo.getMayAlias());
 
+  const RecordDecl *rec = field->getParent();
+  bool mayAlias = rec->isUnion() || rec->hasAttr<MayAliasAttr>();
+  if (mayAlias)
+    FieldBaseInfo.setMayAlias(true);
+
   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());
     return LValue::MakeBitfield(Addr, Info, fieldType, FieldBaseInfo);
   }
 
-  const RecordDecl *rec = field->getParent();
   QualType type = field->getType();
-
-  bool mayAlias = rec->hasAttr<MayAliasAttr>();
-
   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;

test/CodeGen/union-tbaa1.c

+// RUN: %clang_cc1 %s -triple hexagon-unknown-elf -O2 -emit-llvm -o - | FileCheck %s
+
+typedef union __attribute__((aligned(4))) {
+  unsigned short uh[2];
+  unsigned uw;
+} vect32;
+
+void bar(vect32 p[][2]);
+
+// CHECK-LABEL: define void @fred
+void fred(unsigned Num, int Vec[2], int *Index, int Arr[4][2]) {
+  vect32 Tmp[4][2];
+// Generate tbaa for the load of Index:
+// CHECK: load i32, i32* %Index{{.*}}tbaa
+// But no tbaa for the two stores:
+// CHECK: %uw[[UW1:[0-9]*]] = getelementptr
+// CHECK: store{{.*}}%uw[[UW1]]
+// CHECK: tbaa ![[OCPATH:[0-9]+]]
+// There will be a load after the store, and it will use tbaa. Make sure
+// the check-not above doesn't find it:
+// CHECK: load
+  Tmp[*Index][0].uw = Arr[*Index][0] * Num;
+// CHECK: %uw[[UW2:[0-9]*]] = getelementptr
+// CHECK: store{{.*}}%uw[[UW2]]
+// CHECK: tbaa ![[OCPATH]]
+  Tmp[*Index][1].uw = Arr[*Index][1] * Num;
+// Same here, don't generate tbaa for the loads:
+// CHECK: %uh[[UH1:[0-9]*]] = bitcast %union.vect32
+// CHECK: %arrayidx[[AX1:[0-9]*]] = getelementptr{{.*}}%uh[[UH1]]
+// CHECK: load i16, i16* %arrayidx[[AX1]]
+// CHECK: tbaa ![[OCPATH]]
+// CHECK: store
+  Vec[0] = Tmp[*Index][0].uh[1];
+// CHECK: %uh[[UH2:[0-9]*]] = bitcast %union.vect32
+// CHECK: %arrayidx[[AX2:[0-9]*]] = getelementptr{{.*}}%uh[[UH2]]
+// CHECK: load i16, i16* %arrayidx[[AX2]]
+// CHECK: tbaa ![[OCPATH]]
+// CHECK: store
+  Vec[1] = Tmp[*Index][1].uh[1];
+  bar(Tmp);
+}
+
+// CHECK: ![[CHAR:[0-9]+]] = !{!"omnipotent char"
+// CHECK: ![[OCPATH]] = !{![[CHAR]]