[llvm-exegesis] Switch from intptr_t to uintptr_t in most cases #102860
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-tools-llvm-exegesis Author: Aiden Grossman (boomanaiden154) ChangesThis patch switches most of the uses of intptr_t to uintptr_t within llvm-exegesis for the subprocess memory support. In the vast majority of cases we do not want a signed component of the address, hence making intptr_t undesirable. intptr_t is left for error handling, for example when making syscalls and we need to see if the syscall returned -1. Full diff: https://github.com/llvm/llvm-project/pull/102860.diff 12 Files Affected:
diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.h b/llvm/tools/llvm-exegesis/lib/Assembler.h
index d85d7fdcf04f54..198b8754468098 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.h
+++ b/llvm/tools/llvm-exegesis/lib/Assembler.h
@@ -115,7 +115,7 @@ class ExecutableFunction {
// Executes the function.
void operator()(char *Memory) const {
- ((void (*)(char *))(intptr_t)FunctionBytes.data())(Memory);
+ ((void (*)(char *))(uintptr_t)FunctionBytes.data())(Memory);
}
StringRef FunctionBytes;
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
index 4ae6bc2a54cd50..3c09a8380146e5 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
@@ -53,7 +53,7 @@ struct MemoryValue {
struct MemoryMapping {
// The address to place the mapping at.
- intptr_t Address;
+ uintptr_t Address;
// The name of the value that should be mapped.
std::string MemoryValueName;
};
@@ -73,7 +73,7 @@ struct BenchmarkKey {
std::string Config;
// The address that the snippet should be loaded in at if the execution mode
// being used supports it.
- intptr_t SnippetAddress = 0;
+ uintptr_t SnippetAddress = 0;
// The register that should be used to hold the loop counter.
unsigned LoopRegister;
};
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index adee869967d98a..d2ed46242c70fb 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -379,7 +379,7 @@ class SubProcessFunctionExecutorImpl
if (ChildSignalInfo.si_signo == SIGSEGV)
return make_error<SnippetSegmentationFault>(
- reinterpret_cast<intptr_t>(ChildSignalInfo.si_addr));
+ reinterpret_cast<uintptr_t>(ChildSignalInfo.si_addr));
return make_error<SnippetSignal>(ChildSignalInfo.si_signo);
}
@@ -478,7 +478,7 @@ class SubProcessFunctionExecutorImpl
RseqStructSize = 32;
long RseqDisableOutput =
- syscall(SYS_rseq, (intptr_t)__builtin_thread_pointer() + __rseq_offset,
+ syscall(SYS_rseq, (uintptr_t)__builtin_thread_pointer() + __rseq_offset,
RseqStructSize, RSEQ_FLAG_UNREGISTER, RSEQ_SIG);
if (RseqDisableOutput != 0)
exit(ChildProcessExitCodeE::RSeqDisableFailed);
@@ -502,7 +502,7 @@ class SubProcessFunctionExecutorImpl
char *FunctionDataCopy =
(char *)mmap(MapAddress, FunctionDataCopySize, PROT_READ | PROT_WRITE,
MapFlags, 0, 0);
- if ((intptr_t)FunctionDataCopy == -1)
+ if (reinterpret_cast<intptr_t>(FunctionDataCopy) == -1)
exit(ChildProcessExitCodeE::FunctionDataMappingFailed);
memcpy(FunctionDataCopy, this->Function.FunctionBytes.data(),
@@ -515,7 +515,7 @@ class SubProcessFunctionExecutorImpl
if (!AuxMemFDOrError)
exit(ChildProcessExitCodeE::AuxiliaryMemorySetupFailed);
- ((void (*)(size_t, int))(intptr_t)FunctionDataCopy)(FunctionDataCopySize,
+ ((void (*)(size_t, int))(uintptr_t)FunctionDataCopy)(FunctionDataCopySize,
*AuxMemFDOrError);
exit(0);
diff --git a/llvm/tools/llvm-exegesis/lib/Error.h b/llvm/tools/llvm-exegesis/lib/Error.h
index 2587c45f557434..9f48d007a11b2c 100644
--- a/llvm/tools/llvm-exegesis/lib/Error.h
+++ b/llvm/tools/llvm-exegesis/lib/Error.h
@@ -52,15 +52,15 @@ class SnippetExecutionFailure : public ErrorInfo<SnippetExecutionFailure> {
class SnippetSegmentationFault : public SnippetExecutionFailure {
public:
static char ID;
- SnippetSegmentationFault(intptr_t SegFaultAddress)
+ SnippetSegmentationFault(uintptr_t SegFaultAddress)
: Address(SegFaultAddress){};
- intptr_t getAddress() { return Address; }
+ uintptr_t getAddress() { return Address; }
void log(raw_ostream &OS) const override;
private:
- intptr_t Address;
+ uintptr_t Address;
};
// A class representing all other non-specific failures that happen during
diff --git a/llvm/tools/llvm-exegesis/lib/SnippetFile.cpp b/llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
index 08da04cc62d9cf..bb4223fec72772 100644
--- a/llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
+++ b/llvm/tools/llvm-exegesis/lib/SnippetFile.cpp
@@ -151,7 +151,7 @@ class BenchmarkCodeStreamer : public MCStreamer, public AsmCommentConsumer {
}
if (CommentText.consume_front("SNIPPET-ADDRESS")) {
// LLVM-EXEGESIS-SNIPPET-ADDRESS <address>
- if (!to_integer<intptr_t>(CommentText.trim(), Result->Key.SnippetAddress,
+ if (!to_integer<uintptr_t>(CommentText.trim(), Result->Key.SnippetAddress,
16)) {
errs() << "invalid comment 'LLVM-EXEGESIS-SNIPPET-ADDRESS "
<< CommentText
diff --git a/llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp b/llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp
index 89d7b197079e4f..ce0ce918ae0d7b 100644
--- a/llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp
+++ b/llvm/tools/llvm-exegesis/lib/SubprocessMemory.cpp
@@ -126,7 +126,7 @@ Expected<int> SubprocessMemory::setupAuxiliaryMemoryInSubprocess(
int *AuxiliaryMemoryMapping =
(int *)mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_SHARED,
AuxiliaryMemoryFileDescriptor, 0);
- if ((intptr_t)AuxiliaryMemoryMapping == -1)
+ if (reinterpret_cast<intptr_t>(AuxiliaryMemoryMapping) == -1)
return make_error<Failure>("Mapping auxiliary memory failed");
AuxiliaryMemoryMapping[0] = CounterFileDescriptor;
for (auto &[Name, MemVal] : MemoryDefinitions) {
diff --git a/llvm/tools/llvm-exegesis/lib/Target.h b/llvm/tools/llvm-exegesis/lib/Target.h
index 522c75d15703d5..92cc1cb248a1c0 100644
--- a/llvm/tools/llvm-exegesis/lib/Target.h
+++ b/llvm/tools/llvm-exegesis/lib/Target.h
@@ -118,8 +118,8 @@ class ExegesisTarget {
// Generates the code to mmap a region of code. The code generated by this
// function may clobber registers.
virtual std::vector<MCInst>
- generateMmap(intptr_t Address, size_t Length,
- intptr_t FileDescriptorAddress) const {
+ generateMmap(uintptr_t Address, size_t Length,
+ uintptr_t FileDescriptorAddress) const {
report_fatal_error(
"generateMmap is not implemented on the current architecture");
}
@@ -161,7 +161,7 @@ class ExegesisTarget {
"current architectures");
}
- virtual intptr_t getAuxiliaryMemoryStartAddress() const {
+ virtual uintptr_t getAuxiliaryMemoryStartAddress() const {
report_fatal_error("getAuxiliaryMemoryStartAddress is not implemented on "
"the current architecture");
}
diff --git a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
index dbb2bf409316e7..4709dede5b2e20 100644
--- a/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/X86/Target.cpp
@@ -747,8 +747,8 @@ class ExegesisX86Target : public ExegesisTarget {
std::vector<MCInst> generateExitSyscall(unsigned ExitCode) const override;
std::vector<MCInst>
- generateMmap(intptr_t Address, size_t Length,
- intptr_t FileDescriptorAddress) const override;
+ generateMmap(uintptr_t Address, size_t Length,
+ uintptr_t FileDescriptorAddress) const override;
void generateMmapAuxMem(std::vector<MCInst> &GeneratedCode) const override;
@@ -758,7 +758,7 @@ class ExegesisX86Target : public ExegesisTarget {
std::vector<MCInst> setStackRegisterToAuxMem() const override;
- intptr_t getAuxiliaryMemoryStartAddress() const override;
+ uintptr_t getAuxiliaryMemoryStartAddress() const override;
std::vector<MCInst> configurePerfCounter(long Request, bool SaveRegisters) const override;
@@ -1091,9 +1091,9 @@ std::vector<MCInst> ExegesisX86Target::setRegTo(const MCSubtargetInfo &STI,
#ifdef __linux__
#ifdef __arm__
-static constexpr const intptr_t VAddressSpaceCeiling = 0xC0000000;
+static constexpr const uintptr_t VAddressSpaceCeiling = 0xC0000000;
#else
-static constexpr const intptr_t VAddressSpaceCeiling = 0x0000800000000000;
+static constexpr const uintptr_t VAddressSpaceCeiling = 0x0000800000000000;
#endif
void generateRoundToNearestPage(unsigned int Register,
@@ -1180,8 +1180,8 @@ ExegesisX86Target::generateExitSyscall(unsigned ExitCode) const {
}
std::vector<MCInst>
-ExegesisX86Target::generateMmap(intptr_t Address, size_t Length,
- intptr_t FileDescriptorAddress) const {
+ExegesisX86Target::generateMmap(uintptr_t Address, size_t Length,
+ uintptr_t FileDescriptorAddress) const {
std::vector<MCInst> MmapCode;
MmapCode.push_back(loadImmediate(X86::RDI, 64, APInt(64, Address)));
MmapCode.push_back(loadImmediate(X86::RSI, 64, APInt(64, Length)));
@@ -1249,7 +1249,7 @@ std::vector<MCInst> ExegesisX86Target::setStackRegisterToAuxMem() const {
SubprocessMemory::AuxiliaryMemorySize)};
}
-intptr_t ExegesisX86Target::getAuxiliaryMemoryStartAddress() const {
+uintptr_t ExegesisX86Target::getAuxiliaryMemoryStartAddress() const {
// Return the second to last page in the virtual address space to try and
// prevent interference with memory annotations in the snippet
return VAddressSpaceCeiling - 2 * getpagesize();
diff --git a/llvm/unittests/tools/llvm-exegesis/BenchmarkRunnerTest.cpp b/llvm/unittests/tools/llvm-exegesis/BenchmarkRunnerTest.cpp
index 9414a48c989e4e..a188a7f266c206 100644
--- a/llvm/unittests/tools/llvm-exegesis/BenchmarkRunnerTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/BenchmarkRunnerTest.cpp
@@ -17,7 +17,7 @@ namespace {
TEST(ScratchSpaceTest, Works) {
BenchmarkRunner::ScratchSpace Space;
- EXPECT_EQ(reinterpret_cast<intptr_t>(Space.ptr()) %
+ EXPECT_EQ(reinterpret_cast<uintptr_t>(Space.ptr()) %
BenchmarkRunner::ScratchSpace::kAlignment,
0u);
Space.ptr()[0] = 42;
diff --git a/llvm/unittests/tools/llvm-exegesis/X86/SnippetFileTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/SnippetFileTest.cpp
index f1fa891171177c..de883ab750d201 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/SnippetFileTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/X86/SnippetFileTest.cpp
@@ -81,7 +81,8 @@ MATCHER_P3(MemoryDefinitionIs, Name, Value, Size, "") {
}
MATCHER_P2(MemoryMappingIs, Address, Name, "") {
- if (arg.Address == Address && arg.MemoryValueName == Name)
+ if (arg.Address == static_cast<uintptr_t>(Address) &&
+ arg.MemoryValueName == Name)
return true;
*result_listener << "expected: {" << Address << ", " << Name << "} ";
*result_listener << "actual: {" << arg.Address << ", " << arg.MemoryValueName
@@ -216,7 +217,7 @@ TEST_F(X86SnippetFileTest, SnippetAddress) {
ASSERT_TRUE(static_cast<bool>(Snippets));
EXPECT_THAT(*Snippets, SizeIs(1));
const auto &Snippet = (*Snippets)[0];
- EXPECT_EQ(Snippet.Key.SnippetAddress, 0x10000);
+ EXPECT_EQ(Snippet.Key.SnippetAddress, static_cast<uintptr_t>(0x10000));
}
TEST_F(X86SnippetFileTest, LoopRegister) {
diff --git a/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
index f61254ac74e140..a0cad289e978f4 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
@@ -64,7 +64,7 @@ class SubprocessMemoryTest : public X86TestBase {
shm_open(DefinitionName.c_str(), O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
uint8_t *SharedMemoryMapping = (uint8_t *)mmap(
NULL, DefinitionSize, PROT_READ, MAP_SHARED, SharedMemoryFD, 0);
- EXPECT_NE((intptr_t)SharedMemoryMapping, -1);
+ EXPECT_NE(reinterpret_cast<intptr_t>(SharedMemoryMapping), -1);
for (size_t I = 0; I < ExpectedValue.size(); ++I) {
EXPECT_EQ(SharedMemoryMapping[I], ExpectedValue[I]);
}
diff --git a/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp
index e0427664d2c3cf..3a028bad486ccc 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/X86/TargetTest.cpp
@@ -599,9 +599,9 @@ TEST_F(X86Core2TargetTest, GenerateLowerMunmapTest) {
}
#ifdef __arm__
-static constexpr const intptr_t VAddressSpaceCeiling = 0xC0000000;
+static constexpr const uintptr_t VAddressSpaceCeiling = 0xC0000000;
#else
-static constexpr const intptr_t VAddressSpaceCeiling = 0x0000800000000000;
+static constexpr const uintptr_t VAddressSpaceCeiling = 0x0000800000000000;
#endif
TEST_F(X86Core2TargetTest, GenerateUpperMunmapTest) {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
This patch switches most of the uses of intptr_t to uintptr_t within llvm-exegesis for the subprocess memory support. In the vast majority of cases we do not want a signed component of the address, hence making intptr_t undesirable. intptr_t is left for error handling, for example when making syscalls and we need to see if the syscall returned -1.
boomanaiden154
force-pushed
the
exegesis-intptr-to-uintptr
branch
from
d654256 to
4c3ed60
Compare
legrosbuffle
approved these changes
Aug 27, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/4535 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch switches most of the uses of intptr_t to uintptr_t within llvm-exegesis for the subprocess memory support. In the vast majority of cases we do not want a signed component of the address, hence making intptr_t undesirable. intptr_t is left for error handling, for example when making syscalls and we need to see if the syscall returned -1.