On Mon, 18 May 2015, Jan Hubicka wrote:
> Hi,
> this patch adds basic checking of TYPE_CANOINCAL. It checkes TYPE_CANONICAL
> forms a tree and it moves lto's gimple_canonical_types_compatible_p back to
> middle-end and uses it to check that TYPE_CANONICAL is compatible and thus
> none
> of the FEs produce types sharing TYPE_CANONICAL that would be considered
> different by LTO's TBAA.
>
> I added trust_type_canonical argument and changed:
>
> - /* If the types have been previously registered and found equal
> - they still are. */
> - if (TYPE_CANONICAL (t1)
> - && TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2))
> - return true;
>
> to:
>
> + /* If the types have been previously registered and found equal
> + they still are. */
> + if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t2)
> + && trust_type_canonical)
> + return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
>
> (notice that I blocked the recursion when TYPE_CANONICAL is known to be
> different).
>
> The patch originally triggered an ICE in testsuite becuase C++ FE builds
> FUNCTION_TYPE type variant with different attributes but same TYPE_CANONICAL.
> I think C++ FE is correct and we may want to drop:
>
> + if (!comp_type_attributes (t1, t2))
> + return false;
>
> Because I think the TBAA does not care about attribute lists. I suppose this
> is kind-of harmless because of:
I think that was about attributes like 'packed', but those should have
their effects applied at stor-layout time. So yes, I think this can be
dropped.
> + /* For canonical type comparisons we do not want to build SCCs
> + so we cannot compare pointed-to types. But we can, for now,
> + require the same pointed-to type kind and match what
> + useless_type_conversion_p would do. */
> + if (POINTER_TYPE_P (t1))
> + {
> + if (TYPE_ADDR_SPACE (TREE_TYPE (t1))
> + != TYPE_ADDR_SPACE (TREE_TYPE (t2)))
> + return false;
> +
> + if (TREE_CODE (TREE_TYPE (t1)) != TREE_CODE (TREE_TYPE (t2)))
> + return false;
> + }
>
> Is the reason for not making differences between differnt pointer types
> really just lazyness to not deal with SCCs? For that it is easy to add
> set of visited type pairs, just like odr_types_equivalent_p does.
Well - TBAA doesn't care about pointed-to types. See get_alias_set
else if (POINTER_TYPE_P (t)
&& t != ptr_type_node)
set = get_alias_set (ptr_type_node);
so the above would at most matter for pointer-type fields of aggregates.
And no, we don't want to deal with SCCs - I doubt it would bring any
benefit and the issue here is that I am worried about SCCs being present
dependent on the order of streaming (well, the code pre-dates SCC
streaming).
> Because we now stream in SCC order anyway, most of time we won't even
> need to populate it. Shall I implement this?
You can certainly try - but I think for cross-language interoperability
we want to treat
struct X { void *p; }
struct Y { void *p; }
and
struct X { Y *p; }
struct Y { X *q; }
as compatible so I don't think we can use SCCs to partition alias sets
further (that is, we _do_ have to treat pointers conservatively). The
current code is as far as we can get IMHO (well, even the current code
is too strict in making struct { void *p } and struct { int *p } not
have the same alias-set.
> Other issue I run into is that for Ada bootstrap we have variadic type whose
> canonical types are having different temporary set as size. I think this is
> valid and perhaps gimple_canonical_types_compatible_p should consider
> variadic arrays to be compatible with any array of compatible type?
Those are all local types and thus the strict equality compare should be
ok? Not sure if we can do in C sth like
void foo (int n)
{
struct { int i; int a[n]; } a;
struct { int i; int a[n]; } b;
}
and if we'd have to assign the same alias-sets to 'a' and 'b'.
> I am not quite convinced we get variadic types right at LTO time, because
> they bypass canonical type calculation anyway and their canonical type
> is set by TYPE_MAIN_VARIANT in lto_read_body_or_constructor which I think
> is not safe. I will look for a testcase.
That is because if they are streamed locally they do not enter type
merging, but they still go via gimple_register_canonical_type, so I'm not
sure where you see they always get their main variant as canonical type.
> I also added check that TYPE_CANONICAL agrees for type variants. I think it
> should because few times in the middle-end uses TYPE_MAIN_VARIANT and seem to
> expect this to happen:
Yes. Otherwise we'd run in circles for code that does
type = TYPE_CANONICAL (TYPE_MAIN_VARIANT (type));
because the result might not be a main variant. That's something to
verify (both TYPE_CANONICAL and TYPE_MAIN_VARIANT should form a tree).
> static inline int
>
> same_type_for_tbaa (tree type1, tree type2)
>
> {
>
> type1 = TYPE_MAIN_VARIANT (type1);
>
> type2 = TYPE_MAIN_VARIANT (type2);
>
>
>
> /* If we would have to do structural comparison bail out. */
>
> if (TYPE_STRUCTURAL_EQUALITY_P (type1)
>
> || TYPE_STRUCTURAL_EQUALITY_P (type2))
>
> return -1;
>
>
>
> /* Compare the canonical types. */
>
> if (TYPE_CANONICAL (type1) == TYPE_CANONICAL (type2))
>
> return 1;
>
>
> and some places where we look for TYPE_MAIN_VARIANT to discard qualifiers.
> This check triggers for Ada, but I will look into it separately.
>
> Bootstrapped/regtested ppc64-linux, LTO bootstrap in progress. OK?
>
> Honza
>
> * lto/lto.c (gimple_canonical_types_compatible_p): Move to tree.c
> * tree.c (verify_type_variant): Fix #undef.
> (gimple_canonical_types_compatible_p): Move here from lto.c
> (verify_type): Verify TYPE_CANONICAL compatibility.
tree.h change is missing.
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c (revision 223260)
> +++ lto/lto.c (working copy)
> @@ -441,208 +441,6 @@ gimple_canonical_type_hash (const void *
> }
>
>
> -/* The TYPE_CANONICAL merging machinery. It should closely resemble
> - the middle-end types_compatible_p function. It needs to avoid
> - claiming types are different for types that should be treated
> - the same with respect to TBAA. Canonical types are also used
> - for IL consistency checks via the useless_type_conversion_p
> - predicate which does not handle all type kinds itself but falls
> - back to pointer-comparison of TYPE_CANONICAL for aggregates
> - for example. */
> -
> -/* Return true iff T1 and T2 are structurally identical for what
> - TBAA is concerned. */
> -
> -static bool
> -gimple_canonical_types_compatible_p (tree t1, tree t2)
> -{
> - /* Before starting to set up the SCC machinery handle simple cases. */
> -
> - /* Check first for the obvious case of pointer identity. */
> - if (t1 == t2)
> - return true;
> -
> - /* Check that we have two types to compare. */
> - if (t1 == NULL_TREE || t2 == NULL_TREE)
> - return false;
> -
> - /* If the types have been previously registered and found equal
> - they still are. */
> - if (TYPE_CANONICAL (t1)
> - && TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2))
> - return true;
> -
> - /* Can't be the same type if the types don't have the same code. */
> - if (TREE_CODE (t1) != TREE_CODE (t2))
> - return false;
> -
> - /* Qualifiers do not matter for canonical type comparison purposes. */
> -
> - /* Void types and nullptr types are always the same. */
> - if (TREE_CODE (t1) == VOID_TYPE
> - || TREE_CODE (t1) == NULLPTR_TYPE)
> - return true;
> -
> - /* Can't be the same type if they have different mode. */
> - if (TYPE_MODE (t1) != TYPE_MODE (t2))
> - return false;
> -
> - /* Non-aggregate types can be handled cheaply. */
> - if (INTEGRAL_TYPE_P (t1)
> - || SCALAR_FLOAT_TYPE_P (t1)
> - || FIXED_POINT_TYPE_P (t1)
> - || TREE_CODE (t1) == VECTOR_TYPE
> - || TREE_CODE (t1) == COMPLEX_TYPE
> - || TREE_CODE (t1) == OFFSET_TYPE
> - || POINTER_TYPE_P (t1))
> - {
> - /* Can't be the same type if they have different sign or precision. */
> - if (TYPE_PRECISION (t1) != TYPE_PRECISION (t2)
> - || TYPE_UNSIGNED (t1) != TYPE_UNSIGNED (t2))
> - return false;
> -
> - if (TREE_CODE (t1) == INTEGER_TYPE
> - && TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2))
> - return false;
> -
> - /* For canonical type comparisons we do not want to build SCCs
> - so we cannot compare pointed-to types. But we can, for now,
> - require the same pointed-to type kind and match what
> - useless_type_conversion_p would do. */
> - if (POINTER_TYPE_P (t1))
> - {
> - if (TYPE_ADDR_SPACE (TREE_TYPE (t1))
> - != TYPE_ADDR_SPACE (TREE_TYPE (t2)))
> - return false;
> -
> - if (TREE_CODE (TREE_TYPE (t1)) != TREE_CODE (TREE_TYPE (t2)))
> - return false;
> - }
> -
> - /* Tail-recurse to components. */
> - if (TREE_CODE (t1) == VECTOR_TYPE
> - || TREE_CODE (t1) == COMPLEX_TYPE)
> - return gimple_canonical_types_compatible_p (TREE_TYPE (t1),
> - TREE_TYPE (t2));
> -
> - return true;
> - }
> -
> - /* Do type-specific comparisons. */
> - switch (TREE_CODE (t1))
> - {
> - case ARRAY_TYPE:
> - /* Array types are the same if the element types are the same and
> - the number of elements are the same. */
> - if (!gimple_canonical_types_compatible_p (TREE_TYPE (t1), TREE_TYPE
> (t2))
> - || TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2)
> - || TYPE_NONALIASED_COMPONENT (t1) != TYPE_NONALIASED_COMPONENT (t2))
> - return false;
> - else
> - {
> - tree i1 = TYPE_DOMAIN (t1);
> - tree i2 = TYPE_DOMAIN (t2);
> -
> - /* For an incomplete external array, the type domain can be
> - NULL_TREE. Check this condition also. */
> - if (i1 == NULL_TREE && i2 == NULL_TREE)
> - return true;
> - else if (i1 == NULL_TREE || i2 == NULL_TREE)
> - return false;
> - else
> - {
> - tree min1 = TYPE_MIN_VALUE (i1);
> - tree min2 = TYPE_MIN_VALUE (i2);
> - tree max1 = TYPE_MAX_VALUE (i1);
> - tree max2 = TYPE_MAX_VALUE (i2);
> -
> - /* The minimum/maximum values have to be the same. */
> - if ((min1 == min2
> - || (min1 && min2
> - && ((TREE_CODE (min1) == PLACEHOLDER_EXPR
> - && TREE_CODE (min2) == PLACEHOLDER_EXPR)
> - || operand_equal_p (min1, min2, 0))))
> - && (max1 == max2
> - || (max1 && max2
> - && ((TREE_CODE (max1) == PLACEHOLDER_EXPR
> - && TREE_CODE (max2) == PLACEHOLDER_EXPR)
> - || operand_equal_p (max1, max2, 0)))))
> - return true;
> - else
> - return false;
> - }
> - }
> -
> - case METHOD_TYPE:
> - case FUNCTION_TYPE:
> - /* Function types are the same if the return type and arguments types
> - are the same. */
> - if (!gimple_canonical_types_compatible_p (TREE_TYPE (t1), TREE_TYPE
> (t2)))
> - return false;
> -
> - if (!comp_type_attributes (t1, t2))
> - return false;
> -
> - if (TYPE_ARG_TYPES (t1) == TYPE_ARG_TYPES (t2))
> - return true;
> - else
> - {
> - tree parms1, parms2;
> -
> - for (parms1 = TYPE_ARG_TYPES (t1), parms2 = TYPE_ARG_TYPES (t2);
> - parms1 && parms2;
> - parms1 = TREE_CHAIN (parms1), parms2 = TREE_CHAIN (parms2))
> - {
> - if (!gimple_canonical_types_compatible_p
> - (TREE_VALUE (parms1), TREE_VALUE (parms2)))
> - return false;
> - }
> -
> - if (parms1 || parms2)
> - return false;
> -
> - return true;
> - }
> -
> - case RECORD_TYPE:
> - case UNION_TYPE:
> - case QUAL_UNION_TYPE:
> - {
> - tree f1, f2;
> -
> - /* For aggregate types, all the fields must be the same. */
> - for (f1 = TYPE_FIELDS (t1), f2 = TYPE_FIELDS (t2);
> - f1 || f2;
> - f1 = TREE_CHAIN (f1), f2 = TREE_CHAIN (f2))
> - {
> - /* Skip non-fields. */
> - while (f1 && TREE_CODE (f1) != FIELD_DECL)
> - f1 = TREE_CHAIN (f1);
> - while (f2 && TREE_CODE (f2) != FIELD_DECL)
> - f2 = TREE_CHAIN (f2);
> - if (!f1 || !f2)
> - break;
> - /* The fields must have the same name, offset and type. */
> - if (DECL_NONADDRESSABLE_P (f1) != DECL_NONADDRESSABLE_P (f2)
> - || !gimple_compare_field_offset (f1, f2)
> - || !gimple_canonical_types_compatible_p
> - (TREE_TYPE (f1), TREE_TYPE (f2)))
> - return false;
> - }
> -
> - /* If one aggregate has more fields than the other, they
> - are not the same. */
> - if (f1 || f2)
> - return false;
> -
> - return true;
> - }
> -
> - default:
> - gcc_unreachable ();
> - }
> -}
> -
>
> /* Returns nonzero if P1 and P2 are equal. */
>
> Index: tree.c
> ===================================================================
> --- tree.c (revision 223260)
> +++ tree.c (working copy)
> @@ -12687,10 +12687,222 @@ verify_type_variant (const_tree t, tree
> return false;
> }
> return true;
> -#undef verify_type_variant
> +#undef verify_variant_match
> }
>
>
> +/* The TYPE_CANONICAL merging machinery. It should closely resemble
> + the middle-end types_compatible_p function. It needs to avoid
> + claiming types are different for types that should be treated
> + the same with respect to TBAA. Canonical types are also used
> + for IL consistency checks via the useless_type_conversion_p
> + predicate which does not handle all type kinds itself but falls
> + back to pointer-comparison of TYPE_CANONICAL for aggregates
> + for example. */
> +
> +/* Return true iff T1 and T2 are structurally identical for what
> + TBAA is concerned. */
> +
> +bool
> +gimple_canonical_types_compatible_p (const_tree t1, const_tree t2,
> + bool trust_type_canonical)
> +{
> + /* Before starting to set up the SCC machinery handle simple cases. */
> +
> + /* Check first for the obvious case of pointer identity. */
> + if (t1 == t2)
> + return true;
> +
> + /* Check that we have two types to compare. */
> + if (t1 == NULL_TREE || t2 == NULL_TREE)
> + return false;
> +
> + /* If the types have been previously registered and found equal
> + they still are. */
> + if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t2)
> + && trust_type_canonical)
> + return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
> +
> + /* Can't be the same type if the types don't have the same code. */
> + if (TREE_CODE (t1) != TREE_CODE (t2))
> + return false;
> +
> + /* Qualifiers do not matter for canonical type comparison purposes. */
> +
> + /* Void types and nullptr types are always the same. */
> + if (TREE_CODE (t1) == VOID_TYPE
> + || TREE_CODE (t1) == NULLPTR_TYPE)
> + return true;
> +
> + /* Can't be the same type if they have different mode. */
> + if (TYPE_MODE (t1) != TYPE_MODE (t2))
> + return false;
> +
> + /* Non-aggregate types can be handled cheaply. */
> + if (INTEGRAL_TYPE_P (t1)
> + || SCALAR_FLOAT_TYPE_P (t1)
> + || FIXED_POINT_TYPE_P (t1)
> + || TREE_CODE (t1) == VECTOR_TYPE
> + || TREE_CODE (t1) == COMPLEX_TYPE
> + || TREE_CODE (t1) == OFFSET_TYPE
> + || POINTER_TYPE_P (t1))
> + {
> + /* Can't be the same type if they have different sign or precision. */
> + if (TYPE_PRECISION (t1) != TYPE_PRECISION (t2)
> + || TYPE_UNSIGNED (t1) != TYPE_UNSIGNED (t2))
> + return false;
> +
> + if (TREE_CODE (t1) == INTEGER_TYPE
> + && TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2))
> + return false;
> +
> + /* For canonical type comparisons we do not want to build SCCs
> + so we cannot compare pointed-to types. But we can, for now,
> + require the same pointed-to type kind and match what
> + useless_type_conversion_p would do. */
> + if (POINTER_TYPE_P (t1))
> + {
> + if (TYPE_ADDR_SPACE (TREE_TYPE (t1))
> + != TYPE_ADDR_SPACE (TREE_TYPE (t2)))
> + return false;
> +
> + if (TREE_CODE (TREE_TYPE (t1)) != TREE_CODE (TREE_TYPE (t2)))
> + return false;
> + }
> +
> + /* Tail-recurse to components. */
> + if (TREE_CODE (t1) == VECTOR_TYPE
> + || TREE_CODE (t1) == COMPLEX_TYPE)
> + return gimple_canonical_types_compatible_p (TREE_TYPE (t1),
> + TREE_TYPE (t2),
> + trust_type_canonical);
> +
> + return true;
> + }
> +
> + /* Do type-specific comparisons. */
> + switch (TREE_CODE (t1))
> + {
> + case ARRAY_TYPE:
> + /* Array types are the same if the element types are the same and
> + the number of elements are the same. */
> + if (!gimple_canonical_types_compatible_p (TREE_TYPE (t1), TREE_TYPE
> (t2),
> + trust_type_canonical)
> + || TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2)
> + || TYPE_NONALIASED_COMPONENT (t1) != TYPE_NONALIASED_COMPONENT (t2))
> + return false;
> + else
> + {
> + tree i1 = TYPE_DOMAIN (t1);
> + tree i2 = TYPE_DOMAIN (t2);
> +
> + /* For an incomplete external array, the type domain can be
> + NULL_TREE. Check this condition also. */
> + if (i1 == NULL_TREE && i2 == NULL_TREE)
> + return true;
> + else if (i1 == NULL_TREE || i2 == NULL_TREE)
> + return false;
> + else
> + {
> + tree min1 = TYPE_MIN_VALUE (i1);
> + tree min2 = TYPE_MIN_VALUE (i2);
> + tree max1 = TYPE_MAX_VALUE (i1);
> + tree max2 = TYPE_MAX_VALUE (i2);
> +
> + /* The minimum/maximum values have to be the same. */
> + if ((min1 == min2
> + || (min1 && min2
> + && ((TREE_CODE (min1) == PLACEHOLDER_EXPR
> + && TREE_CODE (min2) == PLACEHOLDER_EXPR)
> + || operand_equal_p (min1, min2, 0))))
> + && (max1 == max2
> + || (max1 && max2
> + && ((TREE_CODE (max1) == PLACEHOLDER_EXPR
> + && TREE_CODE (max2) == PLACEHOLDER_EXPR)
> + || operand_equal_p (max1, max2, 0)))))
> + return true;
> + else
> + return false;
> + }
> + }
> +
> + case METHOD_TYPE:
> + case FUNCTION_TYPE:
> + /* Function types are the same if the return type and arguments types
> + are the same. */
> + if (!gimple_canonical_types_compatible_p (TREE_TYPE (t1), TREE_TYPE
> (t2),
> + trust_type_canonical))
> + return false;
> +
> + if (!comp_type_attributes (t1, t2))
> + return false;
> +
> + if (TYPE_ARG_TYPES (t1) == TYPE_ARG_TYPES (t2))
> + return true;
> + else
> + {
> + tree parms1, parms2;
> +
> + for (parms1 = TYPE_ARG_TYPES (t1), parms2 = TYPE_ARG_TYPES (t2);
> + parms1 && parms2;
> + parms1 = TREE_CHAIN (parms1), parms2 = TREE_CHAIN (parms2))
> + {
> + if (!gimple_canonical_types_compatible_p
> + (TREE_VALUE (parms1), TREE_VALUE (parms2),
> + trust_type_canonical))
> + return false;
> + }
> +
> + if (parms1 || parms2)
> + return false;
> +
> + return true;
> + }
> +
> + case RECORD_TYPE:
> + case UNION_TYPE:
> + case QUAL_UNION_TYPE:
> + {
> + tree f1, f2;
> +
> + /* For aggregate types, all the fields must be the same. */
> + for (f1 = TYPE_FIELDS (t1), f2 = TYPE_FIELDS (t2);
> + f1 || f2;
> + f1 = TREE_CHAIN (f1), f2 = TREE_CHAIN (f2))
> + {
> + /* Skip non-fields. */
> + while (f1 && TREE_CODE (f1) != FIELD_DECL)
> + f1 = TREE_CHAIN (f1);
> + while (f2 && TREE_CODE (f2) != FIELD_DECL)
> + f2 = TREE_CHAIN (f2);
> + if (!f1 || !f2)
> + break;
> + /* The fields must have the same name, offset and type. */
> + if (DECL_NONADDRESSABLE_P (f1) != DECL_NONADDRESSABLE_P (f2)
> + || !gimple_compare_field_offset (f1, f2)
> + || !gimple_canonical_types_compatible_p
> + (TREE_TYPE (f1), TREE_TYPE (f2),
> + trust_type_canonical))
> + return false;
> + }
> +
> + /* If one aggregate has more fields than the other, they
> + are not the same. */
> + if (f1 || f2)
> + return false;
> +
> + return true;
> + }
> +
> + default:
> + /* Consider all types with language specific trees in them mutually
> + compatible. This is executed only from verify_type and false
> + positives can be tolerated. */
> + gcc_assert (!in_lto_p);
> + return true;
> + }
> +}
> +
> /* Verify type T. */
>
> void
> @@ -12712,6 +12924,35 @@ verify_type (const_tree t)
> else if (t != mv && !verify_type_variant (t, mv))
> error_found = true;
>
> + tree ct = TYPE_CANONICAL (t);
> + if (!ct)
> + ;
> + else if (TYPE_CANONICAL (t) != ct)
> + {
> + error ("TYPE_CANONICAL has different TYPE_CANONICAL");
> + debug_tree (ct);
> + error_found = true;
> + }
So here you'd verify that
TYPE_MAIN_VARIANT (ct) == ct
&& TYPE_CANONICAL (TYPE_MAIN_VARIANT (t)) == ct
> + /* Method and function types can not be used to address memory and thus
> + TYPE_CANONICAL really matters only for determining useless conversions.
> +
> + FIXME: C++ FE does not agree with gimple_canonical_types_compatible_p
> + here. gimple_canonical_types_compatible_p calls comp_type_attributes
> + while for C++ FE the attributes does not make difference. */
> + else if (TREE_CODE (t) == FUNCTION_TYPE || TREE_CODE (t) == METHOD_TYPE)
> + ;
> + else if (t != ct && !gimple_canonical_types_compatible_p (t, ct, false)
> + /* FIXME: gimple_canonical_types_compatible_p can not compare types
> + with variably sized arrays because their sizes possibly
> + gimplified to different variables. */
> + && !variably_modified_type_p (ct, NULL))
So with LTO this check should never trigger. I'd probably check
!variably_modified_type_p (ct, NULL) first.
Ok with that changes.
Thanks,
Richard.
> + {
> + error ("TYPE_CANONICAL is not compatible");
> + debug_tree (ct);
> + error_found = true;
> + }
> +
> +
> /* Check various uses of TYPE_MINVAL. */
> if (RECORD_OR_UNION_TYPE_P (t))
> {
> Index: tree.h
> ===================================================================
> --- tree.h (revision 223260)
> +++ tree.h (working copy)
> @@ -4564,6 +4564,8 @@ extern int tree_map_base_eq (const void
> extern unsigned int tree_map_base_hash (const void *);
> extern int tree_map_base_marked_p (const void *);
> extern void DEBUG_FUNCTION verify_type (const_tree t);
> +extern bool gimple_canonical_types_compatible_p (const_tree, const_tree,
> + bool trust_type_canonical =
> true);
>
> #define tree_map_eq tree_map_base_eq
> extern unsigned int tree_map_hash (const void *);
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham
Norton, HRB 21284 (AG Nuernberg)
