Revert "[BPF] Add preserve_access_index attribute for record definition"

This reverts commit 4a5aa1a.

There are some other test failures. Investigate them first.

Author: yonghong-song

clang/include/clang/Basic/Attr.td

@@ -332,7 +332,6 @@ class TargetArch<list<string> arches> : TargetSpec {
 }
 def TargetARM : TargetArch<["arm", "thumb", "armeb", "thumbeb"]>;
 def TargetAVR : TargetArch<["avr"]>;
-def TargetBPF : TargetArch<["bpfel", "bpfeb"]>;
 def TargetMips32 : TargetArch<["mips", "mipsel"]>;
 def TargetAnyMips : TargetArch<["mips", "mipsel", "mips64", "mips64el"]>;
 def TargetMSP430 : TargetArch<["msp430"]>;
@@ -1579,12 +1578,6 @@ def AMDGPUNumVGPR : InheritableAttr {
   let Subjects = SubjectList<[Function], ErrorDiag, "kernel functions">;
 }
 
-def BPFPreserveAccessIndex : InheritableAttr,
-                             TargetSpecificAttr<TargetBPF>  {
-  let Spellings = [Clang<"preserve_access_index">];
-  let Documentation = [BPFPreserveAccessIndexDocs];
-}
-
 def WebAssemblyImportModule : InheritableAttr,
                               TargetSpecificAttr<TargetWebAssembly> {
   let Spellings = [Clang<"import_module">];

clang/include/clang/Basic/AttrDocs.td

@@ -1634,17 +1634,6 @@ The semantics are as follows:
   }];
 }
 
