[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
Jonas Paulsson via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 27 07:50:52 PDT 2024
https://github.com/JonPsson1 updated https://github.com/llvm/llvm-project/pull/86691
>From ea3c2d02073ecd3761b53783ad78c132ba88486e Mon Sep 17 00:00:00 2001
From: Jonas Paulsson <paulson1 at linux.ibm.com>
Date: Tue, 26 Mar 2024 01:16:25 +0100
Subject: [PATCH 1/2] Refactoring with shouldCastToInt(). Also include atomic
pointers.
---
clang/lib/CodeGen/CGAtomic.cpp | 80 +++++++++++++++++-----------------
clang/test/CodeGen/atomic.c | 9 ++--
2 files changed, 44 insertions(+), 45 deletions(-)
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index fb03d013e8afc7..f543529efe5b69 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -197,11 +197,11 @@ namespace {
llvm::Value *getScalarRValValueOrNull(RValue RVal) const;
/// Converts an rvalue to integer value if needed.
- llvm::Value *convertRValueToInt(RValue RVal, bool CastFP = true) const;
+ llvm::Value *convertRValueToInt(RValue RVal, bool CmpXchg = false) const;
RValue ConvertToValueOrAtomic(llvm::Value *IntVal, AggValueSlot ResultSlot,
SourceLocation Loc, bool AsValue,
- bool CastFP = true) const;
+ bool CmpXchg = false) const;
/// Copy an atomic r-value into atomic-layout memory.
void emitCopyIntoMemory(RValue rvalue) const;
@@ -264,7 +264,7 @@ namespace {
llvm::AtomicOrdering AO, bool IsVolatile);
/// Emits atomic load as LLVM instruction.
llvm::Value *EmitAtomicLoadOp(llvm::AtomicOrdering AO, bool IsVolatile,
- bool CastFP = true);
+ bool CmpXchg = false);
/// Emits atomic compare-and-exchange op as a libcall.
llvm::Value *EmitAtomicCompareExchangeLibcall(
llvm::Value *ExpectedAddr, llvm::Value *DesiredAddr,
@@ -272,7 +272,9 @@ namespace {
llvm::AtomicOrdering::SequentiallyConsistent,
llvm::AtomicOrdering Failure =
llvm::AtomicOrdering::SequentiallyConsistent);
- /// Emits atomic compare-and-exchange op as LLVM instruction.
+ /// Emits atomic compare-and-exchange op as LLVM instruction. Operands
+ /// must be of integer or pointer type, so float must be casted.
+ /// TODO: this could change - see comment in AtomicExpandPass.cpp.
std::pair<llvm::Value *, llvm::Value *> EmitAtomicCompareExchangeOp(
llvm::Value *ExpectedVal, llvm::Value *DesiredVal,
llvm::AtomicOrdering Success =
@@ -1399,13 +1401,22 @@ RValue AtomicInfo::convertAtomicTempToRValue(Address addr,
LVal.getBaseInfo(), TBAAAccessInfo()));
}
+static bool shouldCastToInt(llvm::Type *ValTy, bool CmpXchg) {
+ // TODO: Also pass through non-ieee FP types.
+ bool KeepType = (ValTy->isIntegerTy() || ValTy->isPointerTy() ||
+ (ValTy->isIEEELikeFPTy() && !CmpXchg));
+ return !KeepType;
+}
+
RValue AtomicInfo::ConvertToValueOrAtomic(llvm::Value *Val,
AggValueSlot ResultSlot,
SourceLocation Loc, bool AsValue,
- bool CastFP) const {
+ bool CmpXchg) const {
// Try not to in some easy cases.
- assert((Val->getType()->isIntegerTy() || Val->getType()->isIEEELikeFPTy()) &&
- "Expected integer or floating point value");
+ assert((Val->getType()->isIntegerTy() || Val->getType()->isPointerTy() ||
+ Val->getType()->isIEEELikeFPTy()) &&
+ "Expected integer, pointer or floating point value when converting "
+ "result.");
if (getEvaluationKind() == TEK_Scalar &&
(((!LVal.isBitField() ||
LVal.getBitFieldInfo().Size == ValueSizeInBits) &&
@@ -1414,13 +1425,11 @@ RValue AtomicInfo::ConvertToValueOrAtomic(llvm::Value *Val,
auto *ValTy = AsValue
? CGF.ConvertTypeForMem(ValueTy)
: getAtomicAddress().getElementType();
- if (ValTy->isIntegerTy() || (!CastFP && ValTy->isIEEELikeFPTy())) {
+ if (!shouldCastToInt(ValTy, CmpXchg)) {
assert((!ValTy->isIntegerTy() || Val->getType() == ValTy) &&
"Different integer types.");
return RValue::get(CGF.EmitFromMemory(Val, ValueTy));
- } else if (ValTy->isPointerTy())
- return RValue::get(CGF.Builder.CreateIntToPtr(Val, ValTy));
- else if (llvm::CastInst::isBitCastable(Val->getType(), ValTy))
+ } else if (llvm::CastInst::isBitCastable(Val->getType(), ValTy))
return RValue::get(CGF.Builder.CreateBitCast(Val, ValTy));
}
@@ -1457,10 +1466,10 @@ void AtomicInfo::EmitAtomicLoadLibcall(llvm::Value *AddForLoaded,
}
llvm::Value *AtomicInfo::EmitAtomicLoadOp(llvm::AtomicOrdering AO,
- bool IsVolatile, bool CastFP) {
+ bool IsVolatile, bool CmpXchg) {
// Okay, we're doing this natively.
Address Addr = getAtomicAddress();
- if (!(Addr.getElementType()->isIEEELikeFPTy() && !CastFP))
+ if (shouldCastToInt(Addr.getElementType(), CmpXchg))
Addr = castToAtomicIntPointer(Addr);
llvm::LoadInst *Load = CGF.Builder.CreateLoad(Addr, "atomic-load");
Load->setAtomic(AO);
@@ -1521,7 +1530,7 @@ RValue AtomicInfo::EmitAtomicLoad(AggValueSlot ResultSlot, SourceLocation Loc,
}
// Okay, we're doing this natively.
- auto *Load = EmitAtomicLoadOp(AO, IsVolatile, /*CastFP=*/false);
+ auto *Load = EmitAtomicLoadOp(AO, IsVolatile);
// If we're ignoring an aggregate return, don't do anything.
if (getEvaluationKind() == TEK_Aggregate && ResultSlot.isIgnored())
@@ -1529,8 +1538,7 @@ RValue AtomicInfo::EmitAtomicLoad(AggValueSlot ResultSlot, SourceLocation Loc,
// Okay, turn that back into the original value or atomic (for non-simple
// lvalues) type.
- return ConvertToValueOrAtomic(Load, ResultSlot, Loc, AsValue,
- /*CastFP=*/false);
+ return ConvertToValueOrAtomic(Load, ResultSlot, Loc, AsValue);
}
/// Emit a load from an l-value of atomic type. Note that the r-value
@@ -1599,20 +1607,17 @@ llvm::Value *AtomicInfo::getScalarRValValueOrNull(RValue RVal) const {
return nullptr;
}
-llvm::Value *AtomicInfo::convertRValueToInt(RValue RVal, bool CastFP) const {
+llvm::Value *AtomicInfo::convertRValueToInt(RValue RVal, bool CmpXchg) const {
// If we've got a scalar value of the right size, try to avoid going
// through memory. Floats get casted if needed by AtomicExpandPass.
if (llvm::Value *Value = getScalarRValValueOrNull(RVal)) {
- if (isa<llvm::IntegerType>(Value->getType()) ||
- (!CastFP && Value->getType()->isIEEELikeFPTy()))
+ if (!shouldCastToInt(Value->getType(), CmpXchg))
return CGF.EmitToMemory(Value, ValueTy);
else {
llvm::IntegerType *InputIntTy = llvm::IntegerType::get(
CGF.getLLVMContext(),
LVal.isSimple() ? getValueSizeInBits() : getAtomicSizeInBits());
- if (isa<llvm::PointerType>(Value->getType()))
- return CGF.Builder.CreatePtrToInt(Value, InputIntTy);
- else if (llvm::BitCastInst::isBitCastable(Value->getType(), InputIntTy))
+ if (llvm::BitCastInst::isBitCastable(Value->getType(), InputIntTy))
return CGF.Builder.CreateBitCast(Value, InputIntTy);
}
}
@@ -1685,13 +1690,14 @@ std::pair<RValue, llvm::Value *> AtomicInfo::EmitAtomicCompareExchange(
// If we've got a scalar value of the right size, try to avoid going
// through memory.
- auto *ExpectedVal = convertRValueToInt(Expected);
- auto *DesiredVal = convertRValueToInt(Desired);
+ auto *ExpectedVal = convertRValueToInt(Expected, /*CmpXchg=*/true);
+ auto *DesiredVal = convertRValueToInt(Desired, /*CmpXchg=*/true);
auto Res = EmitAtomicCompareExchangeOp(ExpectedVal, DesiredVal, Success,
Failure, IsWeak);
return std::make_pair(
ConvertToValueOrAtomic(Res.first, AggValueSlot::ignored(),
- SourceLocation(), /*AsValue=*/false),
+ SourceLocation(), /*AsValue=*/false,
+ /*CmpXchg=*/true),
Res.second);
}
@@ -1785,7 +1791,7 @@ void AtomicInfo::EmitAtomicUpdateOp(
auto Failure = llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(AO);
// Do the atomic load.
- auto *OldVal = EmitAtomicLoadOp(Failure, IsVolatile);
+ auto *OldVal = EmitAtomicLoadOp(Failure, IsVolatile, /*CmpXchg=*/true);
// For non-simple lvalues perform compare-and-swap procedure.
auto *ContBB = CGF.createBasicBlock("atomic_cont");
auto *ExitBB = CGF.createBasicBlock("atomic_exit");
@@ -1801,7 +1807,8 @@ void AtomicInfo::EmitAtomicUpdateOp(
CGF.Builder.CreateStore(PHI, NewAtomicIntAddr);
}
auto OldRVal = ConvertToValueOrAtomic(PHI, AggValueSlot::ignored(),
- SourceLocation(), /*AsValue=*/false);
+ SourceLocation(), /*AsValue=*/false,
+ /*CmpXchg=*/true);
EmitAtomicUpdateValue(CGF, *this, OldRVal, UpdateOp, NewAtomicAddr);
auto *DesiredVal = CGF.Builder.CreateLoad(NewAtomicIntAddr);
// Try to write new value using cmpxchg operation.
@@ -1867,7 +1874,7 @@ void AtomicInfo::EmitAtomicUpdateOp(llvm::AtomicOrdering AO, RValue UpdateRVal,
auto Failure = llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(AO);
// Do the atomic load.
- auto *OldVal = EmitAtomicLoadOp(Failure, IsVolatile);
+ auto *OldVal = EmitAtomicLoadOp(Failure, IsVolatile, /*CmpXchg=*/true);
// For non-simple lvalues perform compare-and-swap procedure.
auto *ContBB = CGF.createBasicBlock("atomic_cont");
auto *ExitBB = CGF.createBasicBlock("atomic_exit");
@@ -1966,21 +1973,16 @@ void CodeGenFunction::EmitAtomicStore(RValue rvalue, LValue dest,
}
// Okay, we're doing this natively.
- llvm::Value *ValToStore =
- atomics.convertRValueToInt(rvalue, /*CastFP=*/false);
+ llvm::Value *ValToStore = atomics.convertRValueToInt(rvalue);
// Do the atomic store.
Address Addr = atomics.getAtomicAddress();
- bool ShouldCastToInt = true;
if (llvm::Value *Value = atomics.getScalarRValValueOrNull(rvalue))
- if (isa<llvm::IntegerType>(Value->getType()) ||
- Value->getType()->isIEEELikeFPTy())
- ShouldCastToInt = false;
- if (ShouldCastToInt) {
- Addr = atomics.castToAtomicIntPointer(Addr);
- ValToStore = Builder.CreateIntCast(ValToStore, Addr.getElementType(),
- /*isSigned=*/false);
- }
+ if (shouldCastToInt(Value->getType(), /*CmpXchg=*/false)) {
+ Addr = atomics.castToAtomicIntPointer(Addr);
+ ValToStore = Builder.CreateIntCast(ValToStore, Addr.getElementType(),
+ /*isSigned=*/false);
+ }
llvm::StoreInst *store = Builder.CreateStore(ValToStore, Addr);
if (AO == llvm::AtomicOrdering::Acquire)
diff --git a/clang/test/CodeGen/atomic.c b/clang/test/CodeGen/atomic.c
index af5c056bbfe6e8..e919f2ad374447 100644
--- a/clang/test/CodeGen/atomic.c
+++ b/clang/test/CodeGen/atomic.c
@@ -134,14 +134,11 @@ static _Atomic float glob_flt = 0.0f;
void force_global_uses(void) {
(void)glob_pointer;
- // CHECK: %[[LOCAL_INT:.+]] = load atomic i32, ptr @[[GLOB_POINTER]] seq_cst
- // CHECK-NEXT: inttoptr i32 %[[LOCAL_INT]] to ptr
+ // CHECK: load atomic ptr, ptr @[[GLOB_POINTER]] seq_cst
(void)glob_pointer_from_int;
- // CHECK: %[[LOCAL_INT_2:.+]] = load atomic i32, ptr @[[GLOB_POINTER_FROM_INT]] seq_cst
- // CHECK-NEXT: inttoptr i32 %[[LOCAL_INT_2]] to ptr
+ // CHECK-NEXT: load atomic ptr, ptr @[[GLOB_POINTER_FROM_INT]] seq_cst
(void)nonstatic_glob_pointer_from_int;
- // CHECK: %[[LOCAL_INT_3:.+]] = load atomic i32, ptr @[[NONSTATIC_GLOB_POINTER_FROM_INT]] seq_cst
- // CHECK-NEXT: inttoptr i32 %[[LOCAL_INT_3]] to ptr
+ // CHECK-NEXT: load atomic ptr, ptr @[[NONSTATIC_GLOB_POINTER_FROM_INT]] seq_cst
(void)glob_int;
// CHECK: load atomic i32, ptr @[[GLOB_INT]] seq_cst
(void)glob_flt;
>From 50ff6af3d1936dea32827e780c5d4896fe153668 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson <paulson1 at linux.ibm.com>
Date: Wed, 27 Mar 2024 11:44:15 +0100
Subject: [PATCH 2/2] Updates after review.
---
clang/lib/CodeGen/CGAtomic.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index f543529efe5b69..b3cc4d787a2a4f 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -1402,9 +1402,9 @@ RValue AtomicInfo::convertAtomicTempToRValue(Address addr,
}
static bool shouldCastToInt(llvm::Type *ValTy, bool CmpXchg) {
- // TODO: Also pass through non-ieee FP types.
- bool KeepType = (ValTy->isIntegerTy() || ValTy->isPointerTy() ||
- (ValTy->isIEEELikeFPTy() && !CmpXchg));
+ bool KeepType =
+ (ValTy->isIntegerTy() || ValTy->isPointerTy() ||
+ (ValTy->isFloatingPointTy() && !ValTy->isX86_FP80Ty() && !CmpXchg));
return !KeepType;
}
More information about the cfe-commits
mailing list