[flang-commits] [flang] [flang][nfc] make size and offset optional so you can check if they have been set (PR #170931)
Andre Kuhlenschmidt via flang-commits
flang-commits at lists.llvm.org
Fri Dec 5 13:57:20 PST 2025
https://github.com/akuhlens created https://github.com/llvm/llvm-project/pull/170931
This PR makes the size field optional in symbols so that you can tell when it has been set. A later PR checks if a size is set before running some routines that will fail if it wasn't reasonable.
>From 7f4038bdfb9a63a0acce99ad248c9303595bb551 Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Tue, 2 Dec 2025 15:27:37 -0800
Subject: [PATCH 1/3] make size and align optional
---
flang/include/flang/Semantics/symbol.h | 22 ++++++++++++++++++----
flang/lib/Semantics/compute-offsets.cpp | 13 +++++++++++--
flang/lib/Semantics/data-to-inits.cpp | 2 +-
flang/lib/Semantics/symbol.cpp | 4 ++--
4 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index 95efe1ae2bd5e..92fbf64763e93 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -874,10 +874,24 @@ class Symbol {
Scope *scope() { return scope_; }
const Scope *scope() const { return scope_; }
void set_scope(Scope *scope) { scope_ = scope; }
- std::size_t size() const { return size_; }
+ std::optional<std::size_t> sizeOpt() const { return size_; }
+ std::size_t size() const {
+ // NOTE: I tried to insert the following check, but there are places where we are
+ // relying on the fact that the size is 0 if it is not set.
+ // CHECK(size_.has_value());
+ return size_.value_or(0);
+ }
void set_size(std::size_t size) { size_ = size; }
- std::size_t offset() const { return offset_; }
+ void set_size(std::optional<std::size_t> size) { size_ = size; }
+ std::optional<std::size_t> offsetOpt() const { return offset_; }
+ std::size_t offset() const {
+ // NOTE: I tried to insert the following check, but there are places where we are
+ // relying on the fact that the offset is 0 if it is not set.
+ // CHECK(offset_.has_value());
+ return offset_.value_or(0);
+ }
void set_offset(std::size_t offset) { offset_ = offset; }
+ void set_offset(std::optional<std::size_t> offset) { offset_ = offset; }
// Give the symbol a name with a different source location but same chars.
void ReplaceName(const SourceName &);
static std::string OmpFlagToClauseName(Flag ompFlag);
@@ -986,8 +1000,8 @@ class Symbol {
Attrs implicitAttrs_; // subset of attrs_ that were not explicit
Flags flags_;
Scope *scope_{nullptr};
- std::size_t size_{0}; // size in bytes
- std::size_t offset_{0}; // byte offset in scope or common block
+ std::optional<std::size_t> size_; // size in bytes
+ std::optional<std::size_t> offset_; // byte offset in scope or common block
Details details_;
Symbol() {} // only created in class Symbols
diff --git a/flang/lib/Semantics/compute-offsets.cpp b/flang/lib/Semantics/compute-offsets.cpp
index 1c48d33549a2e..305d6dd143e7f 100644
--- a/flang/lib/Semantics/compute-offsets.cpp
+++ b/flang/lib/Semantics/compute-offsets.cpp
@@ -152,7 +152,8 @@ void ComputeOffsetsHelper::Compute(Scope &scope) {
// disjoint EQUIVALENCE storage sequence.
for (auto &[symbol, dep] : dependents_) {
dep = Resolve(dep);
- CHECK(symbol->size() == 0);
+ //CHECK(symbol->size() == 0);
+ CHECK(!symbol->sizeOpt().has_value());
auto symInfo{GetSizeAndAlignment(*symbol, true)};
symbol->set_size(symInfo.size);
Symbol &base{*dep.symbol};
@@ -211,6 +212,10 @@ void ComputeOffsetsHelper::Compute(Scope &scope) {
}
}
for (auto &[symbol, dep] : dependents_) {
+ if (!dep.symbol->offsetOpt().has_value()) {
+ llvm::errs() << "Symbol " << dep.symbol->name() << " has no offset\n";
+ }
+ CHECK(dep.symbol->offsetOpt().has_value());
symbol->set_offset(dep.symbol->offset() + dep.offset);
if (const auto *block{FindCommonBlockContaining(*dep.symbol)}) {
symbol->get<ObjectEntityDetails>().set_commonBlock(*block);
@@ -281,6 +286,7 @@ void ComputeOffsetsHelper::DoCommonBlock(Symbol &commonBlock) {
} else {
eqIter = equivalenceBlock_.find(base);
base.get<ObjectEntityDetails>().set_commonBlock(commonBlock);
+ CHECK(symbol.offsetOpt().has_value());
base.set_offset(symbol.offset() - dep.offset);
previous.emplace(base);
}
@@ -301,7 +307,7 @@ void ComputeOffsetsHelper::DoCommonBlock(Symbol &commonBlock) {
void ComputeOffsetsHelper::DoEquivalenceBlockBase(
Symbol &symbol, SizeAndAlignment &blockInfo) {
- if (symbol.size() > blockInfo.size) {
+ if (symbol.sizeOpt().has_value() && symbol.size() > blockInfo.size) {
blockInfo.size = symbol.size();
}
}
@@ -393,11 +399,14 @@ std::size_t ComputeOffsetsHelper::ComputeOffset(
std::size_t ComputeOffsetsHelper::DoSymbol(
Symbol &symbol, std::optional<const size_t> newAlign) {
+ // Symbols such as the main program and modules.
if (!symbol.has<ObjectEntityDetails>() && !symbol.has<ProcEntityDetails>()) {
return 0;
}
SizeAndAlignment s{GetSizeAndAlignment(symbol, true)};
if (s.size == 0) {
+ symbol.set_size(0);
+ symbol.set_offset(offset_);
return 0;
}
std::size_t previousOffset{offset_};
diff --git a/flang/lib/Semantics/data-to-inits.cpp b/flang/lib/Semantics/data-to-inits.cpp
index bbf3b28fe03e6..831d2cc77eb81 100644
--- a/flang/lib/Semantics/data-to-inits.cpp
+++ b/flang/lib/Semantics/data-to-inits.cpp
@@ -786,7 +786,7 @@ static bool CombineEquivalencedInitialization(
combinedSymbol.set(Symbol::Flag::CompilerCreated);
inits.emplace(&combinedSymbol, std::move(combined));
auto &details{combinedSymbol.get<ObjectEntityDetails>()};
- combinedSymbol.set_offset(first.offset());
+ combinedSymbol.set_offset(first.offsetOpt());
combinedSymbol.set_size(bytes);
std::size_t minElementBytes{
ComputeMinElementBytes(associated, foldingContext)};
diff --git a/flang/lib/Semantics/symbol.cpp b/flang/lib/Semantics/symbol.cpp
index ed0715a422e78..8289e5afa48f7 100644
--- a/flang/lib/Semantics/symbol.cpp
+++ b/flang/lib/Semantics/symbol.cpp
@@ -723,8 +723,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const Symbol &symbol) {
if (!symbol.flags().empty()) {
os << " (" << symbol.flags() << ')';
}
- if (symbol.size_) {
- os << " size=" << symbol.size_ << " offset=" << symbol.offset_;
+ if (symbol.size_ && *symbol.size_) {
+ os << " size=" << symbol.size_ << " offset=" << symbol.offset();
}
os << ": " << symbol.details_;
return os;
>From 1de746785b5f3231d6978363b8b1dbe239c85419 Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Tue, 2 Dec 2025 16:18:09 -0800
Subject: [PATCH 2/3] remove functional changes
---
flang/lib/Semantics/compute-offsets.cpp | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/flang/lib/Semantics/compute-offsets.cpp b/flang/lib/Semantics/compute-offsets.cpp
index 305d6dd143e7f..1e557121de434 100644
--- a/flang/lib/Semantics/compute-offsets.cpp
+++ b/flang/lib/Semantics/compute-offsets.cpp
@@ -152,8 +152,7 @@ void ComputeOffsetsHelper::Compute(Scope &scope) {
// disjoint EQUIVALENCE storage sequence.
for (auto &[symbol, dep] : dependents_) {
dep = Resolve(dep);
- //CHECK(symbol->size() == 0);
- CHECK(!symbol->sizeOpt().has_value());
+ CHECK(symbol->size() == 0);
auto symInfo{GetSizeAndAlignment(*symbol, true)};
symbol->set_size(symInfo.size);
Symbol &base{*dep.symbol};
@@ -212,10 +211,8 @@ void ComputeOffsetsHelper::Compute(Scope &scope) {
}
}
for (auto &[symbol, dep] : dependents_) {
- if (!dep.symbol->offsetOpt().has_value()) {
- llvm::errs() << "Symbol " << dep.symbol->name() << " has no offset\n";
- }
- CHECK(dep.symbol->offsetOpt().has_value());
+ // We are occassionally relying offset here to return zero when
+ // it has not been set yet.
symbol->set_offset(dep.symbol->offset() + dep.offset);
if (const auto *block{FindCommonBlockContaining(*dep.symbol)}) {
symbol->get<ObjectEntityDetails>().set_commonBlock(*block);
@@ -307,7 +304,7 @@ void ComputeOffsetsHelper::DoCommonBlock(Symbol &commonBlock) {
void ComputeOffsetsHelper::DoEquivalenceBlockBase(
Symbol &symbol, SizeAndAlignment &blockInfo) {
- if (symbol.sizeOpt().has_value() && symbol.size() > blockInfo.size) {
+ if (symbol.size() > blockInfo.size) {
blockInfo.size = symbol.size();
}
}
@@ -405,8 +402,8 @@ std::size_t ComputeOffsetsHelper::DoSymbol(
}
SizeAndAlignment s{GetSizeAndAlignment(symbol, true)};
if (s.size == 0) {
- symbol.set_size(0);
- symbol.set_offset(offset_);
+ // FIXME: Errors that prevent us from computing the symbols size or
+ // variables that really require no storage should be set here.
return 0;
}
std::size_t previousOffset{offset_};
>From 7d07f66e98a9625bad42ebee49180cd1da09904f Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Fri, 5 Dec 2025 13:52:42 -0800
Subject: [PATCH 3/3] reformat
---
flang/include/flang/Semantics/symbol.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index 92fbf64763e93..b2bfc6102349a 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -875,20 +875,20 @@ class Symbol {
const Scope *scope() const { return scope_; }
void set_scope(Scope *scope) { scope_ = scope; }
std::optional<std::size_t> sizeOpt() const { return size_; }
- std::size_t size() const {
- // NOTE: I tried to insert the following check, but there are places where we are
- // relying on the fact that the size is 0 if it is not set.
+ std::size_t size() const {
+ // NOTE: I tried to insert the following check, but there are places where
+ // we are relying on the fact that the size is 0 if it is not set.
// CHECK(size_.has_value());
- return size_.value_or(0);
+ return size_.value_or(0);
}
void set_size(std::size_t size) { size_ = size; }
void set_size(std::optional<std::size_t> size) { size_ = size; }
std::optional<std::size_t> offsetOpt() const { return offset_; }
- std::size_t offset() const {
- // NOTE: I tried to insert the following check, but there are places where we are
- // relying on the fact that the offset is 0 if it is not set.
+ std::size_t offset() const {
+ // NOTE: I tried to insert the following check, but there are places where
+ // we are relying on the fact that the offset is 0 if it is not set.
// CHECK(offset_.has_value());
- return offset_.value_or(0);
+ return offset_.value_or(0);
}
void set_offset(std::size_t offset) { offset_ = offset; }
void set_offset(std::optional<std::size_t> offset) { offset_ = offset; }
More information about the flang-commits
mailing list