[Compiler-RT] [ARM] Where __aeabi_[il]div0 builtins should be implemented?

Hi,

There are several places in compiler-rt which refer to __aeabi_idiv0.
For example, in lib/builtins/arm/udivsi3.S:

#ifdef __ARM_EABI__
    b __aeabi_idiv0
#else
    JMP(lr)
#endif

At the same time there is no definition of it. It looks as if it was
done intentionally so that third-party could provide custom handler for
division by zero.

IMHO It's not very consistent and looks odd as all other __aebi_*
functions are provided by compiler-rt.

Did I get it all right or maybe I'm missing something?

libgcc provides both __aeabi_idiv0 and __aeabi_ldiv0 as weak symbols,
any reasons not to do the same in compiler-rt? Or, to put it
differently, why force external implementation of these functions?

Thanks,
Sergey

Sergey,

Not that it'll save you much hassle, but here's an implementation of __aeabi_idiv0 and __aeabi_ldiv0 that I've been sitting on for a while.

I vaguely remember compnerd suggesting that I don't commit them to compiler_rt, but I don't remember why.

Cheers,

Jon

aeabi_idiv0.c (466 Bytes)

aeabi_ldiv0.c (466 Bytes)

Looks as though whomever implemented the call to __aeabi_idiv0 wanted
to be conservative for non EABI targets.

AFAIK, gnueabi targets recognize all EABI functions, so that should
work well. But I don't know about BSD, Darwin and other targets, how
they would behave if we assumed those functions were always there.

cheers,
--renato

Jon,

Thanks, I have similar stubs to be able to run programs.

Absence of these functions actually surprised me. It's not very
friendly as dynamically linked applications just crash at startup (might
depend on dynamic linker though).

It'd be nice to have this documented somewhere, grepping current
compiler-rt repository gives me nothing except uses of idiv0.

Cheers,
Sergey

Looks as though whomever implemented the call to __aeabi_idiv0 wanted
to be conservative for non EABI targets.

How could it prevent him from providing default implementation of
__aeabi_idiv0() for EABI targets?

AFAIK, gnueabi targets recognize all EABI functions, so that should
work well.

Not sure I understand you, nothing in compiler-rt defines these
functions, they are left unresolved on gnueabit target for me.

But I don't know about BSD, Darwin and other targets, how
they would behave if we assumed those functions were always there.

We do assume they are always there for all EABI targets at the moment.
I'm wondering why it is so. Is it left to be implemented in standard
library or something similar?

Thanks,
Sergey

How could it prevent him from providing default implementation of
__aeabi_idiv0() for EABI targets?

It couldn't. I was referring to this specific ifdef. That file
implements both __aeabi_uidiv and __udivsi3, and from reading that
portion, it looked to me as if that's trying to avoid requiring the
__aeabi_idiv0 symbol on non-EABI targets.

We do assume they are always there for all EABI targets at the moment.

That's correct, and on EABI targets the symbols *should* exist. I'm in
favour for adding them ASAP, but we might need an ifdef to avoid
creating unnecessary (or conflicting) symbols for non-EABI targets.

However, according to RTABI 4.3.2:

The *div0 functions:
 Return the value passed to them as a parameter.
 Or, return a fixed value defined by the execution environment (such as 0).
 Or, raise a signal (often SIGFPE) or throw an exception, and do not return.

The current behaviour is to return zero, but it is setting the zero is
before the call to div0, which is wrong. Jon's implementation would
work in this call, but not anywhere else.

A proper solution would be to have:

LOCAL_LABEL(divby0):
#ifdef __ARM_EABI__
  b __aeabi_idiv0
#else
  mov r0, #0
  JMP(lr)
#endif

And make both __aeabi_{i,l}div0 return 0.

I'm hoping both parts of the ifdef to generate *identical* code, but
with the benefit that we can change the behaviour of div0 by
overriding the function's implementation.

cheers,
--renato

I'm in favour for adding them ASAP, but we might need an ifdef to avoid
creating unnecessary (or conflicting) symbols for non-EABI targets.

Sure, it makes sense.

A proper solution would be to have:

LOCAL_LABEL(divby0):
#ifdef __ARM_EABI__
  b __aeabi_idiv0
#else
  mov r0, #0
  JMP(lr)
#endif

