Skip to content

Commit

Permalink
[flang] Fixed global name creation for literal constants.
Browse files Browse the repository at this point in the history
The global names were created using a hash based on the address
of std::vector::data address. Since the memory may be reused
by different std::vector's, this may cause non-equivalent
constant expressions to map to the same name. This is what is happening
in the modified flang/test/Lower/constant-literal-mangling.f90 test.

I changed the name creation to use a map between the constant expressions
and corresponding unique names. The uniquing is done using a name counter
in FirConverter. The effect of this change is that the equivalent
constant expressions are now mapped to the same global, and the naming
is "stable" (i.e. it does not change from compilation to compilation).

Though, the issue is not HLFIR specific it was affecting several tests
when using HLFIR lowering.

Differential Revision: https://reviews.llvm.org/D150380
  • Loading branch information
vzakhari committed May 12, 2023
1 parent 26a7f42 commit be5747e
Show file tree
Hide file tree
Showing 14 changed files with 781 additions and 617 deletions.
3 changes: 2 additions & 1 deletion flang/include/flang/Evaluate/constant.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ class Constant<Type<TypeCategory::Character, KIND>> : public ConstantBounds {
~Constant();

bool operator==(const Constant &that) const {
return shape() == that.shape() && values_ == that.values_;
return LEN() == that.LEN() && shape() == that.shape() &&
values_ == that.values_;
}
bool empty() const;
std::size_t size() const;
Expand Down
11 changes: 11 additions & 0 deletions flang/include/flang/Lower/AbstractConverter.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,17 @@ class AbstractConverter {
Fortran::semantics::Symbol::Flag flag, bool collectSymbols = true,
bool collectHostAssociatedSymbols = false) = 0;

/// For the given literal constant \p expression, returns a unique name
/// that can be used to create a global object to represent this
/// literal constant. It will return the same name for equivalent
/// literal constant expressions. \p eleTy specifies the data type
/// of the constant elements. For array constants it specifies
/// the array's element type.
virtual llvm::StringRef
getUniqueLitName(mlir::Location loc,
std::unique_ptr<Fortran::lower::SomeExpr> expression,
mlir::Type eleTy) = 0;

//===--------------------------------------------------------------------===//
// Expressions
//===--------------------------------------------------------------------===//
Expand Down
21 changes: 0 additions & 21 deletions flang/include/flang/Lower/IterationSpace.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,30 +37,9 @@ using FrontEndSymbol = const semantics::Symbol *;

class AbstractConverter;

unsigned getHashValue(FrontEndExpr x);
bool isEqual(FrontEndExpr x, FrontEndExpr y);
} // namespace lower
} // namespace Fortran

namespace llvm {
template <>
struct DenseMapInfo<Fortran::lower::FrontEndExpr> {
static inline Fortran::lower::FrontEndExpr getEmptyKey() {
return reinterpret_cast<Fortran::lower::FrontEndExpr>(~0);
}
static inline Fortran::lower::FrontEndExpr getTombstoneKey() {
return reinterpret_cast<Fortran::lower::FrontEndExpr>(~0 - 1);
}
static unsigned getHashValue(Fortran::lower::FrontEndExpr v) {
return Fortran::lower::getHashValue(v);
}
static bool isEqual(Fortran::lower::FrontEndExpr lhs,
Fortran::lower::FrontEndExpr rhs) {
return Fortran::lower::isEqual(lhs, rhs);
}
};
} // namespace llvm

namespace Fortran::lower {

/// Abstraction of the iteration space for building the elemental compute loop
Expand Down
28 changes: 10 additions & 18 deletions flang/include/flang/Lower/Mangler.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ std::string mangleName(const semantics::DerivedTypeSpec &, ScopeBlockIdMap &);
std::string demangleName(llvm::StringRef name);

std::string
mangleArrayLiteral(const uint8_t *addr, size_t size,
mangleArrayLiteral(size_t size,
const Fortran::evaluate::ConstantSubscripts &shape,
Fortran::common::TypeCategory cat, int kind = 0,
Fortran::common::ConstantSubscript charLen = -1,
Expand All @@ -64,35 +64,27 @@ template <Fortran::common::TypeCategory TC, int KIND>
std::string mangleArrayLiteral(
mlir::Type,
const Fortran::evaluate::Constant<Fortran::evaluate::Type<TC, KIND>> &x) {
return mangleArrayLiteral(
reinterpret_cast<const uint8_t *>(x.values().data()),
x.values().size() * sizeof(x.values()[0]), x.shape(), TC, KIND);
return mangleArrayLiteral(x.values().size() * sizeof(x.values()[0]),
x.shape(), TC, KIND);
}

template <int KIND>
std::string
mangleArrayLiteral(mlir::Type,
const Fortran::evaluate::Constant<Fortran::evaluate::Type<
Fortran::common::TypeCategory::Character, KIND>> &x) {
return mangleArrayLiteral(
reinterpret_cast<const uint8_t *>(x.values().data()),
x.values().size() * sizeof(x.values()[0]), x.shape(),
Fortran::common::TypeCategory::Character, KIND, x.LEN());
return mangleArrayLiteral(x.values().size() * sizeof(x.values()[0]),
x.shape(), Fortran::common::TypeCategory::Character,
KIND, x.LEN());
}

// FIXME: derived type mangling is safe but not reproducible between two
// compilation of a same file because `values().data()` is a nontrivial compile
// time data structure containing pointers and vectors. In particular, this
// means that similar structure constructors are not "combined" into the same
// global constant by lowering.
inline std::string mangleArrayLiteral(
mlir::Type eleTy,
const Fortran::evaluate::Constant<Fortran::evaluate::SomeDerived> &x) {
return mangleArrayLiteral(
reinterpret_cast<const uint8_t *>(x.values().data()),
x.values().size() * sizeof(x.values()[0]), x.shape(),
Fortran::common::TypeCategory::Derived, /*kind=*/0, /*charLen=*/-1,
eleTy.cast<fir::RecordType>().getName());
return mangleArrayLiteral(x.values().size() * sizeof(x.values()[0]),
x.shape(), Fortran::common::TypeCategory::Derived,
/*kind=*/0, /*charLen=*/-1,
eleTy.cast<fir::RecordType>().getName());
}

/// Return the compiler-generated name of a static namelist variable descriptor.
Expand Down

0 comments on commit be5747e

Please sign in to comment.