Re: PR target/21299 (reload accepting invalid asm)

[Ian Lance Taylor <iant at google dot com>]

Richard Sandiford <richard@codesourcery.com> writes:

> Ian Lance Taylor <iant@google.com> writes:
> > Richard Sandiford <richard@codesourcery.com> writes:
> >> I'm happy with that.  But I was trying to pin down what the rules were
> >> so that we can enforce those same rules for user asms.  So if that's
> >> the rule, then I think we should be checking something like:
> >> 
> >>    "Two register class constraints may not be used in the same
> >>    alternative for a value of mode M if the backend allows values
> >>    of mode M to be stored in a register that straddles both classes."
> >> 
> >> Hopefully that's a more general fix than the ones suggested so far.
> >
> > My thinking is that the documentation should simply say: we recommend
> > that you do not use more than one register class in a constraint.  If
> > you think you need to, you are most likely doing something wrong.
> >
> > I don't think we should enforce anything like this in the compiler.
> > Gcc's inline assembler extension has historically permitted a great
> > deal of chumminess between the compiler implementation and the code.
> > This obviously has problems, like the fact that most people write
> > inline assembler incorrectly.  But I don't think we should take small
> > steps to try to fix that problem: I think small steps would simply be
> > irritating to the expert and unhelpful to the novice.
> 
> Does this mean this PR should be marked WONTFIX?

No, of course not.  I already suggested how we should handle this
case, and I even pointed to the place in the code where we should make
the change.  We should give an error that the asm has impossible
constraints.

> I don't understand your comment about the suggested change being
> irritating to the expert.  We're talking about a case that will
> ICE otherwise.  If we declare something invalid for .md files,
> then surely we should declare it invalid for user asms too.

Sure, I agree, and we should do it in the documentation, not in the
code.  I see no benefit to adding checks for inline assembler in the
compiler, because the inline assembler is intrinsically dependent on
the backend implementation which can not be changed.  If there is some
technically invalid trick that the expert can play to get their code
to work, I think that is OK.  I don't think we gain by prohibiting it.

And, lest I be misunderstood, let me repeat that I think the current
inline assembler implementation has problems, and let me repeat that I
think a redesign would be appropriate.  However, I think that some
incremental fixes are inappropriate unless we can be somehow certain
that they are not harmful.  The existing inline assembler support is
not an implementation of a clean clearly articulated design which can
be improved in accordance with that design; it is a direct interface
to the internals of the compiler, and it should be treated as such.

Ian