On Sat, Oct 14, 2017 at 01:16:52AM +0000, [email protected] wrote:
> More of my unprofessional opinion since we'll need all the help we can
> get...
>
> +void *
> +dtrace_casptr(volatile void *target, volatile void *cmp, volatile void *new)
> +*/
> +EENTRY(dtrace_casptr)
> +ENTRY(dtrace_cas32)
> +#if __ARM_ARCH >= 6
> +
> +1: ldrex r3, [r0] /* Load target */
> + cmp r3, r1 /* Check if *target == cmp */
> + bne 2f /* No, return */
> + strex ip, r2, [r0] /* Store new to target */
> + cmp ip, #0 /* Did the store succeed? */
> + bne 1b /* No, try again */
> +2: mov r0, r3 /* Return the value loaded from target */
> + RET
> +
>
> ..
>
> Surely we have a function like this already, does dtrace really
> need its own asm implementation>?
dtrace has separate copies of a number of functions that are used during
the execution of a probe, so that the original function can be probed
without creating an infinite loop.
the __ARM_ARCH >= 6 version of this code is unchanged from upstream,
and the other version isn't likely to change ever, so I don't think
this is worth doing differently.
> +static int
> +log2(int size)
> +{
> + switch (size) {
> + case 1: return (0);
> + case 2: return (1);
> + case 4: return (2);
> + case 8: return (3);
> + }
> + return (0);
> +}
>
> Using the name of a standard C function differently sounds dangerous
this function is static so it only affects this one file. this file is
very unlikely to ever want to use the libm log2() that operates on doubles,
so it doesn't seem worth diverging from upstream for this.
in general I agree with you about both of these issues, but I want to take
a pretty firm stance about not diverging from upstream for the CDDL code
unless it's really necessary.
-Chuck
