Re: [RFA:] Don't strip unary ops when matching constraints. (PRtarget/23424)

[Joern RENNECKE <joern dot rennecke at st dot com>]

Hans-Peter Nilsson wrote:

> > I doubt that crafting a regexp 
> > that handles comments
> > correctly is easier than use one of the gen* programs to parse the files 
> > and tell you which
> > define_insn{and_split}s have operands without predicates but with a 
> > constraint.
> >    
>
> That'd require *building* each target.
No, you can warp one of the gen* programs to be a code checker that you can
apply to multiple targets.  There is not much point in doing this now 
since you
already have done a manual check, so the only extra coverage would be
longer patterns / patterns with comments.... But maybe it's something to
keep in mind when you want to know something else about all the ports
in the future. 

> Unfortunately, I notice code differences in four of the 31734
> assembled files, which correspond to locale-inst.cc and
> ostream-inst.cc for two different multilibs (I guess the endian
> multilibs; the code was definitely sh64).
Which multilibs?  How does the difference look like?

Four out of 31734 files seems to few to cause worry about performance in 
itself,
but it indicates that we haven't fully understood what is going on, which
is worrying.  Since recog allows all all unops, allowing some explicitly
shouldn't make a difference.
There are three things I could think of:

- recog strips exactly one unop.  If recog strips one unop, and the 
constraint
 recognizes another, we might now accept operands with two levels of unop.
 We could check if that applies to the files that show changed asm 
output by putting
 a gcc_assert (UNARY_P (op)) after we have stripped the unop and then 
recompile
 these files.
- Or we could have something really nasty like a subreg of a unop.
 I think the way we strip SUBREGs is just as bad as we strip unops.
 When an EXTRA_CONSTRAINT is applied to an operand that was originally
 a SUBREG, you only see the SUBREG_REG, you have no indication of its
 original mode nor its SUBREG_BYTE.
 Likewise, we could check if this is the issue you have run into by 
putting a check
 for UNARY_P after the subreg stripping in recog (that is just after 
the unop
 stripping).
- There might be some code that does something special when it sees a 
constraint
 alternative that only contanins a register class letter.

There are at least three patterns mentioned in your patch that use matching
constraints which are relevant: movdicc_false, movdicc_true,  mcmv .
The matching constraint is to output operand 0, which must be a 
register, but
the matching operand may legitimately be a sign_extend (to DImode) of the
output register in SImode.



> Lots of testsuite failures were due to the stack overlapping
> code, as noticed by ld:
> /home/hp/combined/sh64-sim/./gcc/collect-ld: section .stack [0000000000040000 -> 0000000000040003] overlaps section .text
> [0000000000001060 -> 000000000009737f]
> You may want to move the stack address in the ld scrip.
>  
I do this in my site.exp:

       set board_info(sh64,ldscript) "-Wl,--defsym,_stack=0x1ff000"
       set board_info(sh64/-ml,ldscript) "-Wl,--defsym,_stack=0x1ff000"
       set board_info(sh64/-m5-compact/-ml,ldscript) 
"-Wl,--defsym,_stack=0x1ff000"

> The contrib tarball you pointed to with the sh64 simulator and
> the sh64-sim.exp I used, could have done with having the objects
> and config.status pruned with e.g. "make distclean".
>  
There was some configuration problem that required a manual non-obvious
step before the cgen files could be built.

> +	(eq:SI (and:SI (match_operand:SI 0 "arith_reg_operand" "zUs,rUs")
>  
zUs is bogus.  But then, the predicate doesn't allow unary operators for
TARGET_SH1 anyway.