Skip to content

Commit

Permalink
Revert r351778: IR: Add fp operations to atomicrmw
Browse files Browse the repository at this point in the history
This broke the RISCV build, and even with that fixed, one of the RISCV
tests behaves surprisingly differently with asserts than without,
leaving there no clear test pattern to use. Generally it seems bad for
hte IR to differ substantially due to asserts (as in, an alloca is used
with asserts that isn't needed without!) and nothing I did simply would
fix it so I'm reverting back to green.

This also required reverting the RISCV build fix in r351782.

llvm-svn: 351796
  • Loading branch information
chandlerc committed Jan 22, 2019
1 parent 33c16a3 commit 285fe71
Show file tree
Hide file tree
Showing 26 changed files with 22 additions and 395 deletions.
13 changes: 4 additions & 9 deletions llvm/docs/LangRef.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8667,18 +8667,15 @@ operation. The operation must be one of the following keywords:
- min
- umax
- umin
- fadd
- fsub

For most of these operations, the type of '<value>' must be an integer
type whose bit width is a power of two greater than or equal to eight
and less than or equal to a target-specific size limit. For xchg, this
may also be a floating point type with the same size constraints as
integers. For fadd/fsub, this must be a floating point type. The
type of the '``<pointer>``' operand must be a pointer to that type. If
the ``atomicrmw`` is marked as ``volatile``, then the optimizer is not
allowed to modify the number or order of execution of this
``atomicrmw`` with other :ref:`volatile operations <volatile>`.
integers. The type of the '``<pointer>``' operand must be a pointer to
that type. If the ``atomicrmw`` is marked as ``volatile``, then the
optimizer is not allowed to modify the number or order of execution of
this ``atomicrmw`` with other :ref:`volatile operations <volatile>`.

A ``atomicrmw`` instruction can also take an optional
":ref:`syncscope <syncscope>`" argument.
Expand All @@ -8704,8 +8701,6 @@ operation argument:
comparison)
- umin: ``*ptr = *ptr < val ? *ptr : val`` (using an unsigned
comparison)
- fadd: ``*ptr = *ptr + val`` (using floating point arithmetic)
- fsub: ``*ptr = *ptr - val`` (using floating point arithmetic)

Example:
""""""""
Expand Down
4 changes: 1 addition & 3 deletions llvm/include/llvm/Bitcode/LLVMBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,7 @@ enum RMWOperations {
RMW_MAX = 7,
RMW_MIN = 8,
RMW_UMAX = 9,
RMW_UMIN = 10,
RMW_FADD = 11,
RMW_FSUB = 12
RMW_UMIN = 10
};

