public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC VTV] Fix VTV for targets that have section anchors.
@ 2015-10-09  9:18 Ramana Radhakrishnan
  2015-10-12 20:44 ` Jeff Law
  2015-10-13  9:26 ` Marcus Shawcroft
  0 siblings, 2 replies; 7+ messages in thread
From: Ramana Radhakrishnan @ 2015-10-09  9:18 UTC (permalink / raw)
  To: gcc-patches; +Cc: Caroline Tice, Marcus Shawcroft

[-- Attachment #1: Type: text/plain, Size: 2093 bytes --]

This started as a Friday afternoon project ... 

It turned out enabling VTV for AArch64 and ARM was a matter of fixing PR67868 which essentially comes from building libvtv with section anchors turned on. The problem was that the flow of control from output_object_block through to switch_section did not have the same special casing for the vtable section that exists in assemble_variable. 

Once this was done, I managed to build and test aarch64-none-linux-gnu with --enable-vtable-verify, a similar test was done for armhf. 

Testing showed no regressions in the gcc/ g++ testsuites for aarch64 and armhf
Testing showed no failures in libvtv testsuite for aarch64 but a few more failures - see below.
Testing showed 2 failures in libstdc++-v3 testsuite compared to without vtable verification.

FAIL: libstdc++-abi/abi_check
FAIL: experimental/filesystem/iterators/directory_iterator.cc execution test

However both these failures also occur on x86_64 - so I'm content to declare victory on AArch64
as far as basic enablement goes.

On ARM I see the following failures that I still need to debug - I can see that the failure is because the write to _ZN4_VTVI1BE12__vtable_mapE does not elicit a SEGV but I need to go further than that.

FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O0 -fvtable-verify=std execution test
FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O2 -fvtable-verify=std execution test
FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O0 -fvtable-verify=preinit execution test
FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O2 -fvtable-verify=preinit execution test


Questions -

1. Are the generic changes to varasm.c ok ?
2. Can we take the AArch64 support in now, given this amount of testing ? Marcus / Caroline ?
3. Any suggestions / helpful debug hints for VTV debugging (other than turning VTV_DEBUG on and inspecting trace) ?

There's an arm*-*-* hunk there but I'm easy about applying that right now and figuring out the issues
over time. In case we don't fix it for 6.0 we can rip the support out before release.


Thanks,
Ramana

P.S. (Yes, I'll provide a Changelog :) )

[-- Attachment #2: vtv.txt --]
[-- Type: text/plain, Size: 2330 bytes --]

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 48c3662..c182cd4 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -2236,7 +2236,7 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED,
          code is removed, the variables end up in the same section
          with a single comdat name.
 
-         FIXME:  resolve_unique_section needs to deal better with
+	 FIXME:  resolve_unique_section needs to deal better with
          decls with both DECL_SECTION_NAME and DECL_ONE_ONLY.  Once
          that is fixed, this if-else statement can be replaced with
          a single call to "switch_to_section (sect)".  */
@@ -7329,7 +7329,33 @@ output_object_block (struct object_block *block)
 
   /* Switch to the section and make sure that the first byte is
      suitably aligned.  */
-  switch_to_section (block->sect);
+  /* The following bit of code ensures that vtable_map
+     variables are not only in the comdat section, but that
+     each variable has its own unique comdat name.  If this
+     code is removed, the variables end up in the same section
+     with a single comdat name.
+
+     FIXME:  resolve_unique_section needs to deal better with
+     decls with both DECL_SECTION_NAME and DECL_ONE_ONLY.  Once
+     that is fixed, this if-else statement can be replaced with
+     a single call to "switch_to_section (sect)".  */
+  if (SECTION_STYLE (block->sect) == SECTION_NAMED
+      && block->sect->named.name
+      && (strcmp (block->sect->named.name, ".vtable_map_vars") == 0))
+    {
+#if defined (OBJECT_FORMAT_ELF)
+      targetm.asm_out.named_section (block->sect->named.name,
+				     block->sect->named.common.flags
+				     | SECTION_LINKONCE,
+				     DECL_NAME (block->sect->named.decl));
+      in_section = block->sect;
+#else
+      switch_to_section (block->sect);
+#endif
+    }
+  else
+    switch_to_section (block->sect);
+
   assemble_align (block->alignment);
 
   /* Define the values of all anchors relative to the current section
diff --git a/libvtv/configure.tgt b/libvtv/configure.tgt
index e543371..adcff5c 100644
--- a/libvtv/configure.tgt
+++ b/libvtv/configure.tgt
@@ -37,6 +37,10 @@ case "${target}" in
   sparc*-*-linux*)
 	;;
   arm*-*-linux*)
+	VTV_SUPPORTED=yes
+	;;
+  aarch64*-*-linux*)
+	VTV_SUPPORTED=yes
 	;;
   x86_64-*-darwin[1]* | i?86-*-darwin[1]*)
 	;;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC VTV] Fix VTV for targets that have section anchors.
  2015-10-09  9:18 [RFC VTV] Fix VTV for targets that have section anchors Ramana Radhakrishnan
@ 2015-10-12 20:44 ` Jeff Law
  2015-10-13 12:53   ` Ramana Radhakrishnan
  2015-10-13  9:26 ` Marcus Shawcroft
  1 sibling, 1 reply; 7+ messages in thread
From: Jeff Law @ 2015-10-12 20:44 UTC (permalink / raw)
  To: Ramana Radhakrishnan, gcc-patches; +Cc: Caroline Tice, Marcus Shawcroft

On 10/09/2015 03:17 AM, Ramana Radhakrishnan wrote:
> This started as a Friday afternoon project ...
>
> It turned out enabling VTV for AArch64 and ARM was a matter of fixing
> PR67868 which essentially comes from building libvtv with section
> anchors turned on. The problem was that the flow of control from
> output_object_block through to switch_section did not have the same
> special casing for the vtable section that exists in
> assemble_variable.
That's some ugly code.  You might consider factoring that code into a 
function and just calling it from both places.  Your version doesn't 
seem to handle PECOFF, so I'd probably refactor from assemble_variable.

>
> However both these failures also occur on x86_64 - so I'm content to
> declare victory on AArch64 as far as basic enablement goes.
Cool.

>
> 1. Are the generic changes to varasm.c ok ? 2. Can we take the
> AArch64 support in now, given this amount of testing ? Marcus /
> Caroline ? 3. Any suggestions / helpful debug hints for VTV debugging
> (other than turning VTV_DEBUG on and inspecting trace) ?
I think that with refactoring they'd be good to go.  No opinions on the 
AArch64 specific question -- call for the AArch64 maintainers.

Good to see someone hacking on vtv.  It's in my queue to look at as well.

jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC VTV] Fix VTV for targets that have section anchors.
  2015-10-09  9:18 [RFC VTV] Fix VTV for targets that have section anchors Ramana Radhakrishnan
  2015-10-12 20:44 ` Jeff Law
@ 2015-10-13  9:26 ` Marcus Shawcroft
  1 sibling, 0 replies; 7+ messages in thread
From: Marcus Shawcroft @ 2015-10-13  9:26 UTC (permalink / raw)
  To: Ramana Radhakrishnan, gcc-patches; +Cc: Caroline Tice

On 09/10/15 10:17, Ramana Radhakrishnan wrote:
> This started as a Friday afternoon project ...
>
> It turned out enabling VTV for AArch64 and ARM was a matter of fixing PR67868 which essentially comes from building libvtv with section anchors turned on. The problem was that the flow of control from output_object_block through to switch_section did not have the same special casing for the vtable section that exists in assemble_variable.
>
> Once this was done, I managed to build and test aarch64-none-linux-gnu with --enable-vtable-verify, a similar test was done for armhf.
>
> Testing showed no regressions in the gcc/ g++ testsuites for aarch64 and armhf
> Testing showed no failures in libvtv testsuite for aarch64 but a few more failures - see below.
> Testing showed 2 failures in libstdc++-v3 testsuite compared to without vtable verification.
>
> FAIL: libstdc++-abi/abi_check
> FAIL: experimental/filesystem/iterators/directory_iterator.cc execution test
>
> However both these failures also occur on x86_64 - so I'm content to declare victory on AArch64
> as far as basic enablement goes.
>
> On ARM I see the following failures that I still need to debug - I can see that the failure is because the write to _ZN4_VTVI1BE12__vtable_mapE does not elicit a SEGV but I need to go further than that.
>
> FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O0 -fvtable-verify=std execution test
> FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O2 -fvtable-verify=std execution test
> FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O0 -fvtable-verify=preinit execution test
> FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O2 -fvtable-verify=preinit execution test
>
>
> Questions -
>
> 1. Are the generic changes to varasm.c ok ?
> 2. Can we take the AArch64 support in now, given this amount of testing ? Marcus / Caroline ?

+  aarch64*-*-linux*)
+	VTV_SUPPORTED=yes
  	;;

Ramana, Go ahead an add the aarch64 enable once you have the 
pre-requisite varasm changes approved.

Cheers
/Marcus

> 3. Any suggestions / helpful debug hints for VTV debugging (other than turning VTV_DEBUG on and inspecting trace) ?
>
> There's an arm*-*-* hunk there but I'm easy about applying that right now and figuring out the issues
> over time. In case we don't fix it for 6.0 we can rip the support out before release.
>
>
> Thanks,
> Ramana
>
> P.S. (Yes, I'll provide a Changelog :) )
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC VTV] Fix VTV for targets that have section anchors.
  2015-10-12 20:44 ` Jeff Law
@ 2015-10-13 12:53   ` Ramana Radhakrishnan
  2015-10-19  9:18     ` Ramana Radhakrishnan
  2015-10-20  6:46     ` Jeff Law
  0 siblings, 2 replies; 7+ messages in thread
From: Ramana Radhakrishnan @ 2015-10-13 12:53 UTC (permalink / raw)
  To: Jeff Law, gcc-patches; +Cc: Caroline Tice, Marcus Shawcroft

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]




On 12/10/15 21:44, Jeff Law wrote:
> On 10/09/2015 03:17 AM, Ramana Radhakrishnan wrote:
>> This started as a Friday afternoon project ...
>>
>> It turned out enabling VTV for AArch64 and ARM was a matter of fixing
>> PR67868 which essentially comes from building libvtv with section
>> anchors turned on. The problem was that the flow of control from
>> output_object_block through to switch_section did not have the same
>> special casing for the vtable section that exists in
>> assemble_variable.
> That's some ugly code.  You might consider factoring that code into a function and just calling it from both places.  Your version doesn't seem to handle PECOFF, so I'd probably refactor from assemble_variable.
> 

I was a bit lazy as I couldn't immediately think of a target that would want PECOFF, section anchors and VTV. That combination seems to be quite rare, anyway point taken on the refactor.

Ok if no regressions ?

>>
>> However both these failures also occur on x86_64 - so I'm content to
>> declare victory on AArch64 as far as basic enablement goes.
> Cool.
> 
>>
>> 1. Are the generic changes to varasm.c ok ? 2. Can we take the
>> AArch64 support in now, given this amount of testing ? Marcus /
>> Caroline ? 3. Any suggestions / helpful debug hints for VTV debugging
>> (other than turning VTV_DEBUG on and inspecting trace) ?
> I think that with refactoring they'd be good to go.  No opinions on the AArch64 specific question -- call for the AArch64 maintainers.
> 
> Good to see someone hacking on vtv.  It's in my queue to look at as well.

Yeah figuring out more about vtv is also in my background queue.

regards
Ramana

PR other/67868

* varasm.c (assemble_variable): Move special vtv handling to..
(handle_vtv_comdat_sections): .. here. New function.
(output_object_block): Handle vtv sections.

libvtv/Changelog

* configure.tgt: Support aarch64 and arm.

[-- Attachment #2: vtv-rework.txt --]
[-- Type: text/plain, Size: 5882 bytes --]

diff --git a/gcc/varasm.c b/gcc/varasm.c
index f1564bc..62ad863 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -127,6 +127,7 @@ static void asm_output_aligned_bss (FILE *, tree, const char *,
 #endif /* BSS_SECTION_ASM_OP */
 static void mark_weak (tree);
 static void output_constant_pool (const char *, tree);
+static void handle_vtv_comdat_section (section *, const_tree);
 \f
 /* Well-known sections, each one associated with some sort of *_ASM_OP.  */
 section *text_section;
@@ -2230,56 +2231,10 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED,
     assemble_noswitch_variable (decl, name, sect, align);
   else
     {
-      /* The following bit of code ensures that vtable_map 
-         variables are not only in the comdat section, but that
-         each variable has its own unique comdat name.  If this
-         code is removed, the variables end up in the same section
-         with a single comdat name.
-
-         FIXME:  resolve_unique_section needs to deal better with
-         decls with both DECL_SECTION_NAME and DECL_ONE_ONLY.  Once
-         that is fixed, this if-else statement can be replaced with
-         a single call to "switch_to_section (sect)".  */
+      /* Special-case handling of vtv comdat sections.  */
       if (sect->named.name
 	  && (strcmp (sect->named.name, ".vtable_map_vars") == 0))
-	{
-#if defined (OBJECT_FORMAT_ELF)
-          targetm.asm_out.named_section (sect->named.name,
-					 sect->named.common.flags
-				         | SECTION_LINKONCE,
-			    	         DECL_NAME (decl));
-          in_section = sect;
-#elif defined (TARGET_PECOFF)
-          /* Neither OBJECT_FORMAT_PE, nor OBJECT_FORMAT_COFF is set here.
-             Therefore the following check is used.
-             In case a the target is PE or COFF a comdat group section
-             is created, e.g. .vtable_map_vars$foo. The linker places
-             everything in .vtable_map_vars at the end.
-
-             A fix could be made in
-             gcc/config/i386/winnt.c: i386_pe_unique_section. */
-          if (TARGET_PECOFF)
-          {
-            char *name;
-            
-            if (TREE_CODE (DECL_NAME (decl)) == IDENTIFIER_NODE)
-              name = ACONCAT ((sect->named.name, "$",
-                               IDENTIFIER_POINTER (DECL_NAME (decl)), NULL));
-            else
-              name = ACONCAT ((sect->named.name, "$",
-                    IDENTIFIER_POINTER (DECL_COMDAT_GROUP (DECL_NAME (decl))),
-                    NULL));
-
-            targetm.asm_out.named_section (name,
-                                           sect->named.common.flags
-                                           | SECTION_LINKONCE,
-                                           DECL_NAME (decl));
-            in_section = sect;
-        }
-#else
-          switch_to_section (sect);
-#endif
-        }
+	handle_vtv_comdat_section (sect, decl);
       else
 	switch_to_section (sect);
       if (align > BITS_PER_UNIT)
@@ -7329,7 +7284,14 @@ output_object_block (struct object_block *block)
 
   /* Switch to the section and make sure that the first byte is
      suitably aligned.  */
-  switch_to_section (block->sect);
+  /* Special case VTV comdat sections similar to assemble_variable.  */
+  if (SECTION_STYLE (block->sect) == SECTION_NAMED
+      && block->sect->named.name
+      && (strcmp (block->sect->named.name, ".vtable_map_vars") == 0))
+    handle_vtv_comdat_section (block->sect, block->sect->named.decl);
+  else
+    switch_to_section (block->sect);
+
   assemble_align (block->alignment);
 
   /* Define the values of all anchors relative to the current section
@@ -7772,4 +7734,56 @@ default_asm_output_ident_directive (const char *ident_str)
     fprintf (asm_out_file, "%s\"%s\"\n", ident_asm_op, ident_str);
 }
 
+
+/* This function ensures that vtable_map variables are not only
+   in the comdat section, but that each variable has its own unique
+   comdat name.  Without this the variables end up in the same section
+   with a single comdat name.
+
+   FIXME:  resolve_unique_section needs to deal better with
+   decls with both DECL_SECTION_NAME and DECL_ONE_ONLY.  Once
+   that is fixed, this if-else statement can be replaced with
+   a single call to "switch_to_section (sect)".  */
+
+static void
+handle_vtv_comdat_section (section *sect, const_tree decl)
+{
+#if defined (OBJECT_FORMAT_ELF)
+  targetm.asm_out.named_section (sect->named.name,
+				 sect->named.common.flags
+				 | SECTION_LINKONCE,
+				 DECL_NAME (decl));
+  in_section = sect;
+#elif defined (TARGET_PECOFF)
+  /* Neither OBJECT_FORMAT_PE, nor OBJECT_FORMAT_COFF is set here.
+     Therefore the following check is used.
+     In case a the target is PE or COFF a comdat group section
+     is created, e.g. .vtable_map_vars$foo. The linker places
+     everything in .vtable_map_vars at the end.
+
+     A fix could be made in
+     gcc/config/i386/winnt.c: i386_pe_unique_section.  */
+  if (TARGET_PECOFF)
+    {
+      char *name;
+
+      if (TREE_CODE (DECL_NAME (decl)) == IDENTIFIER_NODE)
+	name = ACONCAT ((sect->named.name, "$",
+			 IDENTIFIER_POINTER (DECL_NAME (decl)), NULL));
+      else
+	name = ACONCAT ((sect->named.name, "$",
+			 IDENTIFIER_POINTER (DECL_COMDAT_GROUP (DECL_NAME (decl))),
+			 NULL));
+
+      targetm.asm_out.named_section (name,
+				     sect->named.common.flags
+				     | SECTION_LINKONCE,
+				     DECL_NAME (decl));
+      in_section = sect;
+    }
+#else
+  switch_to_section (sect);
+#endif
+}
+
 #include "gt-varasm.h"
diff --git a/libvtv/configure.tgt b/libvtv/configure.tgt
index e543371..adcff5c 100644
--- a/libvtv/configure.tgt
+++ b/libvtv/configure.tgt
@@ -37,6 +37,10 @@ case "${target}" in
   sparc*-*-linux*)
 	;;
   arm*-*-linux*)
+	VTV_SUPPORTED=yes
+	;;
+  aarch64*-*-linux*)
+	VTV_SUPPORTED=yes
 	;;
   x86_64-*-darwin[1]* | i?86-*-darwin[1]*)
 	;;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC VTV] Fix VTV for targets that have section anchors.
  2015-10-13 12:53   ` Ramana Radhakrishnan
@ 2015-10-19  9:18     ` Ramana Radhakrishnan
       [not found]       ` <CABtf2+R1LjiwTsRivzZ-PbpQjmVy0=XXHUdcHWNsOGwqi75bQA@mail.gmail.com>
  2015-10-20  6:46     ` Jeff Law
  1 sibling, 1 reply; 7+ messages in thread