And make both __aeabi_{i,l}div0 return 0.

I'm hoping both parts of the ifdef to generate *identical* code, but
with the benefit that we can change the behaviour of div0 by
overriding the function's implementation.

I missed that

    mov r0, #0

line. It really shouldn't be there for EABI targets.

Will see what Saleem (and maybe others) thinks of this, maybe there is a
non-obvious reason to do not implement these functions in compiler-rt.

Regards,
Sergey

One issue is that they depend on OS-specific functionality.
E.g. the ability to raise a signal. That's primarily why I didn't add an
implementation.

Joerg

According to the EABI, you can either return the argument, a specified
integer or raise a signal, so its behaviour could be OS-specific.

The current behaviour is to return 0, which is fine for most purposes,
and would work on *any* OS. We could expand the idiv0 later, based on
OS, but for now we should have the symbols defined.

cheers,
--renato

Sergey,

Not that it'll save you much hassle, but here's an implementation of
__aeabi_idiv0 and __aeabi_ldiv0 that I've been sitting on for a while.

I vaguely remember compnerd suggesting that I don't commit them to
compiler_rt, but I don't remember why.

I did dig into this further and it seems that they are, in fact, considered
part of the RT-ABI :-(. Ive committed a simple conforming implementation
in SVN r217322.

> Will see what Saleem (and maybe others) thinks of this, maybe there is a
> non-obvious reason to do not implement these functions in compiler-rt.

One issue is that they depend on OS-specific functionality.
E.g. the ability to raise a signal. That's primarily why I didn't add an
implementation.

Yes, that would be the best possible solution. However, given that the
specification calls for the definition to be weak, the c library could
override the function and provide an alternate implementation. Conforming
to the first option doesn't require any runtime library support, so, Ive
taken the liberty of implementing that as a means to get compiler-rt usable
in an environment which does not provide a custom division-by-zero handler
from the math library.

Hi Saleem,

This implementation will differ from the current sdiv's expected
(return 0) if called from a place that doesn't mov r0, #0 just before
calling div0.

ARM and GCC both throw an exception, and on non-EABI ARM, we're
returning zero, so it would be good to have at least one consistent
behaviour.

I think we should return zero on both idiv0 and ldiv0 and move the
"mov r0, #0" inside the #else for now.

cheers,
--renato

> I did dig into this further and it seems that they are, in fact,
considered
> part of the RT-ABI :-(. Ive committed a simple conforming
implementation in
> SVN r217322.

Hi Saleem,

This implementation will differ from the current sdiv's expected
(return 0) if called from a place that doesn't mov r0, #0 just before
calling div0.

Why not adjust this instead?

ARM and GCC both throw an exception, and on non-EABI ARM, we're
returning zero, so it would be good to have at least one consistent
behaviour.

I think its better to avoid pulling in a dependency on the target libc,
particularly if you want to permit the use of compiler-rt in a bare-metal
environment. This implementation conforms to the specification and can be
overridden if the libc wishes to catch the div-by-zero.

We could use __rt_raise(2, 2), which would need to call signal as well, so
you end up growing a dependency on the target environment's libc
implementation. I think that expanding the responsibility of compiler-rt
from supporting the code generation from the compiler to integrating into
the target environment's libc can be problematic.

I think we should return zero on both idiv0 and ldiv0 and move the
"mov r0, #0" inside the #else for now.

Why 0 and not infinity? Or some other value?

Why not adjust this instead?

I was just being conservative. I don't know what else depends on this
library and I don't want to change things outside of my scope.

Since returning zero is consistent with the EABI, I don't think we
should deviate from that norm, not without a lot of thought, at least.

We could use __rt_raise(2, 2), which would need to call signal as well, so
you end up growing a dependency on the target environment's libc
implementation. I think that expanding the responsibility of compiler-rt
from supporting the code generation from the compiler to integrating into
the target environment's libc can be problematic.

I wasn't proposing we signal, but that we return zero, instead of the argument.

Today we return either zero (in divmod/div/mod) or the argument (when
calling div0 directly), and that's just wrong.

Why 0 and not infinity? Or some other value?

Because 0 is what it was before.

cheers,
--renato

> Why not adjust this instead?

I was just being conservative. I don't know what else depends on this
library and I don't want to change things outside of my scope.

The current implementations actually return 0. Can you point out where
that doesn't hold please? If you look at the generated code, in reality,
the function is to spec as `bx lr'. The intent is that it provides a hook
where you can break (without changing any state) OR replace it with the
implementation that you prefer.

Since returning zero is consistent with the EABI, I don't think we
should deviate from that norm, not without a lot of thought, at least.

Yes, although the current implementation also is consistent with EABI.

> We could use __rt_raise(2, 2), which would need to call signal as well,
so
> you end up growing a dependency on the target environment's libc
> implementation. I think that expanding the responsibility of compiler-rt
> from supporting the code generation from the compiler to integrating into
> the target environment's libc can be problematic.

I wasn't proposing we signal, but that we return zero, instead of the
argument.

Fair enough. However, the point is that emulating GCC's behavior of the
exception is what I was referring to.

Today we return either zero (in divmod/div/mod) or the argument (when
calling div0 directly), and that's just wrong.

Quoting the RTABI:

   1.

      int __aeabi_idiv0(int return_value);
      long long __aeabi_ldiv0(long long return_value);

   The *div0 functions:

   􏰁 Return the value passed to them as a parameter.

Is my copy out of date?

The current implementations actually return 0. Can you point out where that
doesn't hold please?

With the current implementation...

int foo(int a) {
  return __aeabi_idiv0(a);
}

returns 'a', while:

int bar(int a) {
  return __aeabi_idiv(a, 0);
}

returns zero.

Quoting the RTABI:

   int __aeabi_idiv0(int return_value);
   long long __aeabi_ldiv0(long long return_value);

The *div0 functions:

􏰁 Return the value passed to them as a parameter.

Is my copy out of date?

As I said before, all three behaviours are allowed by the RTABI
(exception, constant, parameter), but we now have two different
behaviours in compiler-rt. This is a regression.

We must make it consistent to what there was there before (return
zero). If you *really* want to make it return the parameter or signal,
we should discuss this properly, on another thread, but right now,
division by zero in compiler-rt has to return zero.

--renato

> The current implementations actually return 0. Can you point out where
that
> doesn't hold please?

With the current implementation...

int foo(int a) {
  return __aeabi_idiv0(a);
}

returns 'a', while:

int bar(int a) {
  return __aeabi_idiv(a, 0);
}

returns zero.

> Quoting the RTABI:
>
> int __aeabi_idiv0(int return_value);
> long long __aeabi_ldiv0(long long return_value);
>
> The *div0 functions:
>
> 􏰁 Return the value passed to them as a parameter.
>
> Is my copy out of date?

As I said before, all three behaviours are allowed by the RTABI
(exception, constant, parameter), but we now have two different
behaviours in compiler-rt. This is a regression.

We must make it consistent to what there was there before (return
zero). If you *really* want to make it return the parameter or signal,
we should discuss this properly, on another thread, but right now,
division by zero in compiler-rt has to return zero.

Oh! I think I see what the confusion is all about. __aeabi_[il]div0 are
*NOT* division routines. They are handlers for the error case of division
by zero. They do not perform a division by zero, they effectively are
meant to trap the fact that a division by zero has occurred.

Hi Saleem,

No, that's not the confusion. The {i,l}div0 functions are public and
can be called from anywhere in user code. Let me give you a better
example:

struct div_res {int quo, int rem };

struct div_res my_divmod(int a, int b) {
  struct div_res result;
  result.quo = a/b;
  result.rem = a%b;
  return div;
}

If 'b' is zero, my_divmod, when linking with compiler-rt, will return
zero. Now, a "cautious" developer has a great idea! Let me make this
function safer!

struct div_res {int quo, int rem };

struct div_res my_divmod(int a, int b) {
if (b == 0)
    return __aeabi_idiv0(a);

  struct div_res result;
  result.quo = a/b;
  result.rem = a%b;
  return div;
}

my_divmod will now, return 'a'.

That's not acceptable.

cheers,
--renato

After a chat with Saleem, I agree that the usage in this case is ok,
since the div0 is *meant* to be implemented if any consistent
behaviour is expected, otherwise, here be dragons. Also, in its lowest
form, div0 should be just a return, which in the case of ARM, leaves
R0 intact, so returning the argument makes a lot of sense.

Ignore my musings, all is well.

cheers,
--renato