-def BPFPreserveAccessIndexDocs : Documentation {
-  let Category = DocCatFunction;
-  let Content = [{
-Clang supports the ``__attribute__((preserve_access_index))``
-attribute for the BPF target. This attribute may be attached to a
-struct or union declaration, where if -g is specified, it enables
-preserving struct or union member access debuginfo indicies of this
-struct or union, similar to clang ``__builtin_preserve_acceess_index()``.
-  }];
-}
-
 def MipsInterruptDocs : Documentation {
   let Category = DocCatFunction;
   let Heading = "interrupt (MIPS)";

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10065,7 +10065,6 @@ def err_preserve_field_info_not_field : Error<
   "__builtin_preserve_field_info argument %0 not a field access">;
 def err_preserve_field_info_not_const: Error<
   "__builtin_preserve_field_info argument %0 not a constant">;
-def err_preserve_access_index_wrong_type: Error<"%0 attribute only applies to %1">;
 
 def err_bit_cast_non_trivially_copyable : Error<
   "__builtin_bit_cast %select{source|destination}0 type must be trivially copyable">;

clang/lib/CodeGen/CGExpr.cpp

@@ -3402,67 +3402,11 @@ static QualType getFixedSizeElementType(const ASTContext &ctx,
   return eltType;
 }
 
-/// Given an array base, check whether its member access belongs to a record
-/// with preserve_access_index attribute or not.
-static bool IsPreserveAIArrayBase(CodeGenFunction &CGF, const Expr *ArrayBase) {
-  if (!ArrayBase || !CGF.getDebugInfo())
-    return false;
-
-  const auto *ImplicitCast = dyn_cast<ImplicitCastExpr>(ArrayBase);
-  if (!ImplicitCast)
-    return false;
-
-  // Only support base as either a MemberExpr or DeclRefExpr.
-  // DeclRefExpr to cover cases like:
-  //    struct s { int a; int b[10]; };
-  //    struct s *p;
-  //    p[1].a
-  // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
-  // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast<MemberExpr>(E);
-  if (MemberCast)
-    return MemberCast->getMemberDecl()->hasAttr<BPFPreserveAccessIndexAttr>();
-
-  const auto *DeclRefCast = dyn_cast<DeclRefExpr>(E);
-  if (DeclRefCast) {
-    const VarDecl *VarDef = dyn_cast<VarDecl>(DeclRefCast->getDecl());
-    if (!VarDef)
-      return false;
-
-    const auto *PtrT = dyn_cast<PointerType>(VarDef->getType().getTypePtr());
-    if (!PtrT)
-      return false;
-    const auto *PointeeT = PtrT->getPointeeType().getTypePtr();
-
-    // Peel off typedef's
-    const auto *TypedefT = dyn_cast<TypedefType>(PointeeT);
-    while (TypedefT) {
-      PointeeT = TypedefT->desugar().getTypePtr();
-      TypedefT = dyn_cast<TypedefType>(PointeeT);
-    }
-
-    // Not a typedef any more, it should be an elaborated type.
-    const auto ElaborateT = dyn_cast<ElaboratedType>(PointeeT);
-    if (!ElaborateT)
-      return false;
-
-    const auto *RecT = dyn_cast<RecordType>(ElaborateT->desugar().getTypePtr());
-    if (!RecT)
-      return false;
-
-    return RecT->getDecl()->hasAttr<BPFPreserveAccessIndexAttr>();
-  }
-
-  return false;
-}
-
 static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
                                      ArrayRef<llvm::Value *> indices,
                                      QualType eltType, bool inbounds,
                                      bool signedIndices, SourceLocation loc,
                                      QualType *arrayType = nullptr,
-                                     const Expr *Base = nullptr,
                                      const llvm::Twine &name = "arrayidx") {
   // All the indices except that last must be zero.
 #ifndef NDEBUG
@@ -3484,8 +3428,7 @@ static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
 
   llvm::Value *eltPtr;
   auto LastIndex = dyn_cast<llvm::ConstantInt>(indices.back());
-  if (!LastIndex ||
-      (!CGF.IsInPreservedAIRegion && !IsPreserveAIArrayBase(CGF, Base))) {
+  if (!CGF.IsInPreservedAIRegion || !LastIndex) {
     eltPtr = emitArraySubscriptGEP(
         CGF, addr.getPointer(), indices, inbounds, signedIndices,
         loc, name);
@@ -3639,7 +3582,7 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E,
     Addr = emitArraySubscriptGEP(
         *this, ArrayLV.getAddress(), {CGM.getSize(CharUnits::Zero()), Idx},
         E->getType(), !getLangOpts().isSignedOverflowDefined(), SignedIndices,
-        E->getExprLoc(), &arrayType, E->getBase());
+        E->getExprLoc(), &arrayType);
     EltBaseInfo = ArrayLV.getBaseInfo();
     EltTBAAInfo = CGM.getTBAAInfoForSubobject(ArrayLV, E->getType());
   } else {
@@ -3649,8 +3592,7 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E,
     QualType ptrType = E->getBase()->getType();
     Addr = emitArraySubscriptGEP(*this, Addr, Idx, E->getType(),
                                  !getLangOpts().isSignedOverflowDefined(),
-                                 SignedIndices, E->getExprLoc(), &ptrType,
-                                 E->getBase());
+                                 SignedIndices, E->getExprLoc(), &ptrType);
   }
 
   LValue LV = MakeAddrLValue(Addr, E->getType(), EltBaseInfo, EltTBAAInfo);
