ARM ABI #14194

maleadt · 2015-11-30T12:23:51Z

Hi all,

I wanted to run Julia on my Raspberry Pi 2, so I have taken a stab at implementing an ARM ABI interface for Julia. This fixes the following issues: #13508, #12355, #12347, #12343, and partially fixes #12344 (the parallel test suite doesn't pass yet, but the segfaults are fixed).

Although I'm happy with how most of Julia now works on my ARM-based computer, I don't think that implementing ABI's in this ad-hoc fashion is really future-proof. I'm sure there's missing features (not caught by the current test suite), and I'm not entirely convinced by the current ABI interface (needPassByRef, use_sret, preferred_llvm_type etc). Maybe we should look into using llvm-abi (announcement thread), a library for generating ABI-compliant LLVM IR?

Some questions/uncertainties:

I've added checks on EABI (AAPCS) and VFP procedure calling, currently #erroring out if not found. I'm not sure if this is appropriate, but the ABI currently relies on this.
I'm currently casting heterogeneous aggregates to integer arrays for alignment purposes, because this is what Clang does. On x86 however, Clang uses the align attribute. I don't think this is easy to add without messing with ccall, but it might be a neater way of aligning aggregates.

vtjnash · 2015-12-04T22:20:30Z

src/abi_arm.cpp

+ // TODO: If the argument is an integral Fundamental Data Type that is
+ // smaller than a word, then it is zero- or sign-extended to a full word and
+ // its size is set to 4 bytes. If the argument is a Half-precision Floating
+ // Point Type it is converted to Single Precision.


you can return T_int32 and the callee of this function will zero/sign extend to accommodate

vtjnash · 2015-12-04T22:55:44Z

Maybe we should look into using llvm-abi (announcement thread), a library for generating ABI-compliant LLVM IR?

i had seen that announcement thread, but that library doesn't actually have a very good or complete ABI implementation and yet contains considerably more code.

On x86 however, Clang uses the align attribute

afaics, this is only meaningful when applied to byval arguments. if not specified, it defaults to the platform-default value, which should be perfectly suitable for our requirements.

I'm currently casting heterogeneous aggregates to integer arrays for alignment purposes, because this is what Clang does.

yep, it looks like clang does the following:

define void @test_4(%struct.struct4* noalias sret %agg.result, [3 x i32] %a.coerce) #1 {
  %a = alloca %struct.struct4, align 4
  %1 = bitcast %struct.struct4* %a to [3 x i32]*
  store [3 x i32] %a.coerce, [3 x i32]* %1, align 1

maleadt · 2015-12-07T09:35:28Z

Thanks for the feedback, @vtjnash, I'll be looking at it. Seems like there's some other places I'm dropping the ball (not generating srets where Clang does, and seemingly lucking out). I'm going to have a look at some improved testing, maybe by direct comparison against Clang's function signatures instead of actually executing code and hoping it crashes.

ViralBShah · 2015-12-08T17:14:21Z

Would have been a good idea to keep this open, and force push to this branch. That way the discussion around the ARM ABI could have continued here. Anyways, it can always a new PR.

maleadt · 2015-12-08T17:16:08Z

Huh, the delete & close was actually an unintended artifact of a git push --mirror from a host which didn't have the arm_abi branch... Let's see if I can fix this up.

EDIT: done. I'll try not to delete this branch in the future :-)

ViralBShah · 2015-12-29T16:53:37Z

Any updates on this one to get it to a state where we can merge it? I have been salivating on this PR for a few days and hoping we can even backport it as well.

JeffBezanson · 2015-12-29T19:36:40Z

+1 This is an awesome piece of work. Would be great to bring it to conclusion.

maleadt · 2015-12-29T21:31:25Z

Sorry, haven't had the time to work on this, and I won't for the next few days either. I'll look at it next week, but if anyone is impatient enough to pick this up, be my guest :-)

maleadt · 2016-01-11T18:41:29Z

As promised, here is a reworked version of this PR. The current version is much more thorough, follows the docs better, and aligns with @yuyichao's recent AArch64 work.

