[LLVMdev] [RFC][PATCH][OPENCL] synchronization scopes redux
Sahasrabuddhe, Sameer
sameer.sahasrabuddhe at amd.com
Fri Dec 12 10:25:26 PST 2014
On 12/11/2014 4:28 PM, Sahasrabuddhe, Sameer wrote:
>
> Attached is a sequence of patches that changes the IR to support more
> than two synchronization scopes. This is still a work in progress, and
> these patches are only meant to start a more detailed discussion on
> the way forward.
>
Ping!
Found a simple way to preserve forward compatibility. See below.
> One big issue is the absence of any backend that actually makes use of
> intermediate synchronization scopes. This work is meant to be just one
> part of the ground work required for landing the much-anticipated
> HSAIL backend. Also, more work might be needed for emitting atomic
> instructions via Clang.
>
> The proposed syntax for synchronization scope is as follows:
>
> 1. Synchronization scopes are of arbitrary width, but implemented as
> unsigned in the bitcode, just like address spaces.
> 2. Cross-thread is default, but now encoded as 0.
> 3. Keyword 'singlethread' is unchanged, but now encoded as the
> largest integer (which happens to be ~0U in bitcode).
> 4. New syntax "synchscope(n)" for other scopes.
> 5. There is no keyword for cross-thread, but it can be specified as
> "synchscope(0)".
>
> This change breaks forward compatibility for the bitcode, since the
> meaning of the zero/one values are now changed.
>
> enum SynchronizationScope {
> - SingleThread = 0,
> - CrossThread = 1
> + CrossThread = 0,
> + SingleThread = ~0U
> };
>
> The change passes almost all lit tests including one new test (see
> patch 0005). The failing tests are specifically checking for forward
> compatibility:
>
> Failing Tests (3):
> LLVM :: Bitcode/cmpxchg-upgrade.ll
> LLVM :: Bitcode/memInstructions.3.2.ll
> LLVM :: Bitcode/weak-cmpxchg-upgrade.ll
>
> This breakage remains even if we reverse the order of synchronization
> scopes. One simple way to preserve compatibility is to retain 0 and 1
> with their current meanings, and specify that intermediate scopes are
> represented in an ordered way with numbers greater than one. But this
> is pretty ugly to work with. Would appreciate inputs on how to fix this!
>
The issue here is purely in the bitcode, and we need an encoding that
can represent new intermediate scopes while preserving the two known
values of zero and one. Note that the earlier zero is now ~0U in the
in-memory representation, and the earlier 1 is now zero. This mapping
can be easily accomplished with a simple increment/decrement by one,
ignoring overflow. So the bitreader now subtracts a one when decoding
the synch scope, and bitwriter adds a one when encoding the synch scope.
The attached change number 0006 is meant to replace changes 0003 and
0005 in the previous list, since the assembly and the bitcode need to be
updated simultaneously for this to work.
The new change passes all tests, including the ones checking for forward
compatibility.
Sameer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141212/bcb5048a/attachment.html>
-------------- next part --------------
From 428935e38a4eb13b8fdcc0f091148311082d5582 Mon Sep 17 00:00:00 2001
From: Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com>
Date: Wed, 10 Dec 2014 16:03:40 +0530
Subject: [PATCH 4/4] assembly, bitcode
Introduce new syntax in the assembly:
1. crossthread is default.
2. keyword 'singlethread' as before.
3. new syntax "synchscope(n)" for other scopes.
4. synchscope(0) is the same as crossthread.
Fix the bitcode reader and writer to transparently deal with unsigned integers
for synchronization scope, instead of trying to interpret them.
---
include/llvm/Bitcode/LLVMBitCodes.h | 6 ------
include/llvm/IR/Instructions.h | 4 ++--
lib/AsmParser/LLLexer.cpp | 1 +
lib/AsmParser/LLParser.cpp | 35 ++++++++++++++++++++++++++---------
lib/AsmParser/LLParser.h | 3 ++-
lib/AsmParser/LLToken.h | 1 +
lib/Bitcode/Reader/BitcodeReader.cpp | 18 +++++++-----------
lib/Bitcode/Writer/BitcodeWriter.cpp | 6 +-----
lib/IR/AsmWriter.cpp | 23 +++++++++++++++--------
test/Bitcode/synch_scope.ll | 18 ++++++++++++++++++
10 files changed, 73 insertions(+), 42 deletions(-)
create mode 100644 test/Bitcode/synch_scope.ll
diff --git a/include/llvm/Bitcode/LLVMBitCodes.h b/include/llvm/Bitcode/LLVMBitCodes.h
index c42ecfe..c78218a 100644
--- a/include/llvm/Bitcode/LLVMBitCodes.h
+++ b/include/llvm/Bitcode/LLVMBitCodes.h
@@ -260,12 +260,6 @@ namespace bitc {
ORDERING_SEQCST = 6
};
- /// Encoded SynchronizationScope values.
- enum AtomicSynchScopeCodes {
- SYNCHSCOPE_SINGLETHREAD = 0,
- SYNCHSCOPE_CROSSTHREAD = 1
- };
-
// The function body block (FUNCTION_BLOCK_ID) describes function bodies. It
// can contain a constant block (CONSTANTS_BLOCK_ID).
enum FunctionCodes {
diff --git a/include/llvm/IR/Instructions.h b/include/llvm/IR/Instructions.h
index 9e25717..bb8f3a4 100644
--- a/include/llvm/IR/Instructions.h
+++ b/include/llvm/IR/Instructions.h
@@ -46,8 +46,8 @@ enum AtomicOrdering {
};
enum SynchronizationScope {
- SingleThread = 0,
- CrossThread = 1
+ CrossThread = 0,
+ SingleThread = ~0U
};
/// Returns true if the ordering is at least as strong as acquire
diff --git a/lib/AsmParser/LLLexer.cpp b/lib/AsmParser/LLLexer.cpp
index ca3c9e8..fc1f1fe 100644
--- a/lib/AsmParser/LLLexer.cpp
+++ b/lib/AsmParser/LLLexer.cpp
@@ -540,6 +540,7 @@ lltok::Kind LLLexer::LexIdentifier() {
KEYWORD(acq_rel);
KEYWORD(seq_cst);
KEYWORD(singlethread);
+ KEYWORD(synchscope);
KEYWORD(nnan);
KEYWORD(ninf);
diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp
index ca4ba6e..a6e5939 100644
--- a/lib/AsmParser/LLParser.cpp
+++ b/lib/AsmParser/LLParser.cpp
@@ -1617,20 +1617,37 @@ bool LLParser::ParseOptionalCommaAlign(unsigned &Alignment,
}
/// ParseScopeAndOrdering
-/// if isAtomic: ::= 'singlethread'? AtomicOrdering
+/// if isAtomic: ::= SynchronizationScope ? AtomicOrdering
/// else: ::=
///
/// This sets Scope and Ordering to the parsed values.
-bool LLParser::ParseScopeAndOrdering(bool isAtomic, SynchronizationScope &Scope,
+bool LLParser::ParseScopeAndOrdering(bool isAtomic, unsigned &Scope,
AtomicOrdering &Ordering) {
if (!isAtomic)
return false;
+ ParseOptionalSynchScope(Scope);
+
+ return ParseOrdering(Ordering);
+}
+
+/// ParseOptionalSynchScope
+/// := /*empty*/
+/// := 'singlethread'
+/// := 'synchscope' '(' uint32 ')'
+bool LLParser::ParseOptionalSynchScope(unsigned &Scope) {
Scope = CrossThread;
- if (EatIfPresent(lltok::kw_singlethread))
+
+ if (EatIfPresent(lltok::kw_singlethread)) {
Scope = SingleThread;
+ return false;
+ }
- return ParseOrdering(Ordering);
+ if (!EatIfPresent(lltok::kw_synchscope))
+ return false;
+ return ParseToken(lltok::lparen, "expected '(' in synchronization scope") ||
+ ParseUInt32(Scope) ||
+ ParseToken(lltok::rparen, "expected ')' in synchronization scope");
}
/// ParseOrdering
@@ -4384,7 +4401,7 @@ int LLParser::ParseLoad(Instruction *&Inst, PerFunctionState &PFS) {
bool AteExtraComma = false;
bool isAtomic = false;
AtomicOrdering Ordering = NotAtomic;
- SynchronizationScope Scope = CrossThread;
+ unsigned Scope = CrossThread;
if (Lex.getKind() == lltok::kw_atomic) {
isAtomic = true;
@@ -4425,7 +4442,7 @@ int LLParser::ParseStore(Instruction *&Inst, PerFunctionState &PFS) {
bool AteExtraComma = false;
bool isAtomic = false;
AtomicOrdering Ordering = NotAtomic;
- SynchronizationScope Scope = CrossThread;
+ unsigned Scope = CrossThread;
if (Lex.getKind() == lltok::kw_atomic) {
isAtomic = true;
@@ -4468,7 +4485,7 @@ int LLParser::ParseCmpXchg(Instruction *&Inst, PerFunctionState &PFS) {
bool AteExtraComma = false;
AtomicOrdering SuccessOrdering = NotAtomic;
AtomicOrdering FailureOrdering = NotAtomic;
- SynchronizationScope Scope = CrossThread;
+ unsigned Scope = CrossThread;
bool isVolatile = false;
bool isWeak = false;
@@ -4521,7 +4538,7 @@ int LLParser::ParseAtomicRMW(Instruction *&Inst, PerFunctionState &PFS) {
Value *Ptr, *Val; LocTy PtrLoc, ValLoc;
bool AteExtraComma = false;
AtomicOrdering Ordering = NotAtomic;
- SynchronizationScope Scope = CrossThread;
+ unsigned Scope = CrossThread;
bool isVolatile = false;
AtomicRMWInst::BinOp Operation;
@@ -4574,7 +4591,7 @@ int LLParser::ParseAtomicRMW(Instruction *&Inst, PerFunctionState &PFS) {
/// ::= 'fence' 'singlethread'? AtomicOrdering
int LLParser::ParseFence(Instruction *&Inst, PerFunctionState &PFS) {
AtomicOrdering Ordering = NotAtomic;
- SynchronizationScope Scope = CrossThread;
+ unsigned Scope = CrossThread;
if (ParseScopeAndOrdering(true /*Always atomic*/, Scope, Ordering))
return true;
diff --git a/lib/AsmParser/LLParser.h b/lib/AsmParser/LLParser.h
index d32e58e..fd93b13 100644
--- a/lib/AsmParser/LLParser.h
+++ b/lib/AsmParser/LLParser.h
@@ -227,8 +227,9 @@ namespace llvm {
bool ParseOptionalCallingConv(unsigned &CC);
bool ParseOptionalAlignment(unsigned &Alignment);
bool ParseOptionalDereferenceableBytes(uint64_t &Bytes);
- bool ParseScopeAndOrdering(bool isAtomic, SynchronizationScope &Scope,
+ bool ParseScopeAndOrdering(bool isAtomic, unsigned &Scope,
AtomicOrdering &Ordering);
+ bool ParseOptionalSynchScope(unsigned &Scope);
bool ParseOrdering(AtomicOrdering &Ordering);
bool ParseOptionalStackAlignment(unsigned &Alignment);
bool ParseOptionalCommaAlign(unsigned &Alignment, bool &AteExtraComma);
diff --git a/lib/AsmParser/LLToken.h b/lib/AsmParser/LLToken.h
index a2111ca..ff70e89 100644
--- a/lib/AsmParser/LLToken.h
+++ b/lib/AsmParser/LLToken.h
@@ -63,6 +63,7 @@ namespace lltok {
kw_atomic,
kw_unordered, kw_monotonic, kw_acquire, kw_release, kw_acq_rel, kw_seq_cst,
kw_singlethread,
+ kw_synchscope,
kw_nnan,
kw_ninf,
kw_nsz,
diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index cc61806..81936f5 100644
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -225,12 +225,8 @@ static AtomicOrdering GetDecodedOrdering(unsigned Val) {
}
}
-static SynchronizationScope GetDecodedSynchScope(unsigned Val) {
- switch (Val) {
- case bitc::SYNCHSCOPE_SINGLETHREAD: return SingleThread;
- default: // Map unknown scopes to cross-thread.
- case bitc::SYNCHSCOPE_CROSSTHREAD: return CrossThread;
- }
+static unsigned GetDecodedSynchScope(unsigned Val) {
+ return Val - 1;
}
static Comdat::SelectionKind getDecodedComdatSelectionKind(unsigned Val) {
@@ -3104,7 +3100,7 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
return Error(BitcodeError::InvalidRecord);
if (Ordering != NotAtomic && Record[OpNum] == 0)
return Error(BitcodeError::InvalidRecord);
- SynchronizationScope SynchScope = GetDecodedSynchScope(Record[OpNum+3]);
+ unsigned SynchScope = GetDecodedSynchScope(Record[OpNum+3]);
I = new LoadInst(Op, "", Record[OpNum+1], (1 << Record[OpNum]) >> 1,
Ordering, SynchScope);
@@ -3138,7 +3134,7 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
if (Ordering == NotAtomic || Ordering == Acquire ||
Ordering == AcquireRelease)
return Error(BitcodeError::InvalidRecord);
- SynchronizationScope SynchScope = GetDecodedSynchScope(Record[OpNum+3]);
+ unsigned SynchScope = GetDecodedSynchScope(Record[OpNum+3]);
if (Ordering != NotAtomic && Record[OpNum] == 0)
return Error(BitcodeError::InvalidRecord);
@@ -3162,7 +3158,7 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
AtomicOrdering SuccessOrdering = GetDecodedOrdering(Record[OpNum+1]);
if (SuccessOrdering == NotAtomic || SuccessOrdering == Unordered)
return Error(BitcodeError::InvalidRecord);
- SynchronizationScope SynchScope = GetDecodedSynchScope(Record[OpNum+2]);
+ unsigned SynchScope = GetDecodedSynchScope(Record[OpNum+2]);
AtomicOrdering FailureOrdering;
if (Record.size() < 7)
@@ -3204,7 +3200,7 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
AtomicOrdering Ordering = GetDecodedOrdering(Record[OpNum+2]);
if (Ordering == NotAtomic || Ordering == Unordered)
return Error(BitcodeError::InvalidRecord);
- SynchronizationScope SynchScope = GetDecodedSynchScope(Record[OpNum+3]);
+ unsigned SynchScope = GetDecodedSynchScope(Record[OpNum+3]);
I = new AtomicRMWInst(Operation, Ptr, Val, Ordering, SynchScope);
cast<AtomicRMWInst>(I)->setVolatile(Record[OpNum+1]);
InstructionList.push_back(I);
@@ -3217,7 +3213,7 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
if (Ordering == NotAtomic || Ordering == Unordered ||
Ordering == Monotonic)
return Error(BitcodeError::InvalidRecord);
- SynchronizationScope SynchScope = GetDecodedSynchScope(Record[1]);
+ unsigned SynchScope = GetDecodedSynchScope(Record[1]);
I = new FenceInst(Context, Ordering, SynchScope);
InstructionList.push_back(I);
break;
diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp
index f2a8fcb..da4e8b9 100644
--- a/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -132,11 +132,7 @@ static unsigned GetEncodedOrdering(AtomicOrdering Ordering) {
}
static unsigned GetEncodedSynchScope(unsigned SynchScope) {
- switch (SynchScope) {
- case SingleThread: return bitc::SYNCHSCOPE_SINGLETHREAD;
- case CrossThread: return bitc::SYNCHSCOPE_CROSSTHREAD;
- }
- llvm_unreachable("Invalid synch scope");
+ return SynchScope + 1;
}
static void WriteStringRecord(unsigned Code, StringRef Str,
diff --git a/lib/IR/AsmWriter.cpp b/lib/IR/AsmWriter.cpp
index 69d6a97..9a9d97c 100644
--- a/lib/IR/AsmWriter.cpp
+++ b/lib/IR/AsmWriter.cpp
@@ -1431,15 +1431,25 @@ void AssemblyWriter::writeOperand(const Value *Operand, bool PrintType) {
WriteAsOperandInternal(Out, Operand, &TypePrinter, &Machine, TheModule);
}
+static void printSynchScope(unsigned SynchScope, raw_ostream &Out) {
+ switch (SynchScope) {
+ case CrossThread:
+ break;
+ case SingleThread:
+ Out << " singlethread";
+ break;
+ default:
+ Out << " synchscope(" << SynchScope << ")";
+ break;
+ }
+}
+
void AssemblyWriter::writeAtomic(AtomicOrdering Ordering,
unsigned SynchScope) {
if (Ordering == NotAtomic)
return;
- switch (SynchScope) {
- case SingleThread: Out << " singlethread"; break;
- case CrossThread: break;
- }
+ printSynchScope(SynchScope, Out);
switch (Ordering) {
default: Out << " <bad ordering " << int(Ordering) << ">"; break;
@@ -1457,10 +1467,7 @@ void AssemblyWriter::writeAtomicCmpXchg(AtomicOrdering SuccessOrdering,
unsigned SynchScope) {
assert(SuccessOrdering != NotAtomic && FailureOrdering != NotAtomic);
- switch (SynchScope) {
- case SingleThread: Out << " singlethread"; break;
- case CrossThread: break;
- }
+ printSynchScope(SynchScope, Out);
switch (SuccessOrdering) {
default: Out << " <bad ordering " << int(SuccessOrdering) << ">"; break;
diff --git a/test/Bitcode/synch_scope.ll b/test/Bitcode/synch_scope.ll
new file mode 100644
index 0000000..8be8c14
--- /dev/null
+++ b/test/Bitcode/synch_scope.ll
@@ -0,0 +1,18 @@
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
+define void @test(i32* %addr) {
+ cmpxchg i32* %addr, i32 42, i32 0 monotonic monotonic
+; CHECK: cmpxchg i32* %addr, i32 42, i32 0 monotonic monotonic
+
+ cmpxchg i32* %addr, i32 42, i32 0 singlethread monotonic monotonic
+; CHECK: cmpxchg i32* %addr, i32 42, i32 0 singlethread monotonic monotonic
+
+; The explicit synchscope should be dropped.
+ cmpxchg i32* %addr, i32 42, i32 0 synchscope(0) monotonic monotonic
+; CHECK: cmpxchg i32* %addr, i32 42, i32 0 monotonic monotonic
+
+ cmpxchg i32* %addr, i32 42, i32 0 synchscope(42) monotonic monotonic
+; CHECK: cmpxchg i32* %addr, i32 42, i32 0 synchscope(42) monotonic monotonic
+
+ ret void
+}
--
1.9.1
More information about the llvm-dev
mailing list