@@ -4051,13 +3993,12 @@ LValue CodeGenFunction::EmitLValueForField(LValue base,
     const CGBitFieldInfo &Info = RL.getBitFieldInfo(field);
     Address Addr = base.getAddress();
     unsigned Idx = RL.getLLVMFieldNo(field);
-    const RecordDecl *rec = field->getParent();
-    if (!IsInPreservedAIRegion &&
-        (!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>())) {
+    if (!IsInPreservedAIRegion) {
       if (Idx != 0)
         // For structs, we GEP to the field that the record layout suggests.
         Addr = Builder.CreateStructGEP(Addr, Idx, field->getName());
     } else {
+      const RecordDecl *rec = field->getParent();
       llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateRecordType(
           getContext().getRecordType(rec), rec->getLocation());
       Addr = Builder.CreatePreserveStructAccessIndex(Addr, Idx,
@@ -4140,8 +4081,7 @@ LValue CodeGenFunction::EmitLValueForField(LValue base,
       addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),
                      addr.getAlignment());
 
-    if (IsInPreservedAIRegion ||
-        (getDebugInfo() && rec->hasAttr<BPFPreserveAccessIndexAttr>())) {
+    if (IsInPreservedAIRegion) {
       // Remember the original union field index
       llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateRecordType(
           getContext().getRecordType(rec), rec->getLocation());
@@ -4155,8 +4095,7 @@ LValue CodeGenFunction::EmitLValueForField(LValue base,
       addr = Builder.CreateElementBitCast(
           addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());
   } else {
-    if (!IsInPreservedAIRegion &&
-        (!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>()))
+    if (!IsInPreservedAIRegion)
       // For structs, we GEP to the field that the record layout suggests.
       addr = emitAddrOfFieldStorage(*this, addr, field);
     else

clang/lib/Sema/SemaDeclAttr.cpp

@@ -5700,59 +5700,6 @@ static void handleAVRSignalAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   handleSimpleAttribute<AVRSignalAttr>(S, D, AL);
 }
 
-static void handleBPFPreserveAIRecord(Sema &S, RecordDecl *RD,
-    const BPFPreserveAccessIndexAttr &AL) {
-  // Add preserve_access_index attribute to all fields and inner records.
-  for (DeclContext::decl_iterator D = RD->decls_begin(), DEnd = RD->decls_end();
-       D != DEnd; ++D) {
-    // Any member or inner struct having attribute means done.
-    if (D->hasAttr<BPFPreserveAccessIndexAttr>())
-      return;
-
-    RecordDecl *Rec = dyn_cast<RecordDecl>(*D);
-    if (Rec) {
-      Rec->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
-      handleBPFPreserveAIRecord(S, Rec, AL);
-    } else {
-      D->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
-    }
-  }
-}
-
-static void handleBPFPreserveAIRecord(Sema &S, RecordDecl *RD,
-    const ParsedAttr &AL) {
-  // Add preserve_access_index attribute to all fields and inner records.
-  for (DeclContext::decl_iterator D = RD->decls_begin(), DEnd = RD->decls_end();
-       D != DEnd; ++D) {
-    RecordDecl *Rec = dyn_cast<RecordDecl>(*D);
-    if (Rec) {
-      // Inner record may have been processed.
-      if (!Rec->hasAttr<BPFPreserveAccessIndexAttr>()) {
-        Rec->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
-        handleBPFPreserveAIRecord(S, Rec, AL);
-      }
-    } else {
-      D->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
-    }
-  }
-}
-
-static void handleBPFPreserveAccessIndexAttr(Sema &S, Decl *D,
-    const ParsedAttr &AL) {
-  RecordDecl *Rec = dyn_cast<RecordDecl>(D);
-  if (!Rec) {
-    S.Diag(D->getLocation(), diag::err_preserve_access_index_wrong_type)
-        << "preserve_addess_index" << "struct or union type";
-    return;
-  }
-
-  if (!checkAttributeNumArgs(S, AL, 0))
-    return;
-
-  handleBPFPreserveAIRecord(S, Rec, AL);
-  Rec->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
-}
-
 static void handleWebAssemblyImportModuleAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (!isFunctionOrMethod(D)) {
     S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
@@ -6629,9 +6576,6 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case ParsedAttr::AT_AVRSignal:
     handleAVRSignalAttr(S, D, AL);
     break;
-  case ParsedAttr::AT_BPFPreserveAccessIndex:
-    handleBPFPreserveAccessIndexAttr(S, D, AL);
-    break;
   case ParsedAttr::AT_WebAssemblyImportModule:
     handleWebAssemblyImportModuleAttr(S, D, AL);
     break;
@@ -7381,22 +7325,14 @@ void Sema::ProcessDeclAttributeList(Scope *S, Decl *D,
   }
 }
 
-// Helper for delayed processing TransparentUnion or BPFPreserveAccessIndexAttr
-// attribute.
+// Helper for delayed processing TransparentUnion attribute.
 void Sema::ProcessDeclAttributeDelayed(Decl *D,
                                        const ParsedAttributesView &AttrList) {
   for (const ParsedAttr &AL : AttrList)
     if (AL.getKind() == ParsedAttr::AT_TransparentUnion) {
       handleTransparentUnionAttr(*this, D, AL);
       break;
     }
-
-  // For BPFPreserveAccessIndexAttr, we want to populate the attributes
-  // to fields and inner records as well.
-  if (D->hasAttr<BPFPreserveAccessIndexAttr>()) {
-    handleBPFPreserveAIRecord(*this, cast<RecordDecl>(D),
-        *D->getAttr<BPFPreserveAccessIndexAttr>());
-  }
 }
 
 // Annotation attributes are the only attributes allowed after an access