It successfully passes my extended version of the ccall test suite (see PR #14215), except the Int128 test passing an Int128 to a C function accepting a struct { int64_t, int64_t }. I'm not sure how to make sense of that.

@vtjnash isHomogeneous currently still works on Julia types, will fix tomorrow. I just wanted to get some eyes on this already.

yuyichao · 2016-01-11T18:47:21Z

src/abi_arm.cpp

+//===----------------------------------------------------------------------===https://
+
+// LIMITATIONS
+// - doesn't respect register usage, these should be encoded in the ABI state


I think this is handled by llvm and it should handle it well as long as we emit the correct lowered type.

I think you're right, I had thought the split register/stack mode would need to be generated by the front-end but it seems part of ARMCallingConv.td.

yuyichao · 2016-01-11T18:53:00Z

It successfully passes my extended version of the ccall test suite (see PR #14215), except the Int128 test passing an Int128 to a C function accepting a struct { int64_t, int64_t }. I'm not sure how to make sense of that.

I haven't fully read the AArch32 calling convension doc yet but I do remember __int128 is not a machine type on it (on the other hand it is on AArch64) so the frontend (us) shouldn't emit such types for calling c functions. (i.e. you should probably translate i128 into [2 x i64], this is what I do for bitstypes of invalid (no C correspondence) sizes for AArch64).

That test is technically wrong for AArch64 since struct {int64_t, int64_t} and __int128 has different alignment requirement. It is fine for now though since there's no practical differences when passing as the first/only parameter.

maleadt · 2016-01-12T13:04:58Z

Decomposing the Int128 into [2 x i64] turned out slightly more complex, as in this case it needed to be sret too and Julia didn't honour the ABI's preferred type in case of sret.

maleadt · 2016-01-12T15:18:06Z

This should be pretty feature complete now. I guess the homogeneous aggregate detection could be shared between ABI's, like Clang does it, but I don't really feel like tackling that right now.

ViralBShah · 2016-01-13T19:39:37Z

Do the commits need squashing? While @vtjnash should review and merge the PR, I am guessing that tests are not necessary here since the failing tests that this PR will fix are sufficient to catch future regressions.

maleadt · 2016-01-13T20:05:07Z

Yeah, those tests + those from #14215 should cover the ABI sufficiently. Maybe there should be a CI bot for ARM too? Anyway, I'll squash/rebase/remove the RFC tag tomorrow morning.

ViralBShah · 2016-01-13T20:11:04Z

On Travis, one has to compile with a cross compiling toolchain and then test in qemu, which is probably really slow. There is a plan to at least set up buildbots that can run every night on a scaleway machine. @staticfloat almost has this going, but currently giving armv7 as the option to -march is failing.

yuyichao · 2016-01-13T21:38:55Z

src/abi_arm.cpp

+
+void needPassByRef(AbiState *state,jl_value_t *ty, bool *byRef, bool *inReg)
+{
+ // B.1 (TODO)


This is irrelevant. It doesn't really exist in C and it roughly correspond to our Any which is indeed passing by pointer.

yuyichao · 2016-01-15T02:37:12Z

src/abi_arm.cpp

+
+ base = NULL;
+ for (size_t i = 0; i < members; ++i) {
+ Type *T = isLegalHAType((jl_datatype_t*)jl_field_type(dt,i));


And I believe the members counting is wrong for recursive types.

maleadt · 2016-01-18T11:55:38Z

@yuyichao I addressed your comments.

Other pending reviews? If not, is this good to go?

yuyichao · 2016-01-18T12:13:41Z

LGTM =)

@vtjnash Any comments? If not I'll merge later today.

ARM ABI

ViralBShah · 2016-01-22T10:42:56Z

Bumping again to see if we can backport to 0.4. Looks like it shouldn't be a difficult backport and fairly self-contained.

ViralBShah · 2016-01-25T09:57:41Z

@tkelman As you had mentioned to me, the codegen restructure makes it difficult to have a clean backport to 0.4. Perhaps ARM should just wait for 0.5.

tkelman · 2016-01-25T12:00:19Z

I thought I phrased it the other way around - anything that depends on the codegen rewrite will obviously be tough to backport on its own, but most of the currently backport-tagged arm-related PR's look more or less standalone? Let's see what ends up being easy to backport for 0.4.4 and how many arm bugs we can fix there without spending too much effort.

ViralBShah · 2016-01-25T12:20:43Z

I actually have backported pretty much all of them. There are compile errors in the arm ABI file, because they refer to things that were not in 0.4.

tkelman · 2016-01-25T12:23:03Z

You introduced a bunch of CI failures. Could you have gone through a PR?

ViralBShah · 2016-01-25T12:25:26Z

I never pushed I thought.

ViralBShah · 2016-01-25T12:26:46Z

At least I didn't mean to, but might have done it out of habit, when on a branch - except this was the release branch. Can we get rid of them right away?

tkelman · 2016-01-25T12:33:41Z

Revert the commits that are broken. The release branch is protected so force pushing is disabled. Does release-0.4 compile on arm with llvm 3.7.1? If the new ABI doesn't compile on ARM without the codegen rewrite then that's a regression w.r.t. 0.4.3 so that should probably also be reverted.

ViralBShah · 2016-01-25T12:56:42Z

I have reverted locally the arm abi patches. I think the rest should be ok, and this will give a fully functioning build, with llvm 3.7.1. Once I do that successfully, I can push the reverts.

tkelman · 2016-01-25T13:04:47Z

See my commit comments. d93b819 and dfba26b aren't correct as-is for backporting. And e8c44f6 needs to be adjusted.

edit: should really be testing from a clean build here, since you're picking up uncommitted files

(cherry picked from commit 0eae5e5) ref #14194

tkelman added the system:arm ARMv7 and AArch64 label Nov 30, 2015

maleadt mentioned this pull request Dec 1, 2015

Extend and clean the ccall test suite. #14215

Closed

vtjnash reviewed Dec 4, 2015
View reviewed changes

maleadt closed this Dec 8, 2015

maleadt deleted the arm_abi branch December 8, 2015 12:58

maleadt reopened this Dec 8, 2015

yuyichao mentioned this pull request Jan 6, 2016

@parallel (+) segmentation fault on arm #13508

Closed

maleadt changed the title ~~ARM ABI~~ RFC: ARM ABI Jan 11, 2016

yuyichao reviewed Jan 11, 2016
View reviewed changes

yuyichao reviewed Jan 13, 2016
View reviewed changes

yuyichao reviewed Jan 15, 2016
View reviewed changes

yuyichao mentioned this pull request Jan 15, 2016

Fix HVA and HFA support on AArch64 #14688

Merged

maleadt added 2 commits January 16, 2016 15:55

ARM ABI: fix HA member counting in the case of nested composite types.

b7238d4

ARM ABI: add fast path checks.

d2d6df5

yuyichao added a commit that referenced this pull request Jan 18, 2016

Merge pull request #14194 from maleadt/arm_abi

3f6161a

ARM ABI

yuyichao merged commit 3f6161a into JuliaLang:master Jan 18, 2016

This was referenced Jan 18, 2016

ccall test failure on arm #12343

Closed

strings tests segfaulting on arm #12347

Closed

dates test segfaults on arm #12355

Closed

ViralBShah added the backport pending 0.4 label Jan 22, 2016

yuyichao mentioned this pull request Jan 22, 2016

Implement AArch64 ABI #14519

Merged

KristofferC mentioned this pull request Jan 25, 2016

release-0.4 make error #14789

Closed

tkelman mentioned this pull request Mar 7, 2016

[release-0.4] WIP: backports for 0.4.4 #15385

Merged

8 tasks

tkelman pushed a commit that referenced this pull request Mar 27, 2016

Implement ARM ABI.

33b783d

(cherry picked from commit 0eae5e5) ref #14194

tkelman pushed a commit that referenced this pull request Mar 27, 2016

Implement ARM ABI.

0b2c097

(cherry picked from commit 0eae5e5) ref #14194

tkelman mentioned this pull request Mar 27, 2016

[release-0.4] backport ARM ABI #15638

Merged

yuyichao mentioned this pull request Jun 9, 2016

parse(Float64, "1,2") and parse(Float32, "1,2") cause segfault on ARM 32 bit #16849

Closed

tkelman removed the backport pending 0.4 label Jun 13, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARM ABI #14194

ARM ABI #14194

maleadt commented Nov 30, 2015

vtjnash Dec 4, 2015

vtjnash commented Dec 4, 2015

maleadt commented Dec 7, 2015

ViralBShah commented Dec 8, 2015

maleadt commented Dec 8, 2015

ViralBShah commented Dec 29, 2015

JeffBezanson commented Dec 29, 2015

maleadt commented Dec 29, 2015

maleadt commented Jan 11, 2016

yuyichao Jan 11, 2016

maleadt Jan 12, 2016

yuyichao commented Jan 11, 2016

maleadt commented Jan 12, 2016

maleadt commented Jan 12, 2016

ViralBShah commented Jan 13, 2016

maleadt commented Jan 13, 2016

ViralBShah commented Jan 13, 2016

yuyichao Jan 13, 2016

maleadt Jan 14, 2016

yuyichao Jan 15, 2016

maleadt commented Jan 18, 2016

yuyichao commented Jan 18, 2016

ViralBShah commented Jan 22, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016

ARM ABI #14194

ARM ABI #14194

Conversation

maleadt commented Nov 30, 2015

vtjnash Dec 4, 2015

Choose a reason for hiding this comment

vtjnash commented Dec 4, 2015

maleadt commented Dec 7, 2015

ViralBShah commented Dec 8, 2015

maleadt commented Dec 8, 2015

ViralBShah commented Dec 29, 2015

JeffBezanson commented Dec 29, 2015

maleadt commented Dec 29, 2015

maleadt commented Jan 11, 2016

yuyichao Jan 11, 2016

Choose a reason for hiding this comment

maleadt Jan 12, 2016

Choose a reason for hiding this comment

yuyichao commented Jan 11, 2016

maleadt commented Jan 12, 2016

maleadt commented Jan 12, 2016

ViralBShah commented Jan 13, 2016

maleadt commented Jan 13, 2016

ViralBShah commented Jan 13, 2016

yuyichao Jan 13, 2016

Choose a reason for hiding this comment

maleadt Jan 14, 2016

Choose a reason for hiding this comment

yuyichao Jan 15, 2016

Choose a reason for hiding this comment

maleadt commented Jan 18, 2016

yuyichao commented Jan 18, 2016

ViralBShah commented Jan 22, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016

ViralBShah commented Jan 25, 2016

tkelman commented Jan 25, 2016