From: Ramana Radhakrishnan @ 2015-10-19  9:18 UTC (permalink / raw)
  To: Ramana Radhakrishnan
  Cc: Jeff Law, gcc-patches, Caroline Tice, Marcus Shawcroft

On Tue, Oct 13, 2015 at 1:53 PM, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:
>
>
>
> On 12/10/15 21:44, Jeff Law wrote:
>> On 10/09/2015 03:17 AM, Ramana Radhakrishnan wrote:
>>> This started as a Friday afternoon project ...
>>>
>>> It turned out enabling VTV for AArch64 and ARM was a matter of fixing
>>> PR67868 which essentially comes from building libvtv with section
>>> anchors turned on. The problem was that the flow of control from
>>> output_object_block through to switch_section did not have the same
>>> special casing for the vtable section that exists in
>>> assemble_variable.
>> That's some ugly code.  You might consider factoring that code into a function and just calling it from both places.  Your version doesn't seem to handle PECOFF, so I'd probably refactor from assemble_variable.
>>
>
> I was a bit lazy as I couldn't immediately think of a target that would want PECOFF, section anchors and VTV. That combination seems to be quite rare, anyway point taken on the refactor.
>
> Ok if no regressions ?

Ping.

Ramana

>
>>>
>>> However both these failures also occur on x86_64 - so I'm content to
>>> declare victory on AArch64 as far as basic enablement goes.
>> Cool.
>>
>>>
>>> 1. Are the generic changes to varasm.c ok ? 2. Can we take the
>>> AArch64 support in now, given this amount of testing ? Marcus /
>>> Caroline ? 3. Any suggestions / helpful debug hints for VTV debugging
>>> (other than turning VTV_DEBUG on and inspecting trace) ?
>> I think that with refactoring they'd be good to go.  No opinions on the AArch64 specific question -- call for the AArch64 maintainers.
>>
>> Good to see someone hacking on vtv.  It's in my queue to look at as well.
>
> Yeah figuring out more about vtv is also in my background queue.
>
> regards
> Ramana
>
> PR other/67868
>
> * varasm.c (assemble_variable): Move special vtv handling to..
> (handle_vtv_comdat_sections): .. here. New function.
> (output_object_block): Handle vtv sections.
>
> libvtv/Changelog
>
> * configure.tgt: Support aarch64 and arm.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC VTV] Fix VTV for targets that have section anchors.
       [not found]       ` <CABtf2+R1LjiwTsRivzZ-PbpQjmVy0=XXHUdcHWNsOGwqi75bQA@mail.gmail.com>
@ 2015-10-19 23:49         ` Caroline Tice
  0 siblings, 0 replies; 7+ messages in thread
