Merged main:b797a6aede3d into amd-gfx:a757a754c785

Local branch amd-gfx a757a75 Merged main:b0e28eb83271 into amd-gfx:41686a6f1391
Remote branch main b797a6a [flang] Lower special bind(c) cases without binding labels (llvm#65758)

Author: SC llvm team

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

@@ -260,6 +260,13 @@ class Environment {
   /// if `D` isn't assigned a storage location in the environment.
   StorageLocation *getStorageLocation(const ValueDecl &D) const;
 
+  /// Removes the location assigned to `D` in the environment.
+  ///
+  /// Requirements:
+  ///
+  ///  `D` must have a storage location assigned in the environment.
+  void removeDecl(const ValueDecl &D);
+
   /// Assigns `Loc` as the storage location of the glvalue `E` in the
   /// environment.
   ///

clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp

@@ -97,6 +97,7 @@ ControlFlowContext::build(const Decl &D, Stmt &S, ASTContext &C) {
   Options.AddTemporaryDtors = true;
   Options.AddInitializers = true;
   Options.AddCXXDefaultInitExprInCtors = true;
+  Options.AddLifetime = true;
 
   // Ensure that all sub-expressions in basic blocks are evaluated.
   Options.setAllAlwaysAdd();

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

@@ -35,6 +35,20 @@ namespace dataflow {
 static constexpr int MaxCompositeValueDepth = 3;
 static constexpr int MaxCompositeValueSize = 1000;
 
+/// Returns whether all declarations that `DeclToLoc1` and `DeclToLoc2` have in
+/// common map to the same storage location in both maps.
+bool declToLocConsistent(
+    const llvm::DenseMap<const ValueDecl *, StorageLocation *> &DeclToLoc1,
+    const llvm::DenseMap<const ValueDecl *, StorageLocation *> &DeclToLoc2) {
+  for (auto &Entry : DeclToLoc1) {
+    auto It = DeclToLoc2.find(Entry.first);
+    if (It != DeclToLoc2.end() && Entry.second != It->second)
+      return false;
+  }
+
+  return true;
+}
+
 /// Returns a map consisting of key-value entries that are present in both maps.
 template <typename K, typename V>
 llvm::DenseMap<K, V> intersectDenseMaps(const llvm::DenseMap<K, V> &Map1,
@@ -636,10 +650,7 @@ Environment Environment::join(const Environment &EnvA, const Environment &EnvB,
   else
     JoinedEnv.ReturnLoc = nullptr;
 
-  // FIXME: Once we're able to remove declarations from `DeclToLoc` when their
-  // lifetime ends, add an assertion that there aren't any entries in
-  // `DeclToLoc` and `Other.DeclToLoc` that map the same declaration to
-  // different storage locations.
+  assert(declToLocConsistent(EnvA.DeclToLoc, EnvB.DeclToLoc));
   JoinedEnv.DeclToLoc = intersectDenseMaps(EnvA.DeclToLoc, EnvB.DeclToLoc);
 
   JoinedEnv.ExprToLoc = intersectDenseMaps(EnvA.ExprToLoc, EnvB.ExprToLoc);
@@ -691,6 +702,11 @@ StorageLocation *Environment::getStorageLocation(const ValueDecl &D) const {
   return Loc;
 }
 
+void Environment::removeDecl(const ValueDecl &D) {
+  assert(DeclToLoc.contains(&D));
+  DeclToLoc.erase(&D);
+}
+
 void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) {
   // `DeclRefExpr`s to builtin function types aren't glvalues, for some reason,
   // but we still want to be able to associate a `StorageLocation` with them,

clang/lib/Analysis/FlowSensitive/Transfer.cpp

@@ -696,19 +696,23 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       FieldLocs.insert({Field, &Loc});
     }
 
-    LLVM_DEBUG({
-      // Check that we satisfy the invariant that a `RecordStorageLoation`
-      // contains exactly the set of modeled fields for that type.
-      // `ModeledFields` includes fields from all the bases, but only the
-      // modeled ones. However, if a class type is initialized with an
-      // `InitListExpr`, all fields in the class, including those from base
-      // classes, are included in the set of modeled fields. The code above
-      // should therefore populate exactly the modeled fields.
-      auto ModeledFields = Env.getDataflowAnalysisContext().getModeledFields(Type);
-      assert(ModeledFields.size() == FieldLocs.size());
+    // Check that we satisfy the invariant that a `RecordStorageLoation`
+    // contains exactly the set of modeled fields for that type.
+    // `ModeledFields` includes fields from all the bases, but only the
+    // modeled ones. However, if a class type is initialized with an
+    // `InitListExpr`, all fields in the class, including those from base
+    // classes, are included in the set of modeled fields. The code above
+    // should therefore populate exactly the modeled fields.
+    assert([&]() {
+      auto ModeledFields =
+          Env.getDataflowAnalysisContext().getModeledFields(Type);
+      if (ModeledFields.size() != FieldLocs.size())
+        return false;
       for ([[maybe_unused]] auto [Field, Loc] : FieldLocs)
-        assert(ModeledFields.contains(cast_or_null<FieldDecl>(Field)));
-    });
+        if (!ModeledFields.contains(cast_or_null<FieldDecl>(Field)))
+          return false;
+      return true;
+    }());
 
     auto &Loc =
         Env.getDataflowAnalysisContext().arena().create<RecordStorageLocation>(

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

@@ -439,19 +439,17 @@ static void builtinTransfer(const CFGElement &Elt,
   case CFGElement::Initializer:
     builtinTransferInitializer(Elt.castAs<CFGInitializer>(), State);
     break;
+  case CFGElement::LifetimeEnds:
+    // Removing declarations when their lifetime ends serves two purposes:
+    // - Eliminate unnecessary clutter from `Environment::DeclToLoc`
+    // - Allow us to assert that, when joining two `Environment`s, the two
+    //   `DeclToLoc` maps never contain entries that map the same declaration to
+    //   different storage locations.
+    if (const ValueDecl *VD = Elt.castAs<CFGLifetimeEnds>().getVarDecl())
+      State.Env.removeDecl(*VD);
+    break;
   default:
-    // FIXME: Evaluate other kinds of `CFGElement`, including:
-    // - When encountering `CFGLifetimeEnds`, remove the declaration from
-    //   `Environment::DeclToLoc`. This would serve two purposes:
-    //   a) Eliminate unnecessary clutter from `Environment::DeclToLoc`
-    //   b) Allow us to implement an assertion that, when joining two
-    //      `Environments`, the two `DeclToLoc` maps never contain entries that
-    //      map the same declaration to different storage locations.
-    //   Unfortunately, however, we can't currently process `CFGLifetimeEnds`
-    //   because the corresponding CFG option `AddLifetime` is incompatible with
-    //   the option 'AddImplicitDtors`, which we already use. We will first
-    //   need to modify the CFG implementation to make these two options
-    //   compatible before we can process `CFGLifetimeEnds`.
+    // FIXME: Evaluate other kinds of `CFGElement`
     break;
   }
 }

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

@@ -2957,10 +2957,7 @@ TEST(TransferTest, VarDeclInDoWhile) {
         const auto *BarVal = cast<IntegerValue>(EnvInLoop.getValue(*BarDecl));
         EXPECT_EQ(BarVal, FooPointeeVal);
 
-        // FIXME: This assertion documents current behavior, but we would prefer
-        // declarations to be removed from the environment when their lifetime
-        // ends. Once this is the case, change this assertion accordingly.
-        ASSERT_THAT(EnvAfterLoop.getValue(*BarDecl), BarVal);
+        ASSERT_THAT(EnvAfterLoop.getValue(*BarDecl), IsNull());
       });
 }
 

clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

@@ -108,7 +108,8 @@ TEST(DataflowAnalysisTest, DiagnoseFunctionDiagnoserCalledOnEachElement) {
   // `diagnoseFunction` provides no guarantees about the order in which elements
   // are visited, so we use `UnorderedElementsAre`.
   EXPECT_THAT_EXPECTED(Result, llvm::HasValue(UnorderedElementsAre(
-                                   "0\n", "int x = 0;\n", "x\n", "++x\n")));
+                                   "0\n", "int x = 0;\n", "x\n", "++x\n",
+                                   " (Lifetime ends)\n")));
 }
 
 struct NonConvergingLattice {

flang/lib/Lower/CallInterface.cpp

@@ -23,22 +23,6 @@
 #include "flang/Semantics/tools.h"
 #include <optional>
 
-//===----------------------------------------------------------------------===//
-// BIND(C) mangling helpers
-//===----------------------------------------------------------------------===//
-
-// Return the binding label (from BIND(C...)) or the mangled name of a symbol.
-static std::string getMangledName(Fortran::lower::AbstractConverter &converter,
-                                  const Fortran::semantics::Symbol &symbol) {
-  const std::string *bindName = symbol.GetBindName();
-  // TODO: update GetBindName so that it does not return a label for internal
-  // procedures.
-  if (bindName && Fortran::semantics::ClassifyProcedure(symbol) ==
-                      Fortran::semantics::ProcedureDefinitionClass::Internal)
-    TODO(converter.getCurrentLocation(), "BIND(C) internal procedures");
-  return bindName ? *bindName : converter.mangleName(symbol);
-}
-
 mlir::Type Fortran::lower::getUntypedBoxProcType(mlir::MLIRContext *context) {
   llvm::SmallVector<mlir::Type> resultTys;
   llvm::SmallVector<mlir::Type> inputTys;
@@ -72,8 +56,10 @@ bool Fortran::lower::CallerInterface::hasAlternateReturns() const {
 
 std::string Fortran::lower::CallerInterface::getMangledName() const {
   const Fortran::evaluate::ProcedureDesignator &proc = procRef.proc();
+  // Return the binding label (from BIND(C...)) or the mangled name of the
+  // symbol.
   if (const Fortran::semantics::Symbol *symbol = proc.GetSymbol())
-    return ::getMangledName(converter, symbol->GetUltimate());
+    return converter.mangleName(symbol->GetUltimate());
   assert(proc.GetSpecificIntrinsic() &&
          "expected intrinsic procedure in designator");
   return proc.GetName();
@@ -420,7 +406,7 @@ bool Fortran::lower::CalleeInterface::hasAlternateReturns() const {
 std::string Fortran::lower::CalleeInterface::getMangledName() const {
   if (funit.isMainProgram())
     return fir::NameUniquer::doProgramEntry().str();
-  return ::getMangledName(converter, funit.getSubprogramSymbol());
+  return converter.mangleName(funit.getSubprogramSymbol());
 }
 
 const Fortran::semantics::Symbol *

flang/lib/Lower/Mangler.cpp

@@ -96,14 +96,6 @@ std::string Fortran::lower::mangle::mangleName(
   if (auto *overrideName = ultimateSymbol.GetBindName())
     return *overrideName;
 
-  // TODO: A procedure that inherits BIND(C) through another interface
-  // (procedure(iface)) should be dealt with in GetBindName() or some wrapper.
-  if (!Fortran::semantics::IsPointer(ultimateSymbol) &&
-      Fortran::semantics::IsBindCProcedure(ultimateSymbol) &&
-      Fortran::semantics::ClassifyProcedure(symbol) !=
-          Fortran::semantics::ProcedureDefinitionClass::Internal)
-    return ultimateSymbol.name().ToString();
-
   llvm::StringRef symbolName = toStringRef(ultimateSymbol.name());
   llvm::SmallVector<llvm::StringRef> modules;
   llvm::SmallVector<llvm::StringRef> procs;

flang/lib/Semantics/resolve-names.cpp

@@ -1739,9 +1739,11 @@ bool AttrsVisitor::SetPassNameOn(Symbol &symbol) {
 }
 
 void AttrsVisitor::SetBindNameOn(Symbol &symbol) {
-  if (!attrs_ || !attrs_->test(Attr::BIND_C)) {
+  if ((!attrs_ || !attrs_->test(Attr::BIND_C)) &&
+      !symbol.attrs().test(Attr::BIND_C)) {
     return;
   }
+
   std::optional<std::string> label{
       evaluate::GetScalarConstantValue<evaluate::Ascii>(bindName_)};
   // 18.9.2(2): discard leading and trailing blanks
@@ -1754,6 +1756,9 @@ void AttrsVisitor::SetBindNameOn(Symbol &symbol) {
     }
     auto last{label->find_last_not_of(" ")};
     label = label->substr(first, last - first + 1);
+  } else if (ClassifyProcedure(symbol) == ProcedureDefinitionClass::Internal) {
+    // BIND(C) does not give an implicit binding label to internal procedures.
+    return;
   } else {
     label = symbol.name().ToString();
   }
@@ -4834,6 +4839,13 @@ Symbol &DeclarationVisitor::DeclareProcEntity(
       } else if (interface->test(Symbol::Flag::Subroutine)) {
         symbol.set(Symbol::Flag::Subroutine);
       }
+      if (IsBindCProcedure(*interface) && !IsPointer(symbol) &&
+          !IsDummy(symbol)) {
+        // Inherit BIND_C attribute from the interface, but not the NAME="..."
+        // if any. This is not clearly described in the standard, but matches
+        // the behavior of other compilers.
+        SetImplicitAttr(symbol, Attr::BIND_C);
+      }
     } else if (auto *type{GetDeclTypeSpec()}) {
       SetType(name, *type);
       symbol.set(Symbol::Flag::Function);

flang/test/Lower/HLFIR/bindc-proc-interface.f90

@@ -0,0 +1,69 @@
+! Test mangling with BIND(C) inherited from procedure interface.
+! RUN: bbc -emit-hlfir -o - %s | FileCheck %s
+
+subroutine test()
+  interface
+    subroutine iface_notbindc()
+    end subroutine
+    subroutine iface_bindc() bind(c)
+    end subroutine
+    subroutine iface_explicit_name() bind(c, name="explicit_name")
+    end subroutine
+    subroutine iface_nobinding() bind(c, name="")
+    end subroutine
+  end interface
+
+  procedure(iface_bindc) :: foo_iface_bindc
+  procedure(iface_explicit_name) :: foo_iface_explicit_name
+  procedure(iface_nobinding) :: foo_iface_nobinding
+
+  procedure(iface_bindc), bind(c) :: extra_bindc_iface_bindc
+  procedure(iface_explicit_name), bind(c) :: extra_bindc_iface_explicit_name
+  procedure(iface_nobinding), bind(c) :: extra_bindc_iface_nobinding
+
+  procedure(iface_bindc),  bind(c, name="bar_iface_bindc_2") :: bar_iface_bindc
+  procedure(iface_explicit_name),  bind(c,name="bar_iface_explicit_name_2") ::  bar_iface_explicit_name
+  procedure(iface_nobinding), bind(c, name="bar_iface_nobinding_2") :: bar_iface_nobinding
+
+  procedure(iface_bindc),  bind(c, name="") :: nobinding_iface_bindc
+  procedure(iface_explicit_name),  bind(c, name="") :: nobinding_iface_explicit_name
+  procedure(iface_nobinding), bind(c, name="") :: nobinding_iface_nobinding
+
+  call iface_notbindc()
+  call iface_bindc()
+  call iface_explicit_name()
+  call iface_nobinding()
+
+  call foo_iface_bindc()
+  call foo_iface_explicit_name()
+  call foo_iface_nobinding()
+
+  call extra_bindc_iface_bindc()
+  call extra_bindc_iface_explicit_name()
+  call extra_bindc_iface_nobinding()
+
+  call bar_iface_bindc()
+  call bar_iface_explicit_name()
+  call bar_iface_nobinding()
+
+  call nobinding_iface_bindc()
+  call nobinding_iface_explicit_name()
+  call nobinding_iface_nobinding()
+
+! CHECK:  fir.call @_QPiface_notbindc()
+! CHECK:  fir.call @iface_bindc()
+! CHECK:  fir.call @explicit_name()
+! CHECK:  fir.call @_QPiface_nobinding()
+! CHECK:  fir.call @foo_iface_bindc()
+! CHECK:  fir.call @foo_iface_explicit_name()
+! CHECK:  fir.call @foo_iface_nobinding()
+! CHECK:  fir.call @extra_bindc_iface_bindc()
+! CHECK:  fir.call @extra_bindc_iface_explicit_name()
+! CHECK:  fir.call @extra_bindc_iface_nobinding()
+! CHECK:  fir.call @bar_iface_bindc_2()
+! CHECK:  fir.call @bar_iface_explicit_name_2()
+! CHECK:  fir.call @bar_iface_nobinding_2()
+! CHECK:  fir.call @_QPnobinding_iface_bindc()
+! CHECK:  fir.call @_QPnobinding_iface_explicit_name()
+! CHECK:  fir.call @_QPnobinding_iface_nobinding()
+end subroutine

flang/test/Lower/HLFIR/bindc_empty_name.f90

@@ -0,0 +1,23 @@
+! Test that lowering makes a difference between NAME="" and no NAME
+! in BIND(C). See Fortran 2018 standard 18.10.2 point 2.
+! BIND(C, NAME="") implies there is no binding label, meaning that
+! the Fortran mangled name has to be used.
+! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
+
+!CHECK: func.func @_QPfoo(%{{.*}}: !fir.ref<i16>
+subroutine foo(x) bind(c, name="")
+  integer(2) :: x
+end subroutine
+
+!CHECK: func.func @bar(%{{.*}}: !fir.ref<i32>
+subroutine foo(x) bind(c, name="bar")
+  integer(4) :: x
+end subroutine
+
+!CHECK: func.func @_QMinamodule1Pfoo(%{{.*}}: !fir.ref<i64>
+module inamodule1
+contains
+subroutine foo(x) bind(c, name="")
+  integer(8) :: x
+end subroutine
+end module

flang/test/Lower/HLFIR/bindc_internal_proc.f90

@@ -0,0 +1,24 @@
+! Test that internal procedure with BIND(C) do not have binding labels,
+! that is, that they are generated using usual flang mangling for non BIND(C)
+! internal procedures.
+! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
+
+!CHECK: func.func @_QFsub1Pfoo(%{{.*}}: i32
+subroutine sub1()
+  call foo(42)
+contains
+  subroutine foo(i) bind(c)
+    integer, value :: i
+    print *, i
+  end subroutine
+end subroutine
+
+!CHECK: func.func @_QFsub2Pfoo(%{{.*}}: i64
+subroutine sub2()
+  call foo(42_8)
+contains
+  subroutine foo(i) bind(c)
+    integer(8), value :: i
+    print *, i
+  end subroutine
+end subroutine

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 475914
+#define LLVM_MAIN_REVISION 475920
 
 /* Define if LLVM_ENABLE_DUMP is enabled */
 #cmakedefine LLVM_ENABLE_DUMP