diff mbox

[sdbout] : Fix ICE on -debug testsuite test const2.C for coff

Message ID CAEwic4YufDreB7kqB2C=SOZ5nyrYRSdpxhApdQw6w1gxtD9K0w@mail.gmail.com
State New
Headers show

Commit Message

Kai Tietz Nov. 6, 2014, 7:37 p.m. UTC
Hi,

This fixes recent fallout of debug-tests on Windows target for sdbout
(coff) caused by an ICE.

ChangeLog

2014-11-06  Kai Tietz  <ktietz@redhat.com>

    * sdbout.c (sdbout_symbol): Eliminate register only
    if decl isn't a global variable.


Tested for i686-w64-mingw32, and x86_64-w64-mingw32.  Ok for apply?

Regards,
Kai

Comments

Jeff Law Nov. 7, 2014, 8:03 p.m. UTC | #1
On 11/06/14 12:37, Kai Tietz wrote:
> Hi,
>
> This fixes recent fallout of debug-tests on Windows target for sdbout
> (coff) caused by an ICE.
>
> ChangeLog
>
> 2014-11-06  Kai Tietz  <ktietz@redhat.com>
>
>      * sdbout.c (sdbout_symbol): Eliminate register only
>      if decl isn't a global variable.
Is there a testcase in the suite that triggers this problem?  If not, 
can you try to add one?    Out of curiosity, what was DECL_RTL here?

This is probably OK, but I'd really like to know what kind of goofy RTL 
we passed to eliminate_regs that caused it to fail.



Jeff
Kai Tietz Nov. 8, 2014, 10:27 a.m. UTC | #2
Hi Jeff,

2014-11-07 21:03 GMT+01:00 Jeff Law <law@redhat.com>:
> On 11/06/14 12:37, Kai Tietz wrote:
>>
>> Hi,
>>
>> This fixes recent fallout of debug-tests on Windows target for sdbout
>> (coff) caused by an ICE.
>>
>> ChangeLog
>>
>> 2014-11-06  Kai Tietz  <ktietz@redhat.com>
>>
>>      * sdbout.c (sdbout_symbol): Eliminate register only
>>      if decl isn't a global variable.
>
> Is there a testcase in the suite that triggers this problem?  If not, can
> you try to add one?    Out of curiosity, what was DECL_RTL here?

Sure, as (partial) mentioned in subject-line the testcase
g++.dg/debug/const2.C: Triggers this issue.  So there is no need to
add a separate testcase for it. Testcase fails for any use of -gcoff1
option.

> This is probably OK, but I'd really like to know what kind of goofy RTL we
> passed to eliminate_regs that caused it to fail.

I assume it is ok, as I just mimic dwarf2-behavior, which checks
before call for eliminate_regs, if rtl isn't a global-var itself.

As assumed by checking in debugger I see that eliminate_regs fails on rtx:
(mem/u/c:SI (symbol_ref:DI ("_ZN1b1dE") [flags 0x2] <var_decl 0xffbf0000 d>) [0
d+0 S4 A32])

>
>
> Jeff
>

So a more general question about sdbout.   Most prior dbg-coff targets
using nowadays dwarf2 too.  It seems to me that sdbout didn't got any
substantial maintenance the last years anymore.  Just fallout was
fixed.  AFAIK is coff (or was) mainly used nowadays for embedded
stuff, and in combination with some tools just supporting coff-dbg
information. Do we actually need to support it anymore?

Kai
Kai Tietz Nov. 11, 2014, 9:37 p.m. UTC | #3
Ping
Jeff Law Nov. 11, 2014, 11:10 p.m. UTC | #4
On 11/08/14 03:27, Kai Tietz wrote:
> Hi Jeff,
>
> 2014-11-07 21:03 GMT+01:00 Jeff Law <law@redhat.com>:
>> On 11/06/14 12:37, Kai Tietz wrote:
>>>
>>> Hi,
>>>
>>> This fixes recent fallout of debug-tests on Windows target for sdbout
>>> (coff) caused by an ICE.
>>>
>>> ChangeLog
>>>
>>> 2014-11-06  Kai Tietz  <ktietz@redhat.com>
>>>
>>>       * sdbout.c (sdbout_symbol): Eliminate register only
>>>       if decl isn't a global variable.
>>
>> Is there a testcase in the suite that triggers this problem?  If not, can
>> you try to add one?    Out of curiosity, what was DECL_RTL here?
>
> Sure, as (partial) mentioned in subject-line the testcase
> g++.dg/debug/const2.C: Triggers this issue.  So there is no need to
> add a separate testcase for it. Testcase fails for any use of -gcoff1
> option.
Doh, looked in the body, not the subject, my bad.

>
>> This is probably OK, but I'd really like to know what kind of goofy RTL we
>> passed to eliminate_regs that caused it to fail.
>
> I assume it is ok, as I just mimic dwarf2-behavior, which checks
> before call for eliminate_regs, if rtl isn't a global-var itself.
>
> As assumed by checking in debugger I see that eliminate_regs fails on rtx:
> (mem/u/c:SI (symbol_ref:DI ("_ZN1b1dE") [flags 0x2] <var_decl 0xffbf0000 d>) [0
> d+0 S4 A32])
Ok.  Thanks.  Patch is fine.

>
> So a more general question about sdbout.   Most prior dbg-coff targets
> using nowadays dwarf2 too.  It seems to me that sdbout didn't got any
> substantial maintenance the last years anymore.  Just fallout was
> fixed.  AFAIK is coff (or was) mainly used nowadays for embedded
> stuff, and in combination with some tools just supporting coff-dbg
> information. Do we actually need to support it anymore?
Most of the COFF targets were deprecated years ago.   The windows 
related targets (msdosdjgpp, cygwin, mingw, interix) still reference 
dbxcoff.  I think fixing those to no longer use dbxcoff, then removing 
config/dbxcoff is the first step.  Then we'd need to see if anything 
references arm/coff.h, if not fix that and remove arm/coff.h.

I think at that point sdbout becomes dead code, though you should 
probably check the IBM targets since they use xcoff.

We could go through the same exercise for stabs (dbxout0 :-)

jeff
diff mbox

Patch

Index: sdbout.c
===================================================================
--- sdbout.c    (Revision 217175)
+++ sdbout.c    (Arbeitskopie)
@@ -739,13 +739,16 @@  sdbout_symbol (tree decl, int local)
       if (!DECL_RTL_SET_P (decl))
     return;

-      SET_DECL_RTL (decl,
-            eliminate_regs (DECL_RTL (decl), VOIDmode, NULL_RTX));
+      value = DECL_RTL (decl);
+
+      if (!is_global_var (decl))
+    value = eliminate_regs (value, VOIDmode, NULL_RTX);
+
+      SET_DECL_RTL (decl, value);
 #ifdef LEAF_REG_REMAP
       if (crtl->uses_only_leaf_regs)
-    leaf_renumber_regs_insn (DECL_RTL (decl));
+    leaf_renumber_regs_insn (value);
 #endif
-      value = DECL_RTL (decl);

       /* Don't mention a variable at all
      if it was completely optimized into nothingness.