/// OverflowingBinaryOperatorOptionalFlags - Flags for serializing
Expand Down
5 changes: 2 additions & 3 deletions llvm/include/llvm/CodeGen/TargetLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -1715,9 +1715,8 @@ class TargetLoweringBase {

/// Returns how the IR-level AtomicExpand pass should expand the given
/// AtomicRMW, if at all. Default is to never expand.
virtual AtomicExpansionKind shouldExpandAtomicRMWInIR(AtomicRMWInst *RMW) const {
return RMW->isFloatingPointOperation() ?
AtomicExpansionKind::CmpXChg : AtomicExpansionKind::None;
virtual AtomicExpansionKind shouldExpandAtomicRMWInIR(AtomicRMWInst *) const {
return AtomicExpansionKind::None;
}

/// On some platforms, an AtomicRMW that never actually modifies the value
Expand Down
22 changes: 1 addition & 21 deletions llvm/include/llvm/IR/Instructions.h
Original file line number Diff line number Diff line change
Expand Up @@ -724,14 +724,8 @@ class AtomicRMWInst : public Instruction {
/// *p = old <unsigned v ? old : v
UMin,

/// *p = old + v
FAdd,

/// *p = old - v
FSub,

FIRST_BINOP = Xchg,
LAST_BINOP = FSub,
LAST_BINOP = UMin,
BAD_BINOP
};

Expand All @@ -753,16 +747,6 @@ class AtomicRMWInst : public Instruction {

static StringRef getOperationName(BinOp Op);

static bool isFPOperation(BinOp Op) {
switch (Op) {
case AtomicRMWInst::FAdd:
case AtomicRMWInst::FSub:
return true;
default:
return false;
}
}

void setOperation(BinOp Operation) {
unsigned short SubclassData = getSubclassDataFromInstruction();
setInstructionSubclassData((SubclassData & 31) |
Expand Down Expand Up @@ -820,10 +804,6 @@ class AtomicRMWInst : public Instruction {
return getPointerOperand()->getType()->getPointerAddressSpace();
}

bool isFloatingPointOperation() const {
return isFPOperation(getOperation());
}

// Methods for support type inquiry through isa, cast, and dyn_cast:
static bool classof(const Instruction *I) {
return I->getOpcode() == Instruction::AtomicRMW;
Expand Down
40 changes: 12 additions & 28 deletions llvm/lib/AsmParser/LLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6815,7 +6815,6 @@ int LLParser::ParseAtomicRMW(Instruction *&Inst, PerFunctionState &PFS) {
AtomicOrdering Ordering = AtomicOrdering::NotAtomic;
SyncScope::ID SSID = SyncScope::System;
bool isVolatile = false;
bool IsFP = false;
AtomicRMWInst::BinOp Operation;

if (EatIfPresent(lltok::kw_volatile))
Expand All @@ -6834,14 +6833,6 @@ int LLParser::ParseAtomicRMW(Instruction *&Inst, PerFunctionState &PFS) {
case lltok::kw_min: Operation = AtomicRMWInst::Min; break;
case lltok::kw_umax: Operation = AtomicRMWInst::UMax; break;
case lltok::kw_umin: Operation = AtomicRMWInst::UMin; break;
case lltok::kw_fadd:
Operation = AtomicRMWInst::FAdd;
IsFP = true;
break;
case lltok::kw_fsub:
Operation = AtomicRMWInst::FSub;
IsFP = true;
break;
}
Lex.Lex(); // Eat the operation.

Expand All @@ -6858,25 +6849,18 @@ int LLParser::ParseAtomicRMW(Instruction *&Inst, PerFunctionState &PFS) {
if (cast<PointerType>(Ptr->getType())->getElementType() != Val->getType())
return Error(ValLoc, "atomicrmw value and pointer type do not match");

if (Operation == AtomicRMWInst::Xchg) {
if (!Val->getType()->isIntegerTy() &&
!Val->getType()->isFloatingPointTy()) {
return Error(ValLoc, "atomicrmw " +
AtomicRMWInst::getOperationName(Operation) +
" operand must be an integer or floating point type");
}
} else if (IsFP) {
if (!Val->getType()->isFloatingPointTy()) {
return Error(ValLoc, "atomicrmw " +
AtomicRMWInst::getOperationName(Operation) +
" operand must be a floating point type");
}
} else {
if (!Val->getType()->isIntegerTy()) {
return Error(ValLoc, "atomicrmw " +
AtomicRMWInst::getOperationName(Operation) +
" operand must be an integer");
}
if (Operation != AtomicRMWInst::Xchg && !Val->getType()->isIntegerTy()) {
return Error(ValLoc, "atomicrmw " +
AtomicRMWInst::getOperationName(Operation) +
" operand must be an integer");
}

if (Operation == AtomicRMWInst::Xchg &&
!Val->getType()->isIntegerTy() &&
!Val->getType()->isFloatingPointTy()) {
return Error(ValLoc, "atomicrmw " +
AtomicRMWInst::getOperationName(Operation) +
" operand must be an integer or floating point type");
}

unsigned Size = Val->getType()->getPrimitiveSizeInBits();
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1034,8 +1034,6 @@ static AtomicRMWInst::BinOp getDecodedRMWOperation(unsigned Val) {
case bitc::RMW_MIN: return AtomicRMWInst::Min;
case bitc::RMW_UMAX: return AtomicRMWInst::UMax;
case bitc::RMW_UMIN: return AtomicRMWInst::UMin;
case bitc::RMW_FADD: return AtomicRMWInst::FAdd;
case bitc::RMW_FSUB: return AtomicRMWInst::FSub;
}
}

Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,6 @@ static unsigned getEncodedRMWOperation(AtomicRMWInst::BinOp Op) {
case AtomicRMWInst::Min: return bitc::RMW_MIN;
case AtomicRMWInst::UMax: return bitc::RMW_UMAX;
case AtomicRMWInst::UMin: return bitc::RMW_UMIN;
case AtomicRMWInst::FAdd: return bitc::RMW_FADD;
case AtomicRMWInst::FSub: return bitc::RMW_FSUB;
}
}

Expand Down
6 changes: 0 additions & 6 deletions llvm/lib/CodeGen/AtomicExpandPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -549,10 +549,6 @@ static Value *performAtomicOp(AtomicRMWInst::BinOp Op, IRBuilder<> &Builder,
case AtomicRMWInst::UMin:
NewVal = Builder.CreateICmpULE(Loaded, Inc);
return Builder.CreateSelect(NewVal, Loaded, Inc, "new");
case AtomicRMWInst::FAdd:
return Builder.CreateFAdd(Loaded, Inc, "new");
case AtomicRMWInst::FSub:
return Builder.CreateFSub(Loaded, Inc, "new");
default:
llvm_unreachable("Unknown atomic op");
}
Expand Down Expand Up @@ -1551,8 +1547,6 @@ static ArrayRef<RTLIB::Libcall> GetRMWLibcall(AtomicRMWInst::BinOp Op) {
case AtomicRMWInst::Min:
case AtomicRMWInst::UMax:
case AtomicRMWInst::UMin:
case AtomicRMWInst::FAdd:
case AtomicRMWInst::FSub:
// No atomic libcalls are available for max/min/umax/umin.
return {};
}
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/IR/Instructions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1407,10 +1407,6 @@ StringRef AtomicRMWInst::getOperationName(BinOp Op) {
return "umax";
case AtomicRMWInst::UMin:
return "umin";
case AtomicRMWInst::FAdd:
return "fadd";
case AtomicRMWInst::FSub:
return "fsub";
case AtomicRMWInst::BAD_BINOP:
return "<invalid operation>";
}
Expand Down
5 changes: 0 additions & 5 deletions llvm/lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3435,11 +3435,6 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
AtomicRMWInst::getOperationName(Op) +
" operand must have integer or floating point type!",
&RMWI, ElTy);
} else if (AtomicRMWInst::isFPOperation(Op)) {
Assert(ElTy->isFloatingPointTy(), "atomicrmw " +
AtomicRMWInst::getOperationName(Op) +
" operand must have floating point type!",
&RMWI, ElTy);
} else {
Assert(ElTy->isIntegerTy(), "atomicrmw " +
AtomicRMWInst::getOperationName(Op) +
Expand Down
3 changes: 0 additions & 3 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11600,9 +11600,6 @@ AArch64TargetLowering::shouldExpandAtomicLoadInIR(LoadInst *LI) const {
// For the real atomic operations, we have ldxr/stxr up to 128 bits,
TargetLowering::AtomicExpansionKind
AArch64TargetLowering::shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const {
if (AI->isFloatingPointOperation())
return AtomicExpansionKind::CmpXChg;

unsigned Size = AI->getType()->getPrimitiveSizeInBits();
if (Size > 128) return AtomicExpansionKind::None;
// Nand not supported in LSE.
Expand Down
3 changes: 0 additions & 3 deletions llvm/lib/Target/ARM/ARMISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14645,9 +14645,6 @@ ARMTargetLowering::shouldExpandAtomicLoadInIR(LoadInst *LI) const {
// and up to 64 bits on the non-M profiles
TargetLowering::AtomicExpansionKind
ARMTargetLowering::shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const {
if (AI->isFloatingPointOperation())
return AtomicExpansionKind::CmpXChg;

unsigned Size = AI->getType()->getPrimitiveSizeInBits();
bool hasAtomicRMW = !Subtarget->isThumb() || Subtarget->hasV8MBaselineOps();
return (Size <= (Subtarget->isMClass() ? 32U : 64U) && hasAtomicRMW)
Expand Down
19 changes: 2 additions & 17 deletions llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3110,21 +3110,13 @@ Value *HexagonTargetLowering::emitLoadLinked(IRBuilder<> &Builder, Value *Addr,
AtomicOrdering Ord) const {
BasicBlock *BB = Builder.GetInsertBlock();
Module *M = BB->getParent()->getParent();
auto PT = cast<PointerType>(Addr->getType());
Type *Ty = PT->getElementType();
Type *Ty = cast<PointerType>(Addr->getType())->getElementType();
unsigned SZ = Ty->getPrimitiveSizeInBits();
assert((SZ == 32 || SZ == 64) && "Only 32/64-bit atomic loads supported");
Intrinsic::ID IntID = (SZ == 32) ? Intrinsic::hexagon_L2_loadw_locked
: Intrinsic::hexagon_L4_loadd_locked;

PointerType *NewPtrTy
= Builder.getIntNTy(SZ)->getPointerTo(PT->getAddressSpace());
Addr = Builder.CreateBitCast(Addr, NewPtrTy);

Value *Fn = Intrinsic::getDeclaration(M, IntID);
Value *Call = Builder.CreateCall(Fn, Addr, "larx");

return Builder.CreateBitCast(Call, Ty);
return Builder.CreateCall(Fn, Addr, "larx");
}

/// Perform a store-conditional operation to Addr. Return the status of the
Expand All @@ -3135,17 +3127,10 @@ Value *HexagonTargetLowering::emitStoreConditional(IRBuilder<> &Builder,
Module *M = BB->getParent()->getParent();
Type *Ty = Val->getType();
unsigned SZ = Ty->getPrimitiveSizeInBits();

Type *CastTy = Builder.getIntNTy(SZ);
assert((SZ == 32 || SZ == 64) && "Only 32/64-bit atomic stores supported");
Intrinsic::ID IntID = (SZ == 32) ? Intrinsic::hexagon_S2_storew_locked
: Intrinsic::hexagon_S4_stored_locked;
Value *Fn = Intrinsic::getDeclaration(M, IntID);

unsigned AS = Addr->getType()->getPointerAddressSpace();
Addr = Builder.CreateBitCast(Addr, CastTy->getPointerTo(AS));
Val = Builder.CreateBitCast(Val, CastTy);

Value *Call = Builder.CreateCall(Fn, {Addr, Val}, "stcx");
Value *Cmp = Builder.CreateICmpEQ(Call, Builder.getInt32(0), "");
Value *Ext = Builder.CreateZExt(Cmp, Type::getInt32Ty(M->getContext()));
Expand Down
6 changes: 0 additions & 6 deletions llvm/lib/Target/RISCV/RISCVISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1720,12 +1720,6 @@ Instruction *RISCVTargetLowering::emitTrailingFence(IRBuilder<> &Builder,

TargetLowering::AtomicExpansionKind
RISCVTargetLowering::shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const {
// atomicrmw {fadd,fsub} must be expanded to use compare-exchange, as
// floating point operations can't be used in an lr/sc sequence without
// brekaing the forward-progress guarantee.
if (AI->isFloatingPointOperation())
return AtomicExpansionKind::CmpXChg;

unsigned Size = AI->getType()->getPrimitiveSizeInBits();
if (Size == 8 || Size == 16)
return AtomicExpansionKind::MaskedIntrinsic;
Expand Down
10 changes: 0 additions & 10 deletions llvm/test/Assembler/atomic.ll
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,3 @@ define void @f(i32* %x) {
fence syncscope("device") seq_cst
ret void
}

define void @fp_atomics(float* %x) {
; CHECK: atomicrmw fadd float* %x, float 1.000000e+00 seq_cst
atomicrmw fadd float* %x, float 1.0 seq_cst

; CHECK: atomicrmw volatile fadd float* %x, float 1.000000e+00 seq_cst
atomicrmw volatile fadd float* %x, float 1.0 seq_cst

ret void
}
7 changes: 0 additions & 7 deletions llvm/test/Assembler/invalid-atomicrmw-fadd-must-be-fp-type.ll

This file was deleted.

7 changes: 0 additions & 7 deletions llvm/test/Assembler/invalid-atomicrmw-fsub-must-be-fp-type.ll

This file was deleted.

7 changes: 0 additions & 7 deletions llvm/test/Bitcode/compatibility.ll
Original file line number Diff line number Diff line change
Expand Up @@ -764,13 +764,6 @@ define void @atomics(i32* %word) {
define void @fp_atomics(float* %word) {
; CHECK: %atomicrmw.xchg = atomicrmw xchg float* %word, float 1.000000e+00 monotonic
%atomicrmw.xchg = atomicrmw xchg float* %word, float 1.0 monotonic

; CHECK: %atomicrmw.fadd = atomicrmw fadd float* %word, float 1.000000e+00 monotonic
%atomicrmw.fadd = atomicrmw fadd float* %word, float 1.0 monotonic

; CHECK: %atomicrmw.fsub = atomicrmw fsub float* %word, float 1.000000e+00 monotonic
%atomicrmw.fsub = atomicrmw fsub float* %word, float 1.0 monotonic

ret void
}

Expand Down
47 changes: 0 additions & 47 deletions llvm/test/Transforms/AtomicExpand/AArch64/atomicrmw-fp.ll

This file was deleted.

Loading

0 comments on commit 285fe71

Please sign in to comment.