From: Caroline Tice @ 2015-10-19 23:49 UTC (permalink / raw)
  To: Ramana Radhakrishnan
  Cc: Ramana Radhakrishnan, Jeff Law, gcc-patches, Marcus Shawcroft

Sending this again, as the mailer daemon rejected it last time....

It looks good to me, but I can only approve the files that go into libvtv.

In (belated) response to your earlier question about debugging vtv
problems, there's a fair amount of useful info for debugging in the
User's Guide, off the wiki page (https://gcc.gnu.org/wiki/vtv).   If
you're already read that and have further questions, let me know...


-- Caroline
cmtice@google.com


On Mon, Oct 19, 2015 at 4:39 PM, Caroline Tice <cmtice@google.com> wrote:
> It looks good to me, but I can only approve the files that go into libvtv.
>
> In (belated) response to your earlier question about debugging vtv problems,
> there's a fair amount of useful info for debugging in the User's Guide, off
> the wiki page (https://gcc.gnu.org/wiki/vtv).   If you're already read that
> and have further questions, let me know...
>
>
> -- Caroline
> cmtice@google.com
>
> On Mon, Oct 19, 2015 at 1:54 AM, Ramana Radhakrishnan
> <ramana.gcc@googlemail.com> wrote:
>>
>> On Tue, Oct 13, 2015 at 1:53 PM, Ramana Radhakrishnan
>> <ramana.radhakrishnan@foss.arm.com> wrote:
>> >
>> >
>> >
>> > On 12/10/15 21:44, Jeff Law wrote:
>> >> On 10/09/2015 03:17 AM, Ramana Radhakrishnan wrote:
>> >>> This started as a Friday afternoon project ...
>> >>>
>> >>> It turned out enabling VTV for AArch64 and ARM was a matter of fixing
>> >>> PR67868 which essentially comes from building libvtv with section
>> >>> anchors turned on. The problem was that the flow of control from
>> >>> output_object_block through to switch_section did not have the same
>> >>> special casing for the vtable section that exists in
>> >>> assemble_variable.
>> >> That's some ugly code.  You might consider factoring that code into a
>> >> function and just calling it from both places.  Your version doesn't seem to
>> >> handle PECOFF, so I'd probably refactor from assemble_variable.
>> >>
>> >
>> > I was a bit lazy as I couldn't immediately think of a target that would
>> > want PECOFF, section anchors and VTV. That combination seems to be quite
>> > rare, anyway point taken on the refactor.
>> >
>> > Ok if no regressions ?
>>
>> Ping.
>>
>> Ramana
>>
>> >
>> >>>
>> >>> However both these failures also occur on x86_64 - so I'm content to
>> >>> declare victory on AArch64 as far as basic enablement goes.
>> >> Cool.
>> >>
>> >>>
>> >>> 1. Are the generic changes to varasm.c ok ? 2. Can we take the
>> >>> AArch64 support in now, given this amount of testing ? Marcus /
>> >>> Caroline ? 3. Any suggestions / helpful debug hints for VTV debugging
>> >>> (other than turning VTV_DEBUG on and inspecting trace) ?
>> >> I think that with refactoring they'd be good to go.  No opinions on the
>> >> AArch64 specific question -- call for the AArch64 maintainers.
>> >>
>> >> Good to see someone hacking on vtv.  It's in my queue to look at as
>> >> well.
>> >
>> > Yeah figuring out more about vtv is also in my background queue.
>> >
>> > regards
>> > Ramana
>> >
>> > PR other/67868
>> >
>> > * varasm.c (assemble_variable): Move special vtv handling to..
>> > (handle_vtv_comdat_sections): .. here. New function.
>> > (output_object_block): Handle vtv sections.
>> >
>> > libvtv/Changelog
>> >
>> > * configure.tgt: Support aarch64 and arm.
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC VTV] Fix VTV for targets that have section anchors.
  2015-10-13 12:53   ` Ramana Radhakrishnan
  2015-10-19  9:18     ` Ramana Radhakrishnan
