Skip to content

ARM and Android fixes: version out some HardFloat tests, use TMPDIR logic and declaration from druntime, and correct comment #5358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dlang-bot merged 4 commits into from
Dec 22, 2017
Merged

ARM and Android fixes: version out some HardFloat tests, use TMPDIR logic and declaration from druntime, and correct comment #5358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7fde88d
Use posix_memalign declaration from druntime instead
joakim-noah May 3, 2017
0080647
Don't run HardFloat tests on SoftFloat systems, like ARM_SoftFloat.
joakim-noah May 21, 2017
ee4036a
Use the Posix TMPDIR logic, as D can be used from the command-line to…
joakim-noah Sep 16, 2017
f2186ef
std.system: Add comment fix for Android
joakim-noah Dec 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions std/experimental/allocator/mallocator.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ struct Mallocator
test!Mallocator();
}

version (Posix)
@nogc nothrow
private extern(C) int posix_memalign(void**, size_t, size_t);

version (Windows)
{
// DMD Win 32 bit, DigitalMars C standard library misses the _aligned_xxx
Expand Down Expand Up @@ -229,6 +225,7 @@ struct AlignedMallocator
void[] alignedAllocate(size_t bytes, uint a) shared
{
import core.stdc.errno : ENOMEM, EINVAL;
import core.sys.posix.stdlib : posix_memalign;
assert(a.isGoodDynamicAlignment);
void* result;
auto code = posix_memalign(&result, a, bytes);
Expand Down
5 changes: 0 additions & 5 deletions std/file.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4353,11 +4353,6 @@ string tempDir() @trusted
DWORD len = GetTempPathW(buf.length, buf.ptr);
if (len) cache = buf[0 .. len].to!string;
}
else version(Android)
{
// Don't check for a global temporary directory as
// Android doesn't have one.
}
Copy link
Copy Markdown
Contributor Author

@joakim-noah joakim-noah Sep 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added another Android commit, as ldc is now packaged on Android itself, and the Termux app does set TMPDIR. This was versioned out because it was assumed that most would run D code from their own Android GUI app, ie an apk, which don't have any global temp directory. However, now that D can be used from the command-line on Android itself, with an app that sets TMPDIR for all command-line D tools, just use the below Posix logic. You will have to set the tempDir by hand for each apk anyway, if you use it.

This already hit me when building rdmd for Android, where I had to patch it to check TMPDIR, can take that out with this change.

else version(Posix)
{
import std.process : environment;
Expand Down
4 changes: 2 additions & 2 deletions std/math.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4816,7 +4816,7 @@ public:
assert(ieeeFlags == f);
}

@system unittest
version(D_HardFloat) @system unittest
Copy link
Copy Markdown
Contributor Author

@joakim-noah joakim-noah May 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this added commit, needed when running the tests on Android/ARM for the merge-2.074 branch of ldc.

Copy link
Copy Markdown
Member

@ibuclaw ibuclaw May 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs moving to a separate pr IMO.

Copy link
Copy Markdown
Contributor Author

@joakim-noah joakim-noah May 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so small, I'd rather just do it here. What's the problem with this commit? These three commits are what I need to get the tests passing on Android/ARM, fairly small PR.

Copy link
Copy Markdown
Member

@ibuclaw ibuclaw May 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one of the commits is fine, we can merge it now without waiting on the other two. I'm not sure about this change, but I haven't looked at the body, and I don't wish to discuss here, as the topic is about murmurhash.

Copy link
Copy Markdown
Contributor Author

@joakim-noah joakim-noah May 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The topic is some small changes I had to make for Android/ARM, ie the contents of this PR. The notion that one can't discuss this tiny commit here because of the surrounding discussion about murmurhash3 is laughable, as most non-negligible Phobos PRs have many such topical threads of discussion.

{
import std.meta : AliasSeq;

Expand Down Expand Up @@ -5260,7 +5260,7 @@ private:
ensureDefaults();
}

@system unittest // rounding
version(D_HardFloat) @system unittest // rounding
{
import std.meta : AliasSeq;

Expand Down
2 changes: 1 addition & 1 deletion std/system.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ immutable
{
win32 = 1, /// Microsoft 32 bit Windows systems
win64, /// Microsoft 64 bit Windows systems
linux, /// All Linux Systems
linux, /// All Linux Systems, except for Android
osx, /// Mac OS X
freeBSD, /// FreeBSD
netBSD, /// NetBSD
Expand Down