Skip to content

Commit

Permalink
Merged main:8fb28e45ce04 into amd-gfx:345ce9d5346e
Browse files Browse the repository at this point in the history 
Local branch amd-gfx 345ce9d Merged main:7fcbb64fca5e into amd-gfx:b56d6b265611
Remote branch main 8fb28e4 [BOLT] Fix data race in MCPlusBuilder::getOrCreateAnnotationIndex (llvm#67004)
  • Loading branch information
SC llvm team authored and SC llvm team committed Sep 21, 2023
2 parents 345ce9d + 8fb28e4 commit a2c094c
Show file tree
Hide file tree
Showing 93 changed files with 2,544 additions and 1,139 deletions.
12 changes: 9 additions & 3 deletions bolt/include/bolt/Core/MCPlusBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "llvm/Support/Allocator.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/RWMutex.h"
#include <cassert>
#include <cstdint>
#include <map>
Expand Down Expand Up @@ -166,6 +167,10 @@ class MCPlusBuilder {
/// Names of non-standard annotations.
SmallVector<std::string, 8> AnnotationNames;

/// A mutex that is used to control parallel accesses to
/// AnnotationNameIndexMap and AnnotationsNames.
mutable llvm::sys::RWMutex AnnotationNameMutex;

/// Allocate the TailCall annotation value. Clients of the target-specific
/// MCPlusBuilder classes must use convert/lower/create* interfaces instead.
void setTailCall(MCInst &Inst);
Expand Down Expand Up @@ -1775,6 +1780,7 @@ class MCPlusBuilder {

/// Return annotation index matching the \p Name.
std::optional<unsigned> getAnnotationIndex(StringRef Name) const {
std::shared_lock<llvm::sys::RWMutex> Lock(AnnotationNameMutex);
auto AI = AnnotationNameIndexMap.find(Name);
if (AI != AnnotationNameIndexMap.end())
return AI->second;
Expand All @@ -1784,10 +1790,10 @@ class MCPlusBuilder {
/// Return annotation index matching the \p Name. Create a new index if the
/// \p Name wasn't registered previously.
unsigned getOrCreateAnnotationIndex(StringRef Name) {
auto AI = AnnotationNameIndexMap.find(Name);
if (AI != AnnotationNameIndexMap.end())
return AI->second;
if (std::optional<unsigned> Index = getAnnotationIndex(Name))
return *Index;

std::unique_lock<llvm::sys::RWMutex> Lock(AnnotationNameMutex);
const unsigned Index =
AnnotationNameIndexMap.size() + MCPlus::MCAnnotation::kGeneric;
AnnotationNameIndexMap.insert(std::make_pair(Name, Index));
Expand Down
5 changes: 5 additions & 0 deletions flang/include/flang/Lower/Allocatable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ void genDeallocateBox(AbstractConverter &converter,
const fir::MutableBoxValue &box, mlir::Location loc,
mlir::Value declaredTypeDesc = {});

/// Deallocate an allocatable if it is allocated at the end of its lifetime.
void genDeallocateIfAllocated(AbstractConverter &converter,
const fir::MutableBoxValue &box,
mlir::Location loc);

/// Create a MutableBoxValue for an allocatable or pointer entity.
/// If the variables is a local variable that is not a dummy, it will be
/// initialized to unallocated/diassociated status.
Expand Down
15 changes: 9 additions & 6 deletions flang/include/flang/Optimizer/Builder/MutableBox.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,22 @@ void finalizeRealloc(fir::FirOpBuilder &builder, mlir::Location loc,
bool takeLboundsIfRealloc,
const MutableBoxReallocation &realloc);

/// Finalize a mutable box if it is allocated or associated. This includes both
/// calling the finalizer, if any, and deallocating the storage.
void genFinalization(fir::FirOpBuilder &builder, mlir::Location loc,
const fir::MutableBoxValue &box);
/// Deallocate a mutable box with fir.freemem if it is allocated or associated.
/// This only deallocates the storage and does not call finalization, the
/// mutable box is not nullified.
void genFreememIfAllocated(fir::FirOpBuilder &builder, mlir::Location loc,
const fir::MutableBoxValue &box);

void genInlinedAllocation(fir::FirOpBuilder &builder, mlir::Location loc,
const fir::MutableBoxValue &box,
mlir::ValueRange lbounds, mlir::ValueRange extents,
mlir::ValueRange lenParams, llvm::StringRef allocName,
bool mustBeHeap = false);

mlir::Value genInlinedDeallocate(fir::FirOpBuilder &builder, mlir::Location loc,
const fir::MutableBoxValue &box);
/// Deallocate an mutable box storage with fir.freemem without calling any
/// final procedures. The mutable box is not nullified.
mlir::Value genFreemem(fir::FirOpBuilder &builder, mlir::Location loc,
const fir::MutableBoxValue &box);

/// When the MutableBoxValue was passed as a fir.ref<fir.box> to a call that may
/// have modified it, update the MutableBoxValue according to the
Expand Down
35 changes: 28 additions & 7 deletions flang/lib/Lower/Allocatable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ static mlir::Value genDeallocate(fir::FirOpBuilder &builder, mlir::Location loc,
if (!box.isDerived() && !box.isPolymorphic() &&
!box.isUnlimitedPolymorphic() && !errorManager.hasStatSpec() &&
!useAllocateRuntime) {
return fir::factory::genInlinedDeallocate(builder, loc, box);
return fir::factory::genFreemem(builder, loc, box);
}
// Use runtime calls to deallocate descriptor cases. Sync MutableBoxValue
// with its descriptor before and after calls if needed.
Expand All @@ -770,6 +770,26 @@ void Fortran::lower::genDeallocateBox(
genDeallocate(builder, loc, box, errorManager, declaredTypeDesc);
}

void Fortran::lower::genDeallocateIfAllocated(
Fortran::lower::AbstractConverter &converter,
const fir::MutableBoxValue &box, mlir::Location loc) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
mlir::Value isAllocated =
fir::factory::genIsAllocatedOrAssociatedTest(builder, loc, box);
builder.genIfThen(loc, isAllocated)
.genThen([&]() {
if (mlir::Type eleType = box.getEleTy();
eleType.isa<fir::RecordType>() && box.isPolymorphic()) {
mlir::Value declaredTypeDesc = builder.create<fir::TypeDescOp>(
loc, mlir::TypeAttr::get(eleType));
genDeallocateBox(converter, box, loc, declaredTypeDesc);
} else {
genDeallocateBox(converter, box, loc);
}
})
.end();
}

static void preDeallocationAction(Fortran::lower::AbstractConverter &converter,
fir::FirOpBuilder &builder,
mlir::Value beginOpValue,
Expand Down Expand Up @@ -813,12 +833,13 @@ void Fortran::lower::genDeallocateStmt(
genMutableBoxValue(converter, loc, allocateObject);
mlir::Value declaredTypeDesc = {};
if (box.isPolymorphic()) {
assert(symbol.GetType());
if (const Fortran::semantics::DerivedTypeSpec *derivedTypeSpec =
symbol.GetType()->AsDerived()) {
declaredTypeDesc =
Fortran::lower::getTypeDescAddr(converter, loc, *derivedTypeSpec);
}
mlir::Type eleType = box.getEleTy();
if (eleType.isa<fir::RecordType>())
if (const Fortran::semantics::DerivedTypeSpec *derivedTypeSpec =
symbol.GetType()->AsDerived()) {
declaredTypeDesc =
Fortran::lower::getTypeDescAddr(converter, loc, *derivedTypeSpec);
}
}
mlir::Value beginOpValue =
genDeallocate(builder, loc, box, errorManager, declaredTypeDesc);
Expand Down
9 changes: 1 addition & 8 deletions flang/lib/Lower/Bridge.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,14 +706,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
return;
}
// deallocate allocated in createHostAssociateVarClone value
mlir::Value needs_dealloc =
fir::factory::genIsAllocatedOrAssociatedTest(*builder, loc,
new_box);
builder->genIfThen(loc, needs_dealloc)
.genThen([&]() {
Fortran::lower::genDeallocateBox(*this, new_box, loc);
})
.end();
Fortran::lower::genDeallocateIfAllocated(*this, new_box, loc);
},
[&](const auto &) -> void {
// Do nothing
Expand Down
7 changes: 5 additions & 2 deletions flang/lib/Lower/ConvertCall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,13 @@ fir::ExtendedValue Fortran::lower::genCallOpAndResult(
allocatedResult->match(
[&](const fir::MutableBoxValue &box) {
if (box.isAllocatable() && !cleanupWithDestroy) {
// 9.7.3.2 point 4. Finalize allocatables.
// 9.7.3.2 point 4. Deallocate allocatable results. Note that
// finalization was done independently by calling
// genDerivedTypeDestroy above and is not triggered by this inline
// deallocation.
fir::FirOpBuilder *bldr = &converter.getFirOpBuilder();
stmtCtx.attachCleanup([bldr, loc, box]() {
fir::factory::genFinalization(*bldr, loc, box);
fir::factory::genFreememIfAllocated(*bldr, loc, box);
});
}
},
Expand Down
18 changes: 3 additions & 15 deletions flang/lib/Lower/ConvertExpr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2646,26 +2646,14 @@ class ScalarExprLowering {
}
// Passing a POINTER to a POINTER, or an ALLOCATABLE to an ALLOCATABLE.
fir::MutableBoxValue mutableBox = genMutableBoxValue(*expr);
if (fir::isAllocatableType(argTy) && arg.isIntentOut() &&
Fortran::semantics::IsBindCProcedure(*procRef.proc().GetSymbol()))
Fortran::lower::genDeallocateIfAllocated(converter, mutableBox, loc);
mlir::Value irBox =
fir::factory::getMutableIRBox(builder, loc, mutableBox);
caller.placeInput(arg, irBox);
if (arg.mayBeModifiedByCall())
mutableModifiedByCall.emplace_back(std::move(mutableBox));
if (fir::isAllocatableType(argTy) && arg.isIntentOut() &&
Fortran::semantics::IsBindCProcedure(*procRef.proc().GetSymbol())) {
if (mutableBox.isDerived() || mutableBox.isPolymorphic() ||
mutableBox.isUnlimitedPolymorphic()) {
mlir::Value isAlloc = fir::factory::genIsAllocatedOrAssociatedTest(
builder, loc, mutableBox);
builder.genIfThen(loc, isAlloc)
.genThen([&]() {
Fortran::lower::genDeallocateBox(converter, mutableBox, loc);
})
.end();
} else {
Fortran::lower::genDeallocateBox(converter, mutableBox, loc);
}
}
continue;
}
if (arg.passBy == PassBy::BaseAddress || arg.passBy == PassBy::BoxChar ||
Expand Down
31 changes: 4 additions & 27 deletions flang/lib/Lower/ConvertVariable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,40 +749,17 @@ static void deallocateIntentOut(Fortran::lower::AbstractConverter &converter,
}
mlir::Location loc = converter.getCurrentLocation();
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
auto genDeallocateWithTypeDesc = [&]() {
if (mutBox->isDerived() || mutBox->isPolymorphic() ||
mutBox->isUnlimitedPolymorphic()) {
mlir::Value isAlloc = fir::factory::genIsAllocatedOrAssociatedTest(
builder, loc, *mutBox);
builder.genIfThen(loc, isAlloc)
.genThen([&]() {
if (mutBox->isPolymorphic()) {
mlir::Value declaredTypeDesc;
assert(sym.GetType());
if (const Fortran::semantics::DerivedTypeSpec
*derivedTypeSpec = sym.GetType()->AsDerived()) {
declaredTypeDesc = Fortran::lower::getTypeDescAddr(
converter, loc, *derivedTypeSpec);
}
genDeallocateBox(converter, *mutBox, loc, declaredTypeDesc);
} else {
genDeallocateBox(converter, *mutBox, loc);
}
})
.end();
} else {
genDeallocateBox(converter, *mutBox, loc);
}
};

if (Fortran::semantics::IsOptional(sym)) {
auto isPresent = builder.create<fir::IsPresentOp>(
loc, builder.getI1Type(), fir::getBase(extVal));
builder.genIfThen(loc, isPresent)
.genThen([&]() { genDeallocateWithTypeDesc(); })
.genThen([&]() {
Fortran::lower::genDeallocateIfAllocated(converter, *mutBox, loc);
})
.end();
} else {
genDeallocateWithTypeDesc();
Fortran::lower::genDeallocateIfAllocated(converter, *mutBox, loc);
}
}
}
Expand Down
28 changes: 12 additions & 16 deletions flang/lib/Optimizer/Builder/MutableBox.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,29 +454,27 @@ mlir::Value fir::factory::genIsNotAllocatedOrAssociatedTest(
return builder.genIsNullAddr(loc, addr);
}

/// Generate finalizer call and inlined free. This does not check that the
/// Call freemem. This does not check that the
/// address was allocated.
static void genFinalizeAndFree(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value addr) {
// TODO: call finalizer if any.

static void genFreemem(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value addr) {
// A heap (ALLOCATABLE) object may have been converted to a ptr (POINTER),
// so make sure the heap type is restored before deallocation.
auto cast = builder.createConvert(
loc, fir::HeapType::get(fir::dyn_cast_ptrEleTy(addr.getType())), addr);
builder.create<fir::FreeMemOp>(loc, cast);
}

void fir::factory::genFinalization(fir::FirOpBuilder &builder,
mlir::Location loc,
const fir::MutableBoxValue &box) {
void fir::factory::genFreememIfAllocated(fir::FirOpBuilder &builder,
mlir::Location loc,
const fir::MutableBoxValue &box) {
auto addr = MutablePropertyReader(builder, loc, box).readBaseAddress();
auto isAllocated = builder.genIsNotNullAddr(loc, addr);
auto ifOp = builder.create<fir::IfOp>(loc, isAllocated,
/*withElseRegion=*/false);
auto insPt = builder.saveInsertionPoint();
builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
genFinalizeAndFree(builder, loc, addr);
::genFreemem(builder, loc, addr);
builder.restoreInsertionPoint(insPt);
}

Expand Down Expand Up @@ -753,12 +751,11 @@ void fir::factory::genInlinedAllocation(
fir::MustBeHeapAttr::get(builder.getContext(), mustBeHeap));
}

mlir::Value
fir::factory::genInlinedDeallocate(fir::FirOpBuilder &builder,
mlir::Location loc,
const fir::MutableBoxValue &box) {
mlir::Value fir::factory::genFreemem(fir::FirOpBuilder &builder,
mlir::Location loc,
const fir::MutableBoxValue &box) {
auto addr = MutablePropertyReader(builder, loc, box).readBaseAddress();
genFinalizeAndFree(builder, loc, addr);
::genFreemem(builder, loc, addr);
MutablePropertyWriter{builder, loc, box}.setUnallocatedStatus();
return addr;
}
Expand Down Expand Up @@ -909,8 +906,7 @@ void fir::factory::finalizeRealloc(fir::FirOpBuilder &builder,
auto heap = fir::getBase(realloc.newValue);
auto extents = fir::factory::getExtents(loc, builder, realloc.newValue);
builder.genIfThen(loc, realloc.oldAddressWasAllocated)
.genThen(
[&]() { genFinalizeAndFree(builder, loc, realloc.oldAddress); })
.genThen([&]() { ::genFreemem(builder, loc, realloc.oldAddress); })
.end();
MutablePropertyWriter{builder, loc, box}.updateMutableBox(
heap, lbs, extents, lengths);
Expand Down
28 changes: 14 additions & 14 deletions flang/test/Lower/Intrinsics/system_clock.f90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ subroutine ss(count)
! CHECK: fir.store %[[V_29]] to %arg0 : !fir.ref<i64>
! CHECK: }
! CHECK: %[[V_12:[0-9]+]] = fir.convert %[[V_9]] : (!fir.ptr<i64>) -> i64
! CHECK: %[[V_13:[0-9]+]] = arith.cmpi ne, %[[V_12]], %c0{{.*}}_i64 : i64
! CHECK: %[[V_13:[0-9]+]] = arith.cmpi ne, %[[V_12]], %c0{{.*}} : i64
! CHECK: fir.if %[[V_13]] {
! CHECK: %[[V_29]] = fir.call @_FortranASystemClockCountRate(%c8{{.*}}_i32) {{.*}}: (i32) -> i64
! CHECK: %[[V_29:[0-9]+]] = fir.call @_FortranASystemClockCountRate(%c8{{.*}}_i32) {{.*}}: (i32) -> i64
! CHECK: fir.store %[[V_29]] to %[[V_9]] : !fir.ptr<i64>
! CHECK: }
! CHECK: %[[V_14:[0-9]+]] = fir.convert %[[V_10]] : (!fir.heap<i64>) -> i64
! CHECK: %[[V_15:[0-9]+]] = arith.cmpi ne, %[[V_14]], %c0{{.*}}_i64_0 : i64
! CHECK: %[[V_15:[0-9]+]] = arith.cmpi ne, %[[V_14]], %c0{{.*}} : i64
! CHECK: fir.if %[[V_15]] {
! CHECK: %[[V_29]] = fir.call @_FortranASystemClockCountMax(%c8{{.*}}_i32) {{.*}}: (i32) -> i64
! CHECK: fir.store %[[V_29]] to %[[V_10]] : !fir.heap<i64>
Expand All @@ -77,24 +77,24 @@ subroutine ss(count)
! CHECK: %[[V_39:[0-9]+]] = fir.call @_FortranAioOutputInteger64(%[[V_31]], %[[V_38]]) {{.*}}: (!fir.ref<i8>, i64) -> i1
! CHECK: %[[V_40:[0-9]+]] = fir.call @_FortranAioEndIoStatement(%[[V_31]]) {{.*}}: (!fir.ref<i8>) -> i32
! CHECK: } else {
! CHECK: %[[V_29]] = fir.load %[[V_4]] : !fir.ref<!fir.ptr<i64>>
! CHECK: %[[V_29:[0-9]+]] = fir.load %[[V_4]] : !fir.ref<!fir.ptr<i64>>
! CHECK: %[[V_30:[0-9]+]] = fir.load %[[V_1]] : !fir.ref<!fir.heap<i64>>
! CHECK: %[[V_31]] = fir.convert %[[V_29]] : (!fir.ptr<i64>) -> i64
! CHECK: %[[V_32]] = arith.cmpi ne, %[[V_31]], %c0{{.*}}_i64_3 : i64
! CHECK: %[[V_31:[0-9]+]] = fir.convert %[[V_29]] : (!fir.ptr<i64>) -> i64
! CHECK: %[[V_32:[0-9]+]] = arith.cmpi ne, %[[V_31]], %c0{{.*}} : i64
! CHECK: fir.if %[[V_32]] {
! CHECK: %[[V_45:[0-9]+]] = fir.call @_FortranASystemClockCountRate(%c8{{.*}}_i32) {{.*}}: (i32) -> i64
! CHECK: fir.store %[[V_45]] to %[[V_29]] : !fir.ptr<i64>
! CHECK: }
! CHECK: %[[V_33]] = fir.convert %[[V_30]] : (!fir.heap<i64>) -> i64
! CHECK: %[[V_34]] = arith.cmpi ne, %[[V_33]], %c0{{.*}}_i64_4 : i64
! CHECK: %[[V_33:[0-9]+]] = fir.convert %[[V_30]] : (!fir.heap<i64>) -> i64
! CHECK: %[[V_34:[0-9]+]] = arith.cmpi ne, %[[V_33]], %c0{{.*}} : i64
! CHECK: fir.if %[[V_34]] {
! CHECK: %[[V_45]] = fir.call @_FortranASystemClockCountMax(%c8{{.*}}_i32) {{.*}}: (i32) -> i64
! CHECK: fir.store %[[V_45]] to %[[V_30]] : !fir.heap<i64>
! CHECK: }
! CHECK: %[[V_37]] = fir.call @_FortranAioBeginExternalListOutput
! CHECK: %[[V_38]] = fir.load %[[V_4]] : !fir.ref<!fir.ptr<i64>>
! CHECK: %[[V_39]] = fir.load %[[V_38]] : !fir.ptr<i64>
! CHECK: %[[V_40]] = fir.call @_FortranAioOutputInteger64(%[[V_37]], %[[V_39]]) {{.*}}: (!fir.ref<i8>, i64) -> i1
! CHECK: %[[V_37:[0-9]+]] = fir.call @_FortranAioBeginExternalListOutput
! CHECK: %[[V_38:[0-9]+]] = fir.load %[[V_4]] : !fir.ref<!fir.ptr<i64>>
! CHECK: %[[V_39:[0-9]+]] = fir.load %[[V_38]] : !fir.ptr<i64>
! CHECK: %[[V_40:[0-9]+]] = fir.call @_FortranAioOutputInteger64(%[[V_37]], %[[V_39]]) {{.*}}: (!fir.ref<i8>, i64) -> i1
! CHECK: %[[V_41:[0-9]+]] = fir.load %[[V_1]] : !fir.ref<!fir.heap<i64>>
! CHECK: %[[V_42:[0-9]+]] = fir.load %[[V_41]] : !fir.heap<i64>
! CHECK: %[[V_43:[0-9]+]] = fir.call @_FortranAioOutputInteger64(%[[V_37]], %[[V_42]]) {{.*}}: (!fir.ref<i8>, i64) -> i1
Expand All @@ -120,13 +120,13 @@ subroutine ss(count)
! CHECK: fir.store %[[V_29]] to %arg0 : !fir.ref<i64>
! CHECK: }
! CHECK: %[[V_24:[0-9]+]] = fir.convert %[[V_21]] : (!fir.ptr<i64>) -> i64
! CHECK: %[[V_25:[0-9]+]] = arith.cmpi ne, %[[V_24]], %c0{{.*}}_i64_1 : i64
! CHECK: %[[V_25:[0-9]+]] = arith.cmpi ne, %[[V_24]], %c0{{.*}} : i64
! CHECK: fir.if %[[V_25]] {
! CHECK: %[[V_29]] = fir.call @_FortranASystemClockCountRate(%c8{{.*}}_i32) {{.*}}: (i32) -> i64
! CHECK: fir.store %[[V_29]] to %[[V_21]] : !fir.ptr<i64>
! CHECK: }
! CHECK: %[[V_26:[0-9]+]] = fir.convert %[[V_22]] : (!fir.heap<i64>) -> i64
! CHECK: %[[V_27:[0-9]+]] = arith.cmpi ne, %[[V_26]], %c0{{.*}}_i64_2 : i64
! CHECK: %[[V_27:[0-9]+]] = arith.cmpi ne, %[[V_26]], %c0{{.*}} : i64
! CHECK: fir.if %[[V_27]] {
! CHECK: %[[V_29]] = fir.call @_FortranASystemClockCountMax(%c8{{.*}}_i32) {{.*}}: (i32) -> i64
! CHECK: fir.store %[[V_29]] to %[[V_22]] : !fir.heap<i64>
Expand Down
Loading

0 comments on commit a2c094c

Please sign in to comment.