@ 2015-10-20  6:46     ` Jeff Law
  1 sibling, 0 replies; 7+ messages in thread
From: Jeff Law @ 2015-10-20  6:46 UTC (permalink / raw)
  To: Ramana Radhakrishnan, gcc-patches; +Cc: Caroline Tice, Marcus Shawcroft

On 10/13/2015 06:53 AM, Ramana Radhakrishnan wrote:
>
>
>
> On 12/10/15 21:44, Jeff Law wrote:
>> On 10/09/2015 03:17 AM, Ramana Radhakrishnan wrote:
>>> This started as a Friday afternoon project ...
>>>
>>> It turned out enabling VTV for AArch64 and ARM was a matter of fixing
>>> PR67868 which essentially comes from building libvtv with section
>>> anchors turned on. The problem was that the flow of control from
>>> output_object_block through to switch_section did not have the same
>>> special casing for the vtable section that exists in
>>> assemble_variable.
>> That's some ugly code.  You might consider factoring that code into a function and just calling it from both places.  Your version doesn't seem to handle PECOFF, so I'd probably refactor from assemble_variable.
>>
>
> I was a bit lazy as I couldn't immediately think of a target that would want PECOFF, section anchors and VTV. That combination seems to be quite rare, anyway point taken on the refactor.
>
> Ok if no regressions ?
>
>>>
>>> However both these failures also occur on x86_64 - so I'm content to
>>> declare victory on AArch64 as far as basic enablement goes.
>> Cool.
>>
>>>
>>> 1. Are the generic changes to varasm.c ok ? 2. Can we take the
>>> AArch64 support in now, given this amount of testing ? Marcus /
>>> Caroline ? 3. Any suggestions / helpful debug hints for VTV debugging
>>> (other than turning VTV_DEBUG on and inspecting trace) ?
>> I think that with refactoring they'd be good to go.  No opinions on the AArch64 specific question -- call for the AArch64 maintainers.
>>
>> Good to see someone hacking on vtv.  It's in my queue to look at as well.
>
> Yeah figuring out more about vtv is also in my background queue.
>
> regards
> Ramana
>
> PR other/67868
>
> * varasm.c (assemble_variable): Move special vtv handling to..
> (handle_vtv_comdat_sections): .. here. New function.
> (output_object_block): Handle vtv sections.
>
> libvtv/Changelog
>
> * configure.tgt: Support aarch64 and arm.
OK for the trunk.  Sorry for the delay, I was out on the PTO the latter 
half of last week.

jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-10-20  5:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-09  9:18 [RFC VTV] Fix VTV for targets that have section anchors Ramana Radhakrishnan
2015-10-12 20:44 ` Jeff Law
2015-10-13 12:53   ` Ramana Radhakrishnan
2015-10-19  9:18     ` Ramana Radhakrishnan
     [not found]       ` <CABtf2+R1LjiwTsRivzZ-PbpQjmVy0=XXHUdcHWNsOGwqi75bQA@mail.gmail.com>
2015-10-19 23:49         ` Caroline Tice
2015-10-20  6:46     ` Jeff Law
2015-10-13  9:26 ` Marcus Shawcroft

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).