Re: armv7 small XXX fix

[Artturi Alm]

On Wed, Jul 12, 2017 at 04:21:11PM -0400, Dale Rahn wrote:
> On Wed, Jul 12, 2017 at 11:06:23PM +0300, Artturi Alm wrote:
> > On Wed, Jul 12, 2017 at 06:12:34PM +0200, Mark Kettenis wrote:
> > > > Date: Mon, 10 Jul 2017 23:18:59 +0300
> > > > From: Artturi Alm 
> > > > 
> > > > Hi,
> > > > 
> > > > this does clutter my diffs, and the XXX comment is correct,
> > > 
> > > It probably isn't. None of the other architectures have those macros
> > > in their .
> > > 
> > 
> > Ok, didn't consider that, you're right and the diff withdrawn, but anyway,
> > what i was imprecisely after was that i'd prefer
> > having something like RODATA(name) found from sparc64/include/asm.h,
> > instead of the useless _C_LABEL() to use in armv7 md .S assembly, and
> > just because of the weird name i didn't find intuitive enough, i didn't
> > even suggest the arm64 EENTRY()..
> > Don't get me wrong, i'm more than happy to drop all the labeling
> > macros out of my diffs, and choose my self what is global and what is not,
> > while it's against the minimalism i'm now aiming at to even get the diffs
> > read when freetime does meet:)
> 
> _C_LABEL() is a holdover from the pre-ELF days. At one time (in NetBSD)
> this code still compiled using an a.out toolchain. In that era, it was
> defined to be _C_LABEL(x) _/**/x (this was also pre-ansi)
> (or _ ## x post ansi). On ELF _C_LABEL evaluates to just the symbol. It
> cannot have any association to either text or data because the proper use
> of it can refer to either text or data eg: bl _C_LABEL(c_func).
> 
> Basically it is a relic from the past, it might make sense to just remove
> all references to _C_LABEL() at this point and just the symbol itself.
> 
> Dale Rahn dr...@dalerahn.com

Oh, that explains what i was missing about 'why?', thank you for the info/
details.
At this point, given how harmless it is, i think cleaning up all at once
would be just churn, so i will consider it deprecated myself, and go w/o
it in my future diffs, which would touch any of them for other reasons.

-Artturi

Re: armv7 small XXX fix

[Dale Rahn]

On Wed, Jul 12, 2017 at 11:06:23PM +0300, Artturi Alm wrote:
> On Wed, Jul 12, 2017 at 06:12:34PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 10 Jul 2017 23:18:59 +0300
> > > From: Artturi Alm 
> > > 
> > > Hi,
> > > 
> > > this does clutter my diffs, and the XXX comment is correct,
> > 
> > It probably isn't. None of the other architectures have those macros
> > in their .
> > 
> 
> Ok, didn't consider that, you're right and the diff withdrawn, but anyway,
> what i was imprecisely after was that i'd prefer
> having something like RODATA(name) found from sparc64/include/asm.h,
> instead of the useless _C_LABEL() to use in armv7 md .S assembly, and
> just because of the weird name i didn't find intuitive enough, i didn't
> even suggest the arm64 EENTRY()..
> Don't get me wrong, i'm more than happy to drop all the labeling
> macros out of my diffs, and choose my self what is global and what is not,
> while it's against the minimalism i'm now aiming at to even get the diffs
> read when freetime does meet:)

