Merged main:f58d54ab969b into amd-gfx:25a5d3ddbc50

Local branch amd-gfx 25a5d3d Merged main:0502d8347020 into amd-gfx:4191fabcf57f
Remote branch main f58d54a [clang][Interp] Diagnose uninitialized bases (llvm#67131)

Author: SC llvm team

clang/lib/AST/Interp/ByteCodeExprGen.cpp

@@ -561,7 +561,7 @@ bool ByteCodeExprGen<Emitter>::visitInitList(ArrayRef<const Expr *> Inits,
         if (!this->visitInitializer(Init))
           return false;
 
-        if (!this->emitPopPtr(E))
+        if (!this->emitInitPtrPop(E))
           return false;
         // Base initializers don't increase InitIndex, since they don't count
         // into the Record's fields.
@@ -1718,7 +1718,7 @@ bool ByteCodeExprGen<Emitter>::visitZeroRecordInitializer(const Record *R,
       return false;
     if (!this->visitZeroRecordInitializer(B.R, E))
       return false;
-    if (!this->emitPopPtr(E))
+    if (!this->emitInitPtrPop(E))
       return false;
   }
 

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

@@ -191,7 +191,7 @@ bool ByteCodeStmtGen<Emitter>::visitFunc(const FunctionDecl *F) {
           return false;
         if (!this->visitInitializer(InitExpr))
           return false;
-        if (!this->emitPopPtr(InitExpr))
+        if (!this->emitInitPtrPop(InitExpr))
           return false;
       }
     }

clang/lib/AST/Interp/IntegralAP.h

@@ -32,9 +32,15 @@ template <unsigned Bits, bool Signed> class Integral;
 class Boolean;
 
 template <bool Signed> class IntegralAP final {
-public:
+private:
+  friend IntegralAP<!Signed>;
   APSInt V;
 
+  template <typename T> static T truncateCast(const APSInt &V) {
+    return std::is_signed_v<T> ? V.trunc(sizeof(T) * 8).getSExtValue()
+                               : V.trunc(sizeof(T) * 8).getZExtValue();
+  }
+
 public:
   using AsUnsigned = IntegralAP<false>;
 
@@ -55,14 +61,14 @@ template <bool Signed> class IntegralAP final {
   bool operator<=(IntegralAP RHS) const { return V <= RHS.V; }
 
   explicit operator bool() const { return !V.isZero(); }
-  explicit operator int8_t() const { return V.getSExtValue(); }
-  explicit operator uint8_t() const { return V.getZExtValue(); }
-  explicit operator int16_t() const { return V.getSExtValue(); }
-  explicit operator uint16_t() const { return V.getZExtValue(); }
-  explicit operator int32_t() const { return V.getSExtValue(); }
-  explicit operator uint32_t() const { return V.getZExtValue(); }
-  explicit operator int64_t() const { return V.getSExtValue(); }
-  explicit operator uint64_t() const { return V.getZExtValue(); }
+  explicit operator int8_t() const { return truncateCast<int8_t>(V); }
+  explicit operator uint8_t() const { return truncateCast<uint8_t>(V); }
+  explicit operator int16_t() const { return truncateCast<int16_t>(V); }
+  explicit operator uint16_t() const { return truncateCast<uint16_t>(V); }
+  explicit operator int32_t() const { return truncateCast<int32_t>(V); }
+  explicit operator uint32_t() const { return truncateCast<uint32_t>(V); }
+  explicit operator int64_t() const { return truncateCast<int64_t>(V); }
+  explicit operator uint64_t() const { return truncateCast<uint64_t>(V); }
 
   template <typename T> static IntegralAP from(T Value, unsigned NumBits = 0) {
     assert(NumBits > 0);

clang/lib/AST/Interp/Interp.cpp

@@ -467,6 +467,12 @@ static bool CheckFieldsInitialized(InterpState &S, CodePtr OpPC,
   // Check Fields in all bases
   for (const Record::Base &B : R->bases()) {
     Pointer P = BasePtr.atField(B.Offset);
+    if (!P.isInitialized()) {
+      S.FFDiag(BasePtr.getDeclDesc()->asDecl()->getLocation(),
+               diag::note_constexpr_uninitialized_base)
+          << B.Desc->getType();
+      return false;
+    }
     Result &= CheckFieldsInitialized(S, OpPC, P, B.R);
   }
 

clang/lib/AST/Interp/Interp.h

@@ -1245,6 +1245,12 @@ inline bool GetPtrThisBase(InterpState &S, CodePtr OpPC, uint32_t Off) {
   return true;
 }
 
+inline bool InitPtrPop(InterpState &S, CodePtr OpPC) {
+  const Pointer &Ptr = S.Stk.pop<Pointer>();
+  Ptr.initialize();
+  return true;
+}
+
 inline bool VirtBaseHelper(InterpState &S, CodePtr OpPC, const RecordDecl *Decl,
                            const Pointer &Ptr) {
   Pointer Base = Ptr;

clang/lib/AST/Interp/Opcodes.td

@@ -304,6 +304,10 @@ def GetPtrBasePop : Opcode {
   let Args = [ArgUint32];
 }
 
+def InitPtrPop : Opcode {
+  let Args = [];
+}
+
 def GetPtrDerivedPop : Opcode {
   let Args = [ArgUint32];
 }

clang/test/AST/Interp/constexpr-subobj-initialization.cpp

@@ -0,0 +1,73 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -fexperimental-new-constant-interpreter
+
+/// This is like the version in test/SemaCXX/, but some of the
+/// output types and their location has been adapted.
+/// Differences:
+///   1) The type of the uninitialized base class is printed WITH the namespace,
+///      i.e. 'baseclass_uninit::DelBase' instead of just 'DelBase'.
+///   2) The location is not the base specifier declaration, but the call site
+///      of the constructor.
+
+
+namespace baseclass_uninit {
+struct DelBase {
+  constexpr DelBase() = delete; // expected-note {{'DelBase' has been explicitly marked deleted here}}
+};
+
+struct Foo : DelBase {
+  constexpr Foo() {}; // expected-error {{call to deleted constructor of 'DelBase'}}
+};
+constexpr Foo f; // expected-error {{must be initialized by a constant expression}} \
+                 // expected-note {{constructor of base class 'baseclass_uninit::DelBase' is not called}}
+
+struct Bar : Foo {
+  constexpr Bar() {};
+};
+constexpr Bar bar; // expected-error {{must be initialized by a constant expression}} \
+                   // expected-note {{constructor of base class 'baseclass_uninit::DelBase' is not called}}
+
+struct Base {};
+struct A : Base {
+  constexpr A() : value() {} // expected-error {{member initializer 'value' does not name a non-static data member or base class}}
+};
+
+constexpr A a; // expected-error {{must be initialized by a constant expression}} \
+               // expected-note {{constructor of base class 'baseclass_uninit::Base' is not called}}
+
+
+struct B : Base {
+  constexpr B() : {} // expected-error {{expected class member or base class name}}
+};
+
+constexpr B b; // expected-error {{must be initialized by a constant expression}} \
+               // expected-note {{constructor of base class 'baseclass_uninit::Base' is not called}}
+} // namespace baseclass_uninit
+
+
+struct Foo {
+  constexpr Foo(); // expected-note 2{{declared here}}
+};
+
+constexpr Foo ff; // expected-error {{must be initialized by a constant expression}} \
+                  // expected-note {{undefined constructor 'Foo' cannot be used in a constant expression}}
+
+struct Bar : protected Foo {
+  int i;
+  constexpr Bar() : i(12) {} // expected-note {{undefined constructor 'Foo' cannot be used in a constant expression}}
+};
+
+constexpr Bar bb; // expected-error {{must be initialized by a constant expression}} \
+                  // expected-note {{in call to 'Bar()'}}
+
+template <typename Ty>
+struct Baz {
+  constexpr Baz(); // expected-note {{declared here}}
+};
+
+struct Quux : Baz<Foo>, private Bar {
+  int i;
+  constexpr Quux() : i(12) {} // expected-note {{undefined constructor 'Baz' cannot be used in a constant expression}}
+};
+
+constexpr Quux qx; // expected-error {{must be initialized by a constant expression}} \
+                   // expected-note {{in call to 'Quux()'}}

clang/test/AST/Interp/literals.cpp

@@ -53,7 +53,7 @@ namespace i128 {
   static_assert(Two == 2, "");
 
   constexpr uint128_t AllOnes = ~static_cast<uint128_t>(0);
-  static_assert(AllOnes == static_cast<uint128_t>(-1), "");
+  static_assert(AllOnes == UINT128_MAX, "");
 
 #if __cplusplus >= 201402L
   template <typename T>
@@ -70,6 +70,14 @@ namespace i128 {
   static_assert(CastFrom<double>(12) == 12, "");
   static_assert(CastFrom<long double>(12) == 12, "");
 
+  static_assert(CastFrom<char>(AllOnes) == -1, "");
+  static_assert(CastFrom<unsigned char>(AllOnes) == 0xFF, "");
+  static_assert(CastFrom<long>(AllOnes) == -1, "");
+  static_assert(CastFrom<unsigned short>(AllOnes) == 0xFFFF, "");
+  static_assert(CastFrom<int>(AllOnes) == -1, "");
+  static_assert(CastFrom<int128_t>(AllOnes) == -1, "");
+  static_assert(CastFrom<uint128_t>(AllOnes) == AllOnes, "");
+
   template <typename T>
   constexpr __int128 CastTo(T A) {
     int128_t B = (int128_t)A;

llvm/include/llvm/Analysis/TargetTransformInfo.h

@@ -348,6 +348,9 @@ class TargetTransformInfo {
   /// individual classes of instructions would be better.
   unsigned getInliningThresholdMultiplier() const;
 
+  unsigned getInliningCostBenefitAnalysisSavingsMultiplier() const;
+  unsigned getInliningCostBenefitAnalysisProfitableMultiplier() const;
+
   /// \returns A value to be added to the inlining threshold.
   unsigned adjustInliningThreshold(const CallBase *CB) const;
 
@@ -1696,6 +1699,9 @@ class TargetTransformInfo::Concept {
                        const TTI::PointersChainInfo &Info, Type *AccessTy,
                        TTI::TargetCostKind CostKind) = 0;
   virtual unsigned getInliningThresholdMultiplier() const = 0;
+  virtual unsigned getInliningCostBenefitAnalysisSavingsMultiplier() const = 0;
+  virtual unsigned
+  getInliningCostBenefitAnalysisProfitableMultiplier() const = 0;
   virtual unsigned adjustInliningThreshold(const CallBase *CB) = 0;
   virtual int getInlinerVectorBonusPercent() const = 0;
   virtual unsigned getCallerAllocaCost(const CallBase *CB,
@@ -2068,6 +2074,12 @@ class TargetTransformInfo::Model final : public TargetTransformInfo::Concept {
   unsigned adjustInliningThreshold(const CallBase *CB) override {
     return Impl.adjustInliningThreshold(CB);
   }
+  unsigned getInliningCostBenefitAnalysisSavingsMultiplier() const override {
+    return Impl.getInliningCostBenefitAnalysisSavingsMultiplier();
+  }
+  unsigned getInliningCostBenefitAnalysisProfitableMultiplier() const override {
+    return Impl.getInliningCostBenefitAnalysisProfitableMultiplier();
+  }
   int getInlinerVectorBonusPercent() const override {
     return Impl.getInlinerVectorBonusPercent();
   }

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

@@ -69,6 +69,10 @@ class TargetTransformInfoImplBase {
   }
 
   unsigned getInliningThresholdMultiplier() const { return 1; }
+  unsigned getInliningCostBenefitAnalysisSavingsMultiplier() const { return 8; }
+  unsigned getInliningCostBenefitAnalysisProfitableMultiplier() const {
+    return 8;
+  }
   unsigned adjustInliningThreshold(const CallBase *CB) const { return 0; }
   unsigned getCallerAllocaCost(const CallBase *CB, const AllocaInst *AI) const {
     return 0;

llvm/include/llvm/Config/llvm-config.h.cmake

@@ -16,7 +16,7 @@
 
 /* Indicate that this is LLVM compiled from the amd-gfx branch. */
 #define LLVM_HAVE_BRANCH_AMD_GFX
-#define LLVM_MAIN_REVISION 476604
+#define LLVM_MAIN_REVISION 476609
 
 /* Define if LLVM_ENABLE_DUMP is enabled */
 #cmakedefine LLVM_ENABLE_DUMP

llvm/lib/Analysis/InlineCost.cpp

@@ -88,10 +88,21 @@ static cl::opt<bool> InlineEnableCostBenefitAnalysis(
     "inline-enable-cost-benefit-analysis", cl::Hidden, cl::init(false),
     cl::desc("Enable the cost-benefit analysis for the inliner"));
 
+// InlineSavingsMultiplier overrides per TTI multipliers iff it is
+// specified explicitly in command line options. This option is exposed
+// for tuning and testing.
 static cl::opt<int> InlineSavingsMultiplier(
     "inline-savings-multiplier", cl::Hidden, cl::init(8),
     cl::desc("Multiplier to multiply cycle savings by during inlining"));
 
+// InlineSavingsProfitableMultiplier overrides per TTI multipliers iff it is
+// specified explicitly in command line options. This option is exposed
+// for tuning and testing.
+static cl::opt<int> InlineSavingsProfitableMultiplier(
+    "inline-savings-profitable-multiplier", cl::Hidden, cl::init(4),
+    cl::desc("A multiplier on top of cycle savings to decide whether the "
+             "savings won't justify the cost"));
+
 static cl::opt<int>
     InlineSizeAllowance("inline-size-allowance", cl::Hidden, cl::init(100),
                         cl::desc("The maximum size of a callee that get's "
@@ -815,6 +826,32 @@ class InlineCostCallAnalyzer final : public CallAnalyzer {
     return true;
   }
 
+  // A helper function to choose between command line override and default.
+  unsigned getInliningCostBenefitAnalysisSavingsMultiplier() const {
+    if (InlineSavingsMultiplier.getNumOccurrences())
+      return InlineSavingsMultiplier;
+    return TTI.getInliningCostBenefitAnalysisSavingsMultiplier();
+  }
+
+  // A helper function to choose between command line override and default.
+  unsigned getInliningCostBenefitAnalysisProfitableMultiplier() const {
+    if (InlineSavingsProfitableMultiplier.getNumOccurrences())
+      return InlineSavingsProfitableMultiplier;
+    return TTI.getInliningCostBenefitAnalysisProfitableMultiplier();
+  }
+
+  void OverrideCycleSavingsAndSizeForTesting(APInt &CycleSavings, int &Size) {
+    if (std::optional<int> AttrCycleSavings = getStringFnAttrAsInt(
+            CandidateCall, "inline-cycle-savings-for-test")) {
+      CycleSavings = *AttrCycleSavings;
+    }
+
+    if (std::optional<int> AttrRuntimeCost = getStringFnAttrAsInt(
+            CandidateCall, "inline-runtime-cost-for-test")) {
+      Size = *AttrRuntimeCost;
+    }
+  }
+
   // Determine whether we should inline the given call site, taking into account
   // both the size cost and the cycle savings.  Return std::nullopt if we don't
   // have sufficient profiling information to determine.
@@ -884,29 +921,55 @@ class InlineCostCallAnalyzer final : public CallAnalyzer {
     CycleSavings += getCallsiteCost(this->CandidateCall, DL);
     CycleSavings *= *CallerBFI->getBlockProfileCount(CallerBB);
 
-    // Remove the cost of the cold basic blocks.
+    // Remove the cost of the cold basic blocks to model the runtime cost more
+    // accurately. Both machine block placement and function splitting could
+    // place cold blocks further from hot blocks.
     int Size = Cost - ColdSize;
 
     // Allow tiny callees to be inlined regardless of whether they meet the
     // savings threshold.
     Size = Size > InlineSizeAllowance ? Size - InlineSizeAllowance : 1;
 
+    OverrideCycleSavingsAndSizeForTesting(CycleSavings, Size);
     CostBenefit.emplace(APInt(128, Size), CycleSavings);
 
-    // Return true if the savings justify the cost of inlining.  Specifically,
-    // we evaluate the following inequality:
+    // Let R be the ratio of CycleSavings to Size.  We accept the inlining
+    // opportunity if R is really high and reject if R is really low.  If R is
+    // somewhere in the middle, we fall back to the cost-based analysis.
     //
-    //  CycleSavings      PSI->getOrCompHotCountThreshold()
-    // -------------- >= -----------------------------------
-    //       Size              InlineSavingsMultiplier
+    // Specifically, let R = CycleSavings / Size, we accept the inlining
+    // opportunity if:
     //
-    // Note that the left hand side is specific to a call site.  The right hand
-    // side is a constant for the entire executable.
-    APInt LHS = CycleSavings;
-    LHS *= InlineSavingsMultiplier;
-    APInt RHS(128, PSI->getOrCompHotCountThreshold());
-    RHS *= Size;
-    return LHS.uge(RHS);
+    //             PSI->getOrCompHotCountThreshold()
+    // R > -------------------------------------------------
+    //     getInliningCostBenefitAnalysisSavingsMultiplier()
+    //
+    // and reject the inlining opportunity if:
+    //
+    //                PSI->getOrCompHotCountThreshold()
+    // R <= ----------------------------------------------------
+    //      getInliningCostBenefitAnalysisProfitableMultiplier()
+    //
+    // Otherwise, we fall back to the cost-based analysis.
+    //
+    // Implementation-wise, use multiplication (CycleSavings * Multiplier,
+    // HotCountThreshold * Size) rather than division to avoid precision loss.
+    APInt Threshold(128, PSI->getOrCompHotCountThreshold());
+    Threshold *= Size;
+
+    APInt UpperBoundCycleSavings = CycleSavings;
+    UpperBoundCycleSavings *= getInliningCostBenefitAnalysisSavingsMultiplier();
+    if (UpperBoundCycleSavings.uge(Threshold))
+      return true;
+
+    APInt LowerBoundCycleSavings = CycleSavings;
+    LowerBoundCycleSavings *=
+        getInliningCostBenefitAnalysisProfitableMultiplier();
+    if (LowerBoundCycleSavings.ult(Threshold))
+      return false;
+
+    // Otherwise, fall back to the cost-based analysis.
+    return std::nullopt;
   }
 
   InlineResult finalizeAnalysis() override {

llvm/lib/Analysis/TargetTransformInfo.cpp

@@ -212,6 +212,17 @@ unsigned TargetTransformInfo::getInliningThresholdMultiplier() const {
   return TTIImpl->getInliningThresholdMultiplier();
 }
 
+unsigned
+TargetTransformInfo::getInliningCostBenefitAnalysisSavingsMultiplier() const {
+  return TTIImpl->getInliningCostBenefitAnalysisSavingsMultiplier();
+}
+
+unsigned
+TargetTransformInfo::getInliningCostBenefitAnalysisProfitableMultiplier()
+    const {
+  return TTIImpl->getInliningCostBenefitAnalysisProfitableMultiplier();
+}
+
 unsigned
 TargetTransformInfo::adjustInliningThreshold(const CallBase *CB) const {
   return TTIImpl->adjustInliningThreshold(CB);