This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> Date: Tue, 18 Oct 2005 13:23:57 +0100
> From: Joern RENNECKE <joern.rennecke@st.com>

> Hans-Peter Nilsson wrote:
> 
> >
> >I guess one way would be to save the assembly code and compare.
> >Would you be happy with a test-run that does that for sh64,
> >assuming a baseline builds?
> >
> Are you saying you expect the assembly code to be identical?

Yes, except for the anonymous namespace-manglings in C++.  (I
may be facing a disappointment related to pointer hashing, but
that'd be a bug by itself - the code should be similar enough,
with no *obvious* size/speed regressions.)

>  And what 
> code base
> were you intending to use for the comparison?

Just a regular build and test.

> I'd be willing t accept identical code generation for EEMBC as evidence

No can do, I don't have EEMBC.  Please don't require commercial
benchmarks.  I could've done it for CSiBE, but I think these
in-depth test requirements are moot - it's obvious that the
patch as is, will fail building on sh64-elf; it failed in libgcc
for sh64-linux-gnu.

> >  (I'd argue that if a baseline
> >doesn't build for a target, no further testing should be
> >required.)
> Only if the failure to build is consistent over an extended period of 
> time without
> efforts by the port maintainer to get the situation resolved.  You can't 
> say that
> because a port has been recently broken by someone else, you are entitled to
> break it beyond recoverability.

No, I do not say that.  I say I don't *have* to adjust a port to
fix a bug *if* the port itself doesn't build *when I test it*.
That's anyway an academic question AFAIK.

> >Anyway, it seems far more likely that the bug is just waiting to
> >happen for the SH port,
> >
> Why do you call the interface a bug?

An undocumented quirk should not be called an interface.

I've already said why: It's not documented, and unary operators
are the only ones that are stripped.  It causes for example
(sign_extend:SI (mem:QI ...)) to be passed as (mem:QI ...) to
the constraint-test so it will not match; a constraint for an
unop will never see the same as the predicate.  Just plain
bogus.

> > than that fixing it would uncover a
> >latent bug; particularly as you and presumably aoliva and others
> >did not expect this unop-stripping when working on the port.
> >  
> >
> I didn't expect this unop -stripping ad hoc, but I've encountered it and
> dealt with it.  And subsequently I expected it.

I can see that, now that I looked further at the SH port.  I'm
kind of worried that you didn't bring up this surprise when you
encountered it.  It should be fixable anyway by adding proper
constraints for the unops to the SH port.

> >None.  Like I said, when I looked, only CRIS and SH apparently
> >are affected.
> >  
> >
> Well, but you didn't test sh64.

I was under the impression that sh64 was covered build-wise by
the SH multilibs.  I was wrong.

> >Only if those non-predicate occurrences also use constraints
> >(which I'd argue would seem a bug by itself) and that shouldn't
> >be very common.  I guess I could check for that.
> >  
> >
> Even if the operand doesn't have a predicate, the instruction still 
> might have one.
> Remember, there are multi-alternative constraint sets, but no 
> multi-alternative
> interlocked predicate sets for the operand - you have to use an 
> instruction predicate
> to express that.

By what do you refer to as "instruction predicate"?  Do you mean
the pattern *condition*?  Is your point that predicate-less
operands *with* constraints would seem an indication of a bug?
How many are there?  Is there something worth discussing here?

> >If it wasn't clear, I *did* inspect other ports for
> >unop-predicates with constraint uses.  I tested the main
> >platforms I have easy access to.
> >  
> >
> Obviously, your checking of the SH port did not go as far as figuring 
> out where the
> predicates are actually used, otherwise I wouldn't have had to tell you 
> that the sh64
> target needs to be tested.

Right, besides the wrong assumption on sh64 being built as a
multilib of sh-elf, I didn't go further on checking SH once I'd
identified it as "affected".  I didn't dream of the port being
adjusted with no screaming comment the unexpected "interface".

Anyway, the bottom line is that you proposed solving this by a
hook.  I've accepted that route, though reluctantly.  Mark
Mitchell agreed to solving it by a hook, so I guess the next
thing is for me to adjust my patch.

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]