_C_LABEL() is a holdover from the pre-ELF days. At one time (in NetBSD)
this code still compiled using an a.out toolchain. In that era, it was
defined to be _C_LABEL(x) _/**/x (this was also pre-ansi)
(or _ ## x post ansi). On ELF _C_LABEL evaluates to just the symbol. It
cannot have any association to either text or data because the proper use
of it can refer to either text or data eg: bl   _C_LABEL(c_func).

Basically it is a relic from the past, it might make sense to just remove
all references to _C_LABEL() at this point and just the symbol itself.

Dale Rahn   dr...@dalerahn.com

Re: armv7 small XXX fix

[Artturi Alm]

On Wed, Jul 12, 2017 at 06:12:34PM +0200, Mark Kettenis wrote:
> > Date: Mon, 10 Jul 2017 23:18:59 +0300
> > From: Artturi Alm 
> > 
> > Hi,
> > 
> > this does clutter my diffs, and the XXX comment is correct,
> 
> It probably isn't. None of the other architectures have those macros
> in their .
> 

Ok, didn't consider that, you're right and the diff withdrawn, but anyway,
what i was imprecisely after was that i'd prefer
having something like RODATA(name) found from sparc64/include/asm.h,
instead of the useless _C_LABEL() to use in armv7 md .S assembly, and
just because of the weird name i didn't find intuitive enough, i didn't
even suggest the arm64 EENTRY()..
Don't get me wrong, i'm more than happy to drop all the labeling
macros out of my diffs, and choose my self what is global and what is not,
while it's against the minimalism i'm now aiming at to even get the diffs
read when freetime does meet:)

-Artturi

> > currently used _C_LABEL() is nothing, and i find it's usage
> > directly rather pointless/weird, this does atleast make x .globl,
> > so there is benefit to the added characters in written code
> > be it _C_LABEL() or C_OBJECT() instead of just the label, that
> > i personally prefer, but given wide use of _C_LABEL() i thought
> > this might want fixing so i wont get told to use _C_LABEL() or
> > anything for my diffs...
> > 
> > here is define for _C_LABEL:
> > #define _C_LABEL(x) x
> > 
> > _C_LABEL() is used over 60 times in sys/arch/arm/arm/*.S atm.
> > (C_OBJECT() only 4 times under the #define.
> > if this goes in, there miight be easy cleanup to be done for
> > anyone who does care. there might be even unused variables around:)
> > 
> > -Artturi
> > 
> > 
> > diff --git a/sys/arch/arm/arm/cpufunc_asm_armv7.S 
> > b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> > index 05679df15fa..7b71652c4dc 100644
> > --- a/sys/arch/arm/arm/cpufunc_asm_armv7.S
> > +++ b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> > @@ -234,10 +234,6 @@ ENTRY(armv7_context_switch)
> > isb sy
> > mov pc, lr
> >  
> > -/* XXX The following macros should probably be moved to asm.h */
> > -#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> > -#define C_OBJECT(x)_DATA_OBJECT(_C_LABEL(x))
> > -
> > .align 2
> >  C_OBJECT(armv7_dcache_sets_max)
> > .word   0
> > diff --git a/sys/arch/arm/include/asm.h b/sys/arch/arm/include/asm.h
> > index e1e5bbc4dd2..7f9af03c7a5 100644
> > --- a/sys/arch/arm/include/asm.h
> > +++ b/sys/arch/arm/include/asm.h
> > @@ -64,6 +64,9 @@
> >  #define _ENTRY(x) \
> > .text; _ALIGN_TEXT; .globl x; .type x,_ASM_TYPE_FUNCTION; x:
> >  
> > +#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> > +#define C_OBJECT(x)_DATA_OBJECT(_C_LABEL(x))
> > +
> >  #if defined(PROF) || defined(GPROF)
> >  # define _PROF_PROLOGUE\
> > mov ip, lr; bl __mcount
> > 
> > 

Re: armv7 small XXX fix

[Mark Kettenis]

> Date: Mon, 10 Jul 2017 23:18:59 +0300
> From: Artturi Alm 
> 
> Hi,
> 
> this does clutter my diffs, and the XXX comment is correct,

It probably isn't. None of the other architectures have those macros
in their .

> currently used _C_LABEL() is nothing, and i find it's usage
> directly rather pointless/weird, this does atleast make x .globl,
> so there is benefit to the added characters in written code
> be it _C_LABEL() or C_OBJECT() instead of just the label, that
> i personally prefer, but given wide use of _C_LABEL() i thought
> this might want fixing so i wont get told to use _C_LABEL() or
> anything for my diffs...
> 
> here is define for _C_LABEL:
> #define _C_LABEL(x)   x
> 
> _C_LABEL() is used over 60 times in sys/arch/arm/arm/*.S atm.
> (C_OBJECT() only 4 times under the #define.
> if this goes in, there miight be easy cleanup to be done for
> anyone who does care. there might be even unused variables around:)
> 
> -Artturi
> 
> 
> diff --git a/sys/arch/arm/arm/cpufunc_asm_armv7.S 
> b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> index 05679df15fa..7b71652c4dc 100644
> --- a/sys/arch/arm/arm/cpufunc_asm_armv7.S
> +++ b/sys/arch/arm/arm/cpufunc_asm_armv7.S
> @@ -234,10 +234,6 @@ ENTRY(armv7_context_switch)
>   isb sy
>   mov pc, lr
>  
> -/* XXX The following macros should probably be moved to asm.h */
> -#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> -#define C_OBJECT(x)  _DATA_OBJECT(_C_LABEL(x))
> -
>   .align 2
>  C_OBJECT(armv7_dcache_sets_max)
>   .word   0
> diff --git a/sys/arch/arm/include/asm.h b/sys/arch/arm/include/asm.h
> index e1e5bbc4dd2..7f9af03c7a5 100644
> --- a/sys/arch/arm/include/asm.h
> +++ b/sys/arch/arm/include/asm.h
> @@ -64,6 +64,9 @@
>  #define _ENTRY(x) \
>   .text; _ALIGN_TEXT; .globl x; .type x,_ASM_TYPE_FUNCTION; x:
>  
> +#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
> +#define C_OBJECT(x)  _DATA_OBJECT(_C_LABEL(x))
> +
>  #if defined(PROF) || defined(GPROF)
>  # define _PROF_PROLOGUE  \
>   mov ip, lr; bl __mcount
> 
> 

armv7 small XXX fix

[Artturi Alm]

Hi,

this does clutter my diffs, and the XXX comment is correct,
currently used _C_LABEL() is nothing, and i find it's usage
directly rather pointless/weird, this does atleast make x .globl,
so there is benefit to the added characters in written code
be it _C_LABEL() or C_OBJECT() instead of just the label, that
i personally prefer, but given wide use of _C_LABEL() i thought
this might want fixing so i wont get told to use _C_LABEL() or
anything for my diffs...

here is define for _C_LABEL:
#define _C_LABEL(x) x

_C_LABEL() is used over 60 times in sys/arch/arm/arm/*.S atm.
(C_OBJECT() only 4 times under the #define.
if this goes in, there miight be easy cleanup to be done for
anyone who does care. there might be even unused variables around:)

-Artturi


diff --git a/sys/arch/arm/arm/cpufunc_asm_armv7.S 
b/sys/arch/arm/arm/cpufunc_asm_armv7.S
index 05679df15fa..7b71652c4dc 100644
--- a/sys/arch/arm/arm/cpufunc_asm_armv7.S
+++ b/sys/arch/arm/arm/cpufunc_asm_armv7.S
@@ -234,10 +234,6 @@ ENTRY(armv7_context_switch)
isb sy
mov pc, lr
 
-/* XXX The following macros should probably be moved to asm.h */
-#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
-#define C_OBJECT(x)_DATA_OBJECT(_C_LABEL(x))
-
.align 2
 C_OBJECT(armv7_dcache_sets_max)
.word   0
diff --git a/sys/arch/arm/include/asm.h b/sys/arch/arm/include/asm.h
index e1e5bbc4dd2..7f9af03c7a5 100644
--- a/sys/arch/arm/include/asm.h
+++ b/sys/arch/arm/include/asm.h
@@ -64,6 +64,9 @@
 #define _ENTRY(x) \
.text; _ALIGN_TEXT; .globl x; .type x,_ASM_TYPE_FUNCTION; x:
 
+#define _DATA_OBJECT(x) .globl x; .type x,_ASM_TYPE_OBJECT; x:
+#define C_OBJECT(x)_DATA_OBJECT(_C_LABEL(x))
+
 #if defined(PROF) || defined(GPROF)
 # define _PROF_PROLOGUE\
mov ip, lr; bl __mcount