Patchwork =?UTF-8?Q?ObjC:=20revert=20addition=20of=20TARGET=5F64BIT=20to=20GNU=20r?= =?UTF-8?Q?untime=20code=20generation?=

login
register
mail settings
Submitter Nicola Pero
Date Feb. 19, 2011, 8:11 p.m.
Message ID <1298146292.656512484@192.168.4.58>
Download mbox | patch
Permalink /patch/83711/
State New
Headers show

Comments

Nicola Pero - Feb. 19, 2011, 8:11 p.m.
This patch reverts the usage of TARGET_64BIT in the generation of code for the
GNU Objective-C runtime, making it identical to what it was before the latest
big ObjC patch.  It also reverts the new dependency of GNU metadata generation
on tm.h.

I did look at the issues raised by Iain, in particular why he had a problem
on Apple 64-bit when removing TARGET_64BIT.  I also studied for a while why
Objective-C briefly regressed on i64 linux-gnu with the GNU runtime between
versions 170260 and 170268.

And Iain was right :-) - if compiling with -Wpadded on an LP64 machine, we do
have to silence the -Wpadded warnings generated by the automatic padding.  The
reason we didn't see these warnings is because the testsuite didn't have any test
that would trigger them.  At version 170260, because of the bug that would generate
meta-data even when there is nothing to output, a couple of testcases that use -Wpadded
but would normally generate no metadata started to do so, triggering unexpected
-Wpadded warnings (and hence regressions) on ia64 and Apple 64-bit.

This patch addresses this issue by simply setting input_location = BUILTINS_LOCATION
before starting to emit the runtime data structures.  This seems appropriate when
emitting metadata, and it automatically silences any -Wpadded warnings for these
particular structures (-Wpadded does not warn for structures where location is
BUILTINS_LOCATION).  Moreover, it is a low risk fix as in the worst case it would
break the location of some error messages (as opposed to the TARGET_64BIT change, which
was potentially changing, and hence breaking, code generation on some platforms).

A new testcase explicitly testing -Wpadded is also included.

Bootstrapped and regtested on i686 linux-gnu, ia64 linux-gnu and 32-bit apple.

Ok to commit ?

Thanks
Mike Stump - Feb. 20, 2011, 4:58 p.m.
On Feb 19, 2011, at 12:11 PM, Nicola Pero wrote:
> This patch reverts the usage of TARGET_64BIT in the generation of code for the
> GNU Objective-C runtime, making it identical to what it was before the latest
> big ObjC patch.  It also reverts the new dependency of GNU metadata generation
> on tm.h.

> Ok to commit ?

Ok, thanks.  Iain, if you spot any thing, let us know.

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 170320)
+++ ChangeLog   (working copy)
@@ -1,3 +1,10 @@ 
+2011-02-19  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc-gnu-runtime-abi-01.c (TARGET_64BIT): Removed.  Removed
+       usage of padding fields.  Do not include tm.h.
+       * objc-act.c (objc_write_global_declaration): Set input_location
+       to BUILTINS_LOCATION while generating runtime metadata.
+
 2011-01-19  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        * objc-next-runtime-abi-01.c: Updated comments.
@@ -124,7 +131,7 @@ 
        hook.  Added assert.  Use the get_class_super_ref and
        get_category_super_ref runtime hooks.
        (objc_v2_encode_prop_attr): New.
-       
+
 2011-01-17  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        PR objc/47314
Index: objc-act.c
===================================================================
--- objc-act.c  (revision 170320)
+++ objc-act.c  (working copy)
@@ -466,6 +466,8 @@  objc_write_global_declarations (void)
      and code if only checking syntax, or if generating a PCH file.  */
   if (!flag_syntax_only && !pch_file)
     {
+      location_t saved_location;
+
       /* If gen_declaration desired, open the output file.  */
       if (flag_gen_declaration)
        {
@@ -475,8 +477,24 @@  objc_write_global_declarations (void)
            fatal_error ("can%'t open %s: %m", dumpname);
          free (dumpname);
        }
+
+      /* Set the input location to BUILTINS_LOCATION.  This is good
+        for error messages, in case any is generated while producing
+        the metadata, but it also silences warnings that would be
+        produced when compiling with -Wpadded in case when padding is
+        automatically added to the built-in runtime data structure
+        declarations.  We know about this padding, and it is fine; we
+        don't want users to see any warnings about it if they use
+        -Wpadded.  */
+      saved_location = input_location;
+      input_location = BUILTINS_LOCATION;
+
       /* Compute and emit the meta-data tables for this runtime.  */
       (*runtime.generate_metadata) ();
+
+      /* Restore the original location, just in case it mattered.  */
+      input_location = saved_location;
+
       /* ... and then close any declaration file we opened.  */
       if (gen_declaration_file)
        fclose (gen_declaration_file);
Index: objc-gnu-runtime-abi-01.c
===================================================================
--- objc-gnu-runtime-abi-01.c   (revision 170320)
+++ objc-gnu-runtime-abi-01.c   (working copy)
@@ -21,7 +21,6 @@  along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
-#include "tm.h"
 #include "tree.h"
 
 #ifdef OBJCPLUS
@@ -83,11 +82,6 @@  along with GCC; see the file COPYING3.  If not see
   if (VERS)                                                            \
     DECL_ATTRIBUTES (DECL) = build_tree_list ((VERS), (KIND));
 
-/* FIXME: Remove this macro, not needed.  */
-#ifndef TARGET_64BIT
-#define TARGET_64BIT 0
-#endif
-
 static void gnu_runtime_01_initialize (void);
 
 static void build_selector_template (void);
@@ -1995,9 +1989,7 @@  build_objc_symtab_template (void)
   /* short cat_def_cnt; */
   add_field_decl (short_integer_type_node, "cat_def_cnt", &chain);
 
-  /* FIXME: Remove.  */
-  if (TARGET_64BIT)
-    add_field_decl (integer_type_node, "_explicit_padder", &chain);
+  /* Note that padding will be added here on LP64.  */
 
   /* void *defs[imp_count + cat_count (+ 1)]; */
   /* NB: The index is one less than the size of the array.  */
@@ -2043,19 +2035,9 @@  init_objc_symtab (tree type)
   CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, 
                          build_int_cst (short_integer_type_node, cat_count));
 
-  /* FIXME: Remove.  */
-  if (TARGET_64BIT)
-    CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
-                         build_int_cst (integer_type_node, 0));
-
   /* cls_def = { ..., { &Foo, &Bar, ...}, ... } */
 
   field = TYPE_FIELDS (type);
-
-  /* FIXME: Remove.  */
-  if (TARGET_64BIT)
-    field = DECL_CHAIN (field);
-
   field = DECL_CHAIN (DECL_CHAIN (DECL_CHAIN (DECL_CHAIN (field))));
 
   CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, init_def_list (TREE_TYPE (field)));
Index: ChangeLog
===================================================================
--- ChangeLog   (revision 170320)
+++ ChangeLog   (working copy)
@@ -1,3 +1,10 @@ 
+2011-02-19  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc.dg/layout-2.m: New.
+       * objc.dg/selector-3.m: Adjusted location of error message.
+       * objc.dg/type-size-3.m: Same.
+       * obj-c++.dg/selector-3.mm: Same.
+
 2011-02-19  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
            Tobias Burnus  <burnus@net-b.de>
 
@@ -31,7 +38,7 @@ 
 2011-02-18  Iain Sandoe  <iains@gcc.gnu.org>
 
        * objc/execute/exceptions/foward-1.x: New.
-       
+
 2011-02-18  Janus Weil  <janus@gcc.gnu.org>
 
        PR fortran/47789
Index: objc.dg/type-size-3.m
===================================================================
--- objc.dg/type-size-3.m       (revision 170320)
+++ objc.dg/type-size-3.m       (working copy)
@@ -15,4 +15,6 @@  typedef struct
 @end
 
 @implementation Test
-@end /* { dg-error "instance variable has unknown size" } */
+@end
+
+/* { dg-error "instance variable has unknown size" "" { target *-*-* } 0 } */
Index: objc.dg/layout-2.m
===================================================================
--- objc.dg/layout-2.m  (revision 0)
+++ objc.dg/layout-2.m  (revision 0)
@@ -0,0 +1,14 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, February 2011.  */
+/* Ensure that -Wpadded generates no warnings during runtime structure metadata
+   generation.  */
+/* { dg-do compile } */
+/* { dg-options "-Wpadded" } */
+
+#include "../objc-obj-c++-shared/Object1.h"
+
+/* Implement a class, so that the metadata generation happens.  */
+@interface MyClass : Object
+@end
+
+@implementation MyClass
+@end
Index: objc.dg/selector-3.m
===================================================================
--- objc.dg/selector-3.m        (revision 170320)
+++ objc.dg/selector-3.m        (working copy)
@@ -23,5 +23,8 @@  typedef const struct objc_selector    *SEL;
   a = @selector(b1ar);
   b = @selector(bar);
 }
-@end /* { dg-warning "creating selector for nonexistent method .b1ar." } */
+@end
 
+/* FIXME: The error message should be on the correct line.  */
+/* { dg-warning "creating selector for nonexistent method .b1ar." "" { target *-*-* } 0 } */
+
Index: obj-c++.dg/selector-3.mm
===================================================================
--- obj-c++.dg/selector-3.mm    (revision 170320)
+++ obj-c++.dg/selector-3.mm    (working copy)
@@ -22,5 +22,8 @@  typedef const struct objc_selector    *SEL;
   a = @selector(b1ar);
   b = @selector(bar);
 }
-@end /* { dg-warning "creating selector for nonexistent method .b1ar." } */
+@end
 
+/* FIXME: The error message should be on the correct line.  */
+/* { dg-warning "creating selector for nonexistent method .b1ar." "" { target *-*-* } 0 } */
+