Re: [Patch] Fix use of TARGET_VTABLE_USES_DESCRIPTORS in libgcov.c

[Steve Ellcey <sje at cup dot hp dot com>]

On Sun, 2010-10-31 at 16:45 -0700, Richard Henderson wrote:
> On 10/12/2010 10:45 AM, Steve Ellcey wrote:
> > +   VTABLE_USES_DESCRIPTORS is set to 0 if TARGET_VTABLE_USES_DESCRIPTORS
> > +   is not set by the target or 1 if it is.  This is needed because
> > +   TARGET_VTABLE_USES_DESCRIPTORS could depend on compiler flags and
> > +   cannot be used in libgcov.c.  */
> > +
> > +#ifdef TARGET_VTABLE_USES_DESCRIPTORS
> > +#define VTABLE_USES_DESCRIPTORS 1
> > +#else
> >  #define TARGET_VTABLE_USES_DESCRIPTORS 0
> > +#define VTABLE_USES_DESCRIPTORS 0
> >  #endif
> 
> Not ok for defaults.h.
> 
> It does seem like the smallest change would be an ifdef test in
> libgconv.c, e.g. moving the above sequence into libgconv.c.
> 
> A Proper Fix would seem to be providing a cpp-builtin define,
> but without adding some sort of -fbuilding-libgcc flag, I don't
> know if we want to export such a define to the users.
> 
> 
> r~

I can't just move the check to libgcov.c and leave defaults.h alone
because TARGET_VTABLE_USES_DESCRIPTORS will always be set once I am in
libgcov.c.  defaults.h will have set it to zero if it wasn't set by a
target.

It looks like, since TARGET_VTABLE_USES_DESCRIPTORS can be dependent on
target flags, the ifdef/define for it should be inside the '#ifdef
GCC_INSN_FLAGS_H' portion of default.h, then I could check inside
libgcov.c to see if TARGET_VTABLE_USES_DESCRIPTORS is or isn't set.
(Assuming no target is stupid enough to explicitly set
TARGET_VTABLE_USES_DESCRIPTORS to zero.)

Steve Ellcey
sje@cup.hp.com



How does this look:

2010-11-01  Steve Ellcey  <sje@cup.hp.com>

        * defaults.h (TARGET_VTABLE_USES_DESCRIPTORS): Move under ifdef
        GCC_INSN_FLAGS_H.
        * libgcov.c (__gcov_indirect_call_profiler): Set
        VTABLE_USES_DESCRIPTORS if TARGET_VTABLE_USES_DESCRIPTORS is
        defined and use in place of TARGET_VTABLE_USES_DESCRIPTORS.

Index: defaults.h
===================================================================
--- defaults.h  (revision 166120)
+++ defaults.h  (working copy)
@@ -813,16 +813,6 @@ see the files COPYING3 and COPYING.RUNTI
 #define TARGET_DEFAULT_PACK_STRUCT 0
 #endif
 
-/* By default, the C++ compiler will use function addresses in the
-   vtable entries.  Setting this nonzero tells the compiler to use
-   function descriptors instead.  The value of this macro says how
-   many words wide the descriptor is (normally 2).  It is assumed
-   that the address of a function descriptor may be treated as a
-   pointer to a function.  */
-#ifndef TARGET_VTABLE_USES_DESCRIPTORS
-#define TARGET_VTABLE_USES_DESCRIPTORS 0
-#endif
-
 /* By default, the vtable entries are void pointers, the so the alignment
    is the same as pointer alignment.  The value of this macro specifies
    the alignment of the vtable entry in bits.  It should be defined only
@@ -1381,6 +1371,16 @@ see the files COPYING3 and COPYING.RUNTI
 #define STACK_CHECK_MAX_VAR_SIZE (STACK_CHECK_MAX_FRAME_SIZE / 100)
 #endif
 
+/* By default, the C++ compiler will use function addresses in the
+   vtable entries.  Setting this nonzero tells the compiler to use
+   function descriptors instead.  The value of this macro says how
+   many words wide the descriptor is (normally 2).  It is assumed
+   that the address of a function descriptor may be treated as a
+   pointer to a function.  */
+#ifndef TARGET_VTABLE_USES_DESCRIPTORS
+#define TARGET_VTABLE_USES_DESCRIPTORS 0
+#endif
+
 #ifndef SWITCHABLE_TARGET
 #define SWITCHABLE_TARGET 0
 #endif
Index: libgcov.c
===================================================================
--- libgcov.c   (revision 166120)
+++ libgcov.c   (working copy)
@@ -798,6 +798,24 @@ __gcov_one_value_profiler (gcov_type *co
 #endif
 
 #ifdef L_gcov_indirect_call_profiler
+
+/* By default, the C++ compiler will use function addresses in the
+   vtable entries.  Setting TARGET_VTABLE_USES_DESCRIPTORS to nonzero
+   tells the compiler to use function descriptors instead.  The value
+   of this macro says how many words wide the descriptor is (normally 2).
+   but may be dependent on target flags.  Since we do not have access to
+   the target flags here we just check to see if it is set and use that
+   to set VTABLE_USES_DESCRIPTORS to 0 or 1.
+
+   It is assumed that the address of a function descriptor may be treated
+   as a pointer to a function.  */
+
+#ifdef TARGET_VTABLE_USES_DESCRIPTORS
+#define VTABLE_USES_DESCRIPTORS 1
+#else
+#define VTABLE_USES_DESCRIPTORS 0
+#endif
+
 /* Tries to determine the most common value among its inputs. */
 void
 __gcov_indirect_call_profiler (gcov_type* counter, gcov_type value,
@@ -807,7 +825,7 @@ __gcov_indirect_call_profiler (gcov_type
      function may have multiple descriptors and we need to dereference
      the descriptors to see if they point to the same function.  */
   if (cur_func == callee_func
-      || (TARGET_VTABLE_USES_DESCRIPTORS && callee_func
+      || (VTABLE_USES_DESCRIPTORS && callee_func
          && *(void **) cur_func == *(void **) callee_func))
     __gcov_one_value_profiler_body (counter, value);
 }