src/task.c: Use b instead of br instruction on armv7l

[staticfloat]

Fixes #29710

[yuyichao]

Did you check this works? I think b can only work with label.

[staticfloat]

I am compiling it on a raspberry pi right now, but it is indeed quite slow. I will update this once it is built.

[yuyichao]

And what I wrote before and what I mentioned in the comments was b start_task.

[staticfloat]

I thought we were passing in the location to branch to via %1? Or are you saying to use b start_task %1;?

[yuyichao]

No. I'm saying that according to the doc b doesn't work with registers so it must be b start_task nothing else. fn is passed in as a "r"(register) here which is not supported.

See https://github.com/JuliaLang/julia/pull/13099/files#diff-e33939bb54b7505cfc2f5d12b60d0a9cL391

[staticfloat]

Okay, I see what you're saying now. I feel like Jameson intended for us to have the flexibility of passing in arbitrary function addresses though (even though it is not exercised here, all addresses point to start_task), so I will revert to using bx instead of b.

[yuyichao]

The question I asked in the issue was exactly about this. What was the point of being flexible here? Is there plan to make it not a constant? (If so, that should be commented since the current version is pretty confusing.....)

[staticfloat]

@vtjnash Do we actually need the arbitrary function pointer? If not, is there a performance improvement to using b instead of bx?

[yuyichao]

The code will be shorter and there will be one fewer relocation. It's not a particularly important performance improvement to have but it's confusing only because the more optimized form is actually very simple...

[vtjnash]

it just keeps the code-paths more common

[yuyichao]

But direct jump to label is available on all platforms?

[staticfloat]

With these changes, the code at least compiles and makes it partway through the test suite on an RPi 3B+.

[staticfloat]

Okay the confirmed test error that I'm hitting is the following:

vecelement: Error During Test at /home/pi/local/dist/julia-sf/fixarm/share/julia/test/vecelement.jl:71
  Test threw exception
  Expression: a[2] == Gr(1.0, Bunch((VecElement(2.0), VecElement(3.0))), 4.0)
  StackOverflowError:
  Stacktrace:
   [1] Main.Test40Main_vecelement.Bunch(::Tuple{VecElement{Float64},VecElement{Float64}}) at /home/pi/local/dist/julia-sf/fixarm/share/julia/test/vecelement.jl:35
   [2] top-level scope at /home/pi/local/dist/julia-sf/fixarm/share/julia/test/vecelement.jl:71
   [3] include at ./boot.jl:317 [inlined]
   [4] include_relative(::Module, ::String) at ./loading.jl:1038
   [5] include at ./sysimg.jl:29 [inlined]
   [6] include(::String) at /home/pi/local/dist/julia-sf/fixarm/share/julia/test/testdefs.jl:13
   [7] top-level scope at /home/pi/local/dist/julia-sf/fixarm/share/julia/test/testdefs.jl:22
   [8] top-level scope at /home/pi/tmp/julia-build/julia-sf/fixarm/usr/share/julia/stdlib/v1.1/Test/src/Test.jl:1083
   [9] top-level scope at /home/pi/local/dist/julia-sf/fixarm/share/julia/test/testdefs.jl:21
   [10] top-level scope at util.jl:289
   [11] top-level scope at /home/pi/local/dist/julia-sf/fixarm/share/julia/test/testdefs.jl:19

This seems unrelated to the changes on this branch though, so I think we are ready to merge these fixes.

[ararslan]

Now that we've branched for 1.1, I assume this should be backported to 1.1?

[ararslan]

Is 1.0 affected by this? I thought the commit that broke ARM support wasn't backported.

[staticfloat]

oh you are so right

[ararslan]

What else needs to happen before this can be merged and backported to 1.1?

[staticfloat]

I think this can be merged; I got my Firefly board this morning, so I've been working towards getting true Armv7l buildbots working again, but I think any further changes should be done on different PRs.