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][Revised3] Fix PR41313 with dual approach


On Thu, Oct 08, 2009 at 09:26:09AM -0700, Mike Stump wrote:
>
> Yes, have_named_sections is always true, that part can be simplified.   
> Please disable the 10.6 checking for now, I don't want a linker warning 
> by default.  For example, if the linker removes the unwind information 
> because there is no label when dead stripping, you'd not figure that out 
> until some really obscure code that should gave been able to be thrown 
> through, failed.  To a poor user, the warning _looks_ scary.  Better to 
> wait until the issue is understood and resolved before going down that 
> path.
>

So you don't want to omit the unwind labels on 10.6, correct?
In that case the first part of the patch disappears.

>> I added flag_profile_use to the conditional since
>> -fprofile-use appears to cause break and abort() statements to
>> trigger the emission of duplicate .eh labels for the gcc.dg/tree- 
>> prof/bb-reorg.c
>> and gcc.dg/tree-prof/pr34999.c test cases.
>
> I think it is wrong to use just flag_profile_use here.  That alters  
> codegen and there is an invariant that codegen can't be altered between 
> generation and use.  So, at a minimum, the profile generate flag would 
> have to be used as well.  But, it isn't clear to me why these fail.  I 
> think there is some other more basic issue going on, and that should be 
> the flag used, not the profile flag.  Conservatively, absent the correct 
> flag to use, true would be safer to use, that would just always turn off 
> partitioning.
>
> And last, the entire 10.6 checking code down in the routine is a  
> duplicate of the one at the top.  If the one at the top is true, the  
> routine returns, therefore we know that the second instance _must_ be  
> false.  !false simplifies to true, and true &&  
> flag_reorder_blocks_and_partition simplifies to just  
> flag_reorder_blocks_and_partition.

So reduced patch would look something like...

Index: gcc/config/darwin.c
===================================================================
--- gcc/config/darwin.c (revision 152573)
+++ gcc/config/darwin.c (working copy)
@@ -1697,6 +1697,16 @@
   if (dwarf_strict < 0)
     dwarf_strict = 1;

+  /* Disable -freorder-blocks-and-partition for exception handling or when
+     the target requested unwind info.  */
+  if (flag_reorder_blocks_and_partition && (flag_exceptions || flag_unwind_tables))
+    {
+      inform (input_location,
+              "-freorder-blocks-and-partition does not work with exceptions on this architecture");
+      flag_reorder_blocks_and_partition = 0;
+      flag_reorder_blocks = 1;
+    }
+
   if (flag_mkernel || flag_apple_kext)
     {
       /* -mkernel implies -fapple-kext for C++ */

If we just ignore the failures in gcc.dg/tree- prof/bb-reorg.c and gcc.dg/tree-prof/pr34999.c
for now and concentrate on restoring the behavior pre-r150553 to darwin. Is this more
acceptable?
                   Jack


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