This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch i386]: Add support of "hotfix" -feature for x64


On Jul 7, 2010, at 6:42 AM, Jack Howarth wrote:
>   While I can test any revised patch you come up with, the changes to
> gcc/config/darwin.h really needs to be approved by the Darwin maintainer
> (Mike Stump) before a revised patch is reapplied.

I think the design of the change is busted.  I'd go back to the drawing board, and have someone design in a way to make the change that doesn't break everything and so that the change can be trivially audited for correctness.  If people want to rejigger how the port/ports are written to detangle and make the code more clear and prevent this type of breakage in the future, that would be stellar.

I briefly looked at it, but it seems to want to change the use of ASM_OUTPUT_LABEL by ASM_DECLARE_FUNCTION_NAME  in the x86 ports to instead use ASM_OUTPUT_HOTFIX_LABEL.  Just do that, and then define ASM_OUTPUT_HOTFIX_LABEL in i386.h.  Isn't that simpler?

So, to be concrete, the darwin change would be something like:

--- config/darwin.h.~1~	2010-07-01 07:47:13.000000000 -0700
+++ config/darwin.h	2010-07-08 13:18:10.000000000 -0700
@@ -628,7 +628,7 @@ extern GTY(()) int darwin_ms_struct;
 	 && (!DECL_COMMON (DECL) || !TREE_PUBLIC (DECL)))		\
         || DECL_INITIAL (DECL))						\
       (* targetm.encode_section_info) (DECL, DECL_RTL (DECL), false);	\
-    ASM_OUTPUT_LABEL (FILE, xname);					\
+    ASM_OUTPUT_HOTFIX_LABEL (FILE, xname, DECL);			\
     /* Darwin doesn't support zero-size objects, so give them a		\
        byte.  */							\
     if (tree_low_cst (DECL_SIZE_UNIT (DECL), 1) == 0)			\
@@ -649,7 +649,7 @@ extern GTY(()) int darwin_ms_struct;
 	 && (!DECL_COMMON (DECL) || !TREE_PUBLIC (DECL)))		\
         || DECL_INITIAL (DECL))						\
       (* targetm.encode_section_info) (DECL, DECL_RTL (DECL), false);	\
-    ASM_OUTPUT_LABEL (FILE, xname);					\
+    ASM_OUTPUT_HOTFIX_LABEL (FILE, xname, DECL);					\
   } while (0)
 
 #undef TARGET_ASM_DECLARE_CONSTANT_NAME
--------------

which, to the unaided eye, is trivial to review.  Then, ASM_OUTPUT_HOTFIX_LABEL takes the responsibility to  get the details right, which, I suspect would be easy enough to do...  ?


I'd rather have the x86 port maintainer review and approve this type of patch.  I'm fine with people using the darwin tester to figure out if it worked or not after the fact, if they want, but they do have to watch it and resolve issues.  And I'm fine with a port reviewer's decisions...


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]