On 03/26/2016 05:56 AM, Jake Hamby wrote:
On Mar 23, 2016, at 05:56, Christos Zoulas <chris...@astron.com>
wrote:
In article <f48d0c6b-a6db-410b-bc97-c30d4e8b4...@me.com>, Jake
Hamby <jehamby...@me.com> wrote:
Hi,
Thanks a lot for your patch. I applied it to our gcc-5 in the
tree. Unfortunately gcc-5 seems that it was never tested to even
compile. I fixed the simple compilation issue, but it fails to
compile some files with an internal error trying to construct a
dwarf frame info element with the wrong register. If you have some
time, can you take a look? I will too.
Thanks,
christos
Hi Christos,
I just rebased my patches on top of GCC 5.3, which I see you have
recently switched to. Here’s a brief explanation of how I patched the
dwarf frame error.
The problem is that FIRST_PSEUDO_REGISTER should be increased from 16
to 17, because PSW is considered a real register for DWARF debug
purposes. This necessitated changing a few other macros, but nothing
major. Since it’s marked as one of the FIXED_REGISTERS, it never
actually gets used. Currently I’m doing a full build with GCC 5.3 and
CONFIGURE_ARGS += —enable-checking=all, which is very much slower, of
course.
This patch could probably be pulled out and installed independent of the
rest. It's simple, well isolated and should be reviewable even by folks
without intimate knowledge of hte vax port.
One bug I discovered with —enable-checking=all on GCC 4.8.5 is a call
to XEXP() that may not be valid, but which can be prevented by
checking the other conditions first, and then calling XEXP() if the
other conditions are true.
Where specifically? And what's the hunk of RTL that's being examined
incorrectly (hint, debug_rtx will dump a hunk of RTL symbolically).
There seems to be a code generation bug with C++ that only affects a
few things. Unfortunately, GCC itself (the native version, not the
cross compiler) is one of the programs affected. The symptom when
compiling almost anything complex in GCC 4.8.5 is a stack overflow as
it recursively loops around trying to expand bits and pieces of the
insns. It seems to be branching the wrong way.
Can't do much with this. Given the known problems with set-cc0
elimination on this port, perhaps we address those, then come back to
this problem.
In looking at this, I discovered one really broken issue in the
current vax.md, namely the three peephole optimizations at the
bottom. The comment on the bottom one that says “Leave this commented
out until we can determine whether the second move precedes a jump
which relies on the CC flags being set correctly.” is absolutely
correct and I believe all three should be removed. I’ve done so in
the diff below, and added a comment explaining why.
The comment is essentially useless because it refers back to patterns
that don't exist. But it's also not clear what patterns you're
referring to. I don't see any define_peepholes in vax.md going back to
gcc-4.9. What sources are you using and who's hacked on them?
Note the 0 && in the condition of the last peephole you removed
essentially disabled that peephole.
It would really help if you were submitting patches against the current
GCC trunk.
The idea is that some part of the optimizer is able to remove
explicit tst / cmp insns when a previous one has already set the
flags in a way that’s useful. So my theory is that the current
version mostly works, but it’s very difficult to understand as it’s
written, and it may be very occasionally giving the wrong results due
to either bugs in its logic or the instructions emitted not setting
the flags in the usual way. Unfortunately, the m68k version of
NOTICE_UPDATE_CC (the only other arch supported by NetBSD that uses
the CC0 style) is even more convoluted.
This optimization happens in final.c
Essentially it tracks the state of the cc0 bits and a few other things.
When it finds a comparison (cc0-setter), it refers back to the state of
the bits and if hte bits are in a usable state, it'll "delete" the
cc0-setting insn.
So what I’d like to try next is rewriting the VAX version of
notice_update_cc to use the style that the AVR backend and others use
which is to add a “cc” attribute to the instructions themselves in
the .md file describing what each one does in terms of flags.
This would be a step forward. I would do this work independently of
fixing hte dwarf/EH stuff. Ideally patch submissions are independent
changes rather than fixing several unrelated problems in a single patch.
