Re: syncing the GCC vax port

[Jeff Law <law%redhat.com@localhost>]

On 3/30/19 3:03 AM, coypu%sdf.org@localhost wrote:
> hi folks,
> 
> i was interesting in tackling some problems gcc netbsd/vax has.
> it has some ICEs which are in reload phase. searching around, the answer
> to that is "switch to LRA first". Now, I don't quite know what that is
> yet, but I know I need to try to do it.
> 
> As an initial step, I need to sync the source code.
> netbsd/vax has some outstanding work on GCC.
> 
> I've done this, and I can run programs built by this compiler:
> http://coypu.sdf.org/gcc-9-vax.diff
> 
> (My tree has more detail on the changes done:
> https://github.com/gcc-mirror/gcc/compare/master...coypoop:vax )
> 
> Matt Thomas (the GCC VAX maintainer) is the author of most of these
> changes, I suspect he will not be very responsive by email.
> Not being the author, I might not be able to answer all the questions,
> but I can try my best.
> 
> How do I get this across? comments? straight to gcc-patches? :-)
> 
> I know Jeff Law did not like the change to builtins.md as being wrong. I
> can omit them, I forgot about it until typing this email :)
> 
> caveat: had an ICE during reload in the build process, I hid it
> under the rug with -O0.
I don't recall what I objected to :-)  From looking at the changes I'd
be concerned about the changes from memory_operand to
volatile_mem_operand.  Without knowing more about the problem you're
trying to solve it's hard to ACK something like this.

The new "condjump" expander seems a bit under-specified.  Is there some
reason why you needed that expander and couldn't just add a name to a
existing define_insn?

A nit.  In that expander you have "gen_rtx_NE(VOIDmode ..."  There
should be a space between the NE and the open parenthesis.  That kind of
nit is repeated several times.

The changes in constraints.md do not seem to change functionality at
all, just reordering the oprerands.  However our preferred style is
what's in there right now.

ival >= 0  is the right way   0 <= ival is the wrong way.


I noticed the patch turns off flag_dwarf2_cfi_asm.  Is there a reason
for that?  But yet you create DEF_CFA notes in the prologue.  Is it the
case that the CFI bits just aren't ready for consumption yet?  If not,
then it may be easier to just not include those changes right now.


I can't really comment on the changes to how addreses are handled in
print_operand_address.  I'd just have to assume they're correct.

I don't know if we generally allow debug_rtx calls like are added.
Usually we gcc_assert or gcc_unreachable.

mkrtx needs a function comment and looks like its got some formatting
problems (spaces vs tabs issues and missing space between function name
an the open paren for arguments).  Similarly for vax_output_movmemsi and
legitimate_pic_operand_p, vax_decomposed_dimode_operand_p,

You've got undocumented #ifdefs in legitimate_pic_operand_p.

You've got incorrect operand ordering in the adjacent_operands_p changes.


The netbsd specific changes to the zero_extract patterns looks strange.
 Why why not just have the right operand.  And why change it in the
first place?

Those peepholes look strange.  Why is the first insn not just removed as
dead?

And if these changes were done by Matt Thomas, does he have a copyright
assignment on file?  If not, then we can't really use them.  These are
large enough that they'd require an assignment.

jeff