[clang] b8d121e - [CodeGen] Require use of Address::invalid() for invalid address (NFC)

Nikita Popov via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 14 03:06:17 PST 2021


Author: Nikita Popov
Date: 2021-12-14T12:06:05+01:00
New Revision: b8d121eb1d619adca637bfd926d08a095c93b117

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

LOG: [CodeGen] Require use of Address::invalid() for invalid address (NFC)

This no longer allows creating an invalid Address through the regular
constructor. There were only two places that did this (AggValueSlot
and EHCleanupScope) which did this by converting a potential nullptr
into an Address. I've fixed both of these by directly storing an
Address instead.

This is intended as a bit of preliminary cleanup for D103465.

Differential Revision: https://reviews.llvm.org/D115630

Added: 
    

Modified: 
    clang/lib/CodeGen/Address.h
    clang/lib/CodeGen/CGCall.cpp
    clang/lib/CodeGen/CGCleanup.h
    clang/lib/CodeGen/CGExprAgg.cpp
    clang/lib/CodeGen/CGValue.h

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index 6a8e57f8db33d..0403a89b8ce61 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -24,14 +24,18 @@ namespace CodeGen {
 class Address {
   llvm::Value *Pointer;
   CharUnits Alignment;
+
+protected:
+  Address(nullptr_t) : Pointer(nullptr) {}
+
 public:
   Address(llvm::Value *pointer, CharUnits alignment)
       : Pointer(pointer), Alignment(alignment) {
-    assert((!alignment.isZero() || pointer == nullptr) &&
-           "creating valid address with invalid alignment");
+    assert(pointer != nullptr && "Pointer cannot be null");
+    assert(!alignment.isZero() && "Alignment cannot be zero");
   }
 
-  static Address invalid() { return Address(nullptr, CharUnits()); }
+  static Address invalid() { return Address(nullptr); }
   bool isValid() const { return Pointer != nullptr; }
 
   llvm::Value *getPointer() const {
@@ -72,12 +76,14 @@ class Address {
 /// A specialization of Address that requires the address to be an
 /// LLVM Constant.
 class ConstantAddress : public Address {
+  ConstantAddress(nullptr_t) : Address(nullptr) {}
+
 public:
   ConstantAddress(llvm::Constant *pointer, CharUnits alignment)
     : Address(pointer, alignment) {}
 
   static ConstantAddress invalid() {
-    return ConstantAddress(nullptr, CharUnits());
+    return ConstantAddress(nullptr);
   }
 
   llvm::Constant *getPointer() const {

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index b0b64328b84ed..76d3cffbe6593 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4308,11 +4308,8 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
       type->castAs<RecordType>()->getDecl()->isParamDestroyedInCallee()) {
     // If we're using inalloca, use the argument memory.  Otherwise, use a
     // temporary.
-    AggValueSlot Slot;
-    if (args.isUsingInAlloca())
-      Slot = createPlaceholderSlot(*this, type);
-    else
-      Slot = CreateAggTemp(type, "agg.tmp");
+    AggValueSlot Slot = args.isUsingInAlloca()
+        ? createPlaceholderSlot(*this, type) : CreateAggTemp(type, "agg.tmp");
 
     bool DestroyedInCallee = true, NeedsEHCleanup = true;
     if (const auto *RD = type->getAsCXXRecordDecl())

diff  --git a/clang/lib/CodeGen/CGCleanup.h b/clang/lib/CodeGen/CGCleanup.h
index 1b54c0018d27c..76f3a48f32f34 100644
--- a/clang/lib/CodeGen/CGCleanup.h
+++ b/clang/lib/CodeGen/CGCleanup.h
@@ -242,7 +242,7 @@ class alignas(8) EHCleanupScope : public EHScope {
 
   /// An optional i1 variable indicating whether this cleanup has been
   /// activated yet.
-  llvm::AllocaInst *ActiveFlag;
+  Address ActiveFlag;
 
   /// Extra information required for cleanups that have resolved
   /// branches through them.  This has to be allocated on the side
@@ -290,7 +290,8 @@ class alignas(8) EHCleanupScope : public EHScope {
                  EHScopeStack::stable_iterator enclosingEH)
       : EHScope(EHScope::Cleanup, enclosingEH),
         EnclosingNormal(enclosingNormal), NormalBlock(nullptr),
-        ActiveFlag(nullptr), ExtInfo(nullptr), FixupDepth(fixupDepth) {
+        ActiveFlag(Address::invalid()), ExtInfo(nullptr),
+        FixupDepth(fixupDepth) {
     CleanupBits.IsNormalCleanup = isNormal;
     CleanupBits.IsEHCleanup = isEH;
     CleanupBits.IsActive = true;
@@ -320,13 +321,13 @@ class alignas(8) EHCleanupScope : public EHScope {
   bool isLifetimeMarker() const { return CleanupBits.IsLifetimeMarker; }
   void setLifetimeMarker() { CleanupBits.IsLifetimeMarker = true; }
 
-  bool hasActiveFlag() const { return ActiveFlag != nullptr; }
+  bool hasActiveFlag() const { return ActiveFlag.isValid(); }
   Address getActiveFlag() const {
-    return Address(ActiveFlag, CharUnits::One());
+    return ActiveFlag;
   }
   void setActiveFlag(Address Var) {
     assert(Var.getAlignment().isOne());
-    ActiveFlag = cast<llvm::AllocaInst>(Var.getPointer());
+    ActiveFlag = Var;
   }
 
   void setTestFlagInNormalCleanup() {

diff  --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 5b56a587fa5f7..cc7a333d62907 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -301,7 +301,7 @@ void AggExprEmitter::withReturnValueSlot(
   if (!UseTemp)
     return;
 
-  assert(Dest.getPointer() != Src.getAggregatePointer());
+  assert(Dest.isIgnored() || Dest.getPointer() != Src.getAggregatePointer());
   EmitFinalDestCopy(E->getType(), Src);
 
   if (!RequiresDestruction && LifetimeStartInst) {

diff  --git a/clang/lib/CodeGen/CGValue.h b/clang/lib/CodeGen/CGValue.h
index 4b39a0520833f..cf8d2aa038b56 100644
--- a/clang/lib/CodeGen/CGValue.h
+++ b/clang/lib/CodeGen/CGValue.h
@@ -470,13 +470,11 @@ class LValue {
 /// An aggregate value slot.
 class AggValueSlot {
   /// The address.
-  llvm::Value *Addr;
+  Address Addr;
 
   // Qualifiers
   Qualifiers Quals;
 
-  unsigned Alignment;
-
   /// DestructedFlag - This is set to true if some external code is
   /// responsible for setting up a destructor for the slot.  Otherwise
   /// the code which constructs it should push the appropriate cleanup.
@@ -520,6 +518,14 @@ class AggValueSlot {
   /// them.
   bool SanitizerCheckedFlag : 1;
 
+  AggValueSlot(Address Addr, Qualifiers Quals, bool DestructedFlag,
+               bool ObjCGCFlag, bool ZeroedFlag, bool AliasedFlag,
+               bool OverlapFlag, bool SanitizerCheckedFlag)
+      : Addr(Addr), Quals(Quals), DestructedFlag(DestructedFlag),
+        ObjCGCFlag(ObjCGCFlag), ZeroedFlag(ZeroedFlag),
+        AliasedFlag(AliasedFlag), OverlapFlag(OverlapFlag),
+        SanitizerCheckedFlag(SanitizerCheckedFlag) {}
+
 public:
   enum IsAliased_t { IsNotAliased, IsAliased };
   enum IsDestructed_t { IsNotDestructed, IsDestructed };
@@ -553,22 +559,8 @@ class AggValueSlot {
                               Overlap_t mayOverlap,
                               IsZeroed_t isZeroed = IsNotZeroed,
                        IsSanitizerChecked_t isChecked = IsNotSanitizerChecked) {
-    AggValueSlot AV;
-    if (addr.isValid()) {
-      AV.Addr = addr.getPointer();
-      AV.Alignment = addr.getAlignment().getQuantity();
-    } else {
-      AV.Addr = nullptr;
-      AV.Alignment = 0;
-    }
-    AV.Quals = quals;
-    AV.DestructedFlag = isDestructed;
-    AV.ObjCGCFlag = needsGC;
-    AV.ZeroedFlag = isZeroed;
-    AV.AliasedFlag = isAliased;
-    AV.OverlapFlag = mayOverlap;
-    AV.SanitizerCheckedFlag = isChecked;
-    return AV;
+    return AggValueSlot(addr, quals, isDestructed, needsGC, isZeroed, isAliased,
+                        mayOverlap, isChecked);
   }
 
   static AggValueSlot
@@ -609,19 +601,19 @@ class AggValueSlot {
   }
 
   llvm::Value *getPointer() const {
-    return Addr;
+    return Addr.getPointer();
   }
 
   Address getAddress() const {
-    return Address(Addr, getAlignment());
+    return Addr;
   }
 
   bool isIgnored() const {
-    return Addr == nullptr;
+    return !Addr.isValid();
   }
 
   CharUnits getAlignment() const {
-    return CharUnits::fromQuantity(Alignment);
+    return Addr.getAlignment();
   }
 
   IsAliased_t isPotentiallyAliased() const {


        


More information about the cfe-commits mailing list