Re: [Xen-devel] [PATCH v9 2/5] xen: introduce SYMBOLS_SUBTRACT and SYMBOLS_COMPARE

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.02.19 at 02:17, <sstabellini@xxxxxxxxxx> wrote:
> On Tue, 12 Feb 2019, Jan Beulich wrote:
>> >>> On 12.02.19 at 13:01, <ian.jackson@xxxxxxxxxx> wrote:
>> > I would particularly welcome the opinion of hypervisor maintainers on
>> > my type safety point, below.
>> 
>> I agree with the requirements you put forward; I think I'd
>> prefer the inline function versions I had suggested (or
>> something similar) over macros though, not the least because
>> they come with "built-in" type safety, rather than grafted one
>> (by adding "pseudo" comparisons).
> 
> I don't mind the type checks in principle, I didn't add them to this
> version because, as I wrote in a previous email, with have occurrences
> of all three this possible calls:
> 
>   SYMBOL_COMPARE/SUBTRACT( symbol, symbol )
>   SYMBOL_COMPARE/SUBTRACT( symbol, non-symbol )
>   SYMBOL_COMPARE/SUBTRACT( non-symbol, symbol )
> 
> If you look through the patches you should be able to spot all three.
> As you know "non-symbol" and "symbol" are of different type:
> 
>   non-symbol would be like a "struct wombat*"
>   symbol would be like a "struct wombat[]"

These are declared types. Effective type when used as rvalue
(i.e. also when passed as arguments to functions) is struct
wombat * in both cases.

The important aspect (and new idea) Ian has been introducing
really is that the "end" symbols intentionally no longer be of
the same type as the start ones (but some type derived
thereof).

> However, it is not possible to have symbol as struct wombar[] and
> non-symbol as something entirely different like char*.
> 
> So, my question is: do we need three different variations of these
> macros for the types checks?
> 
> 
> I don't understand from IanJ email whether the suggestion is to change
> the type of all the linker symbols. If so, why are we doing this instead
> of the var.S approach? If we go and change the type of all the linker
> symbols in C-land, this series will become much bigger and at least as
> invasive as the var.S approach, but with added weird macros. It is kind
> of a lose - lose situation.
> 
> Similarly I would prefer to avoid Jan's proposed inline function approach
> because we have a few different array types for the linker symbols
> (vpci_register_init_t*, struct scheduler *, etc.), it looks far more
> work, and I am already waaaay over-budget for this series (as in 700%
> over budget). I would be very happy to "gift it" to somebody else
> willing to take it over :-)
> 
> Seriously, now that all the calls sites are marked appropriately and we
> all agree on the compare/subtract macro approach, it wouldn't be hard
> for somebody else to jump in and write the macros in their favorite way.
> Let me know if you would like to volunteer!

I've indeed been considering this already, as I expected the point
would come up sooner or later. Thing is though that, in particular
with Adrian not having replied at all so far, I'm still unconvinced that
we need to make this many changes (i.e. other than to work around
compiler deficiencies, which would boil down to using SYMBOL_HIDE()
from v7, but only in places where it is known certain compiler versions
might mis-optimize it, and with a clear reference to the involved
compiler bug/versions) at all. It's just that what we're now discussing
is the approach I have the least problem following _if_ such a global
"marking" of linker symbol uses ends up being necessary.

>> Furthermore - do we really need both a subtract and a compare
>> construct? The result subtract produces can be used for
>> comparison purposes as well, after all (just like all CPUs I know
>> details of implement [integer] compare as a subtract discarding
>> its numeric result, instead [or only] updating certain status flags).
> 
> No, we don't. In my first attempt I only had one macro. I am happy to
> follow your suggestion and keep only SUBTRACT.

Except that, as I think Ian has also suggested, DIFFERENCE() (or
SYMBOL_DIFFERENCE()) might be better, as it (hopefully)
reduces the connections to the - operator, and hence the risk
of possibly getting the argument order wrong. Otoh with the
type safety added wrong argument order would cause a
compile time error.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel