public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PING] Target hook for rewriting inline asm constraints
@ 2007-10-30 12:54 Andreas Krebbel
  2007-10-31 16:31 ` Tom Tromey
  2007-11-06 18:20 ` Michael Meissner
  0 siblings, 2 replies; 18+ messages in thread
From: Andreas Krebbel @ 2007-10-30 12:54 UTC (permalink / raw)
  To: gcc-patches

Hello,

could a C front end and/or middle end maintainer please have a look at
this one:

http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01407.html

Thanks

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-30 12:54 [PING] Target hook for rewriting inline asm constraints Andreas Krebbel
@ 2007-10-31 16:31 ` Tom Tromey
  2007-10-31 16:43   ` Richard Guenther
  2007-10-31 17:22   ` Andreas Krebbel
  2007-11-06 18:20 ` Michael Meissner
  1 sibling, 2 replies; 18+ messages in thread
From: Tom Tromey @ 2007-10-31 16:31 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches

>>>>> "Andreas" == Andreas Krebbel <Andreas.Krebbel@de.ibm.com> writes:

Andreas> could a C front end and/or middle end maintainer please have
Andreas> a look at this one:
Andreas> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01407.html

I'm not an FE or ME maintainer but I took a look anyway.
I only looked at the mechanics of the change, since I really don't
know whether it is desirable or not.

Wouldn't the C++ FE need a similar patch?  And, if so, why do this
work in the front ends rather than somewhere more generic?  Or, if the
FE is really the way to go, perhaps a helper function in c-common.c
would be better.


I saw a few coding standard violations in the code.

* Repeated use of strlen in the loop (use an output index)
* Assignments in conditional expressions (frowned on by GNU standards;
  in any case the comma operators seemed gratuitous to me)
* No braces around body of do-while.
* Hard-coded lengths without bounds checking (this one is questionable
  I suppose)

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-31 16:31 ` Tom Tromey
@ 2007-10-31 16:43   ` Richard Guenther
  2007-10-31 18:55     ` Andreas Krebbel
  2007-10-31 17:22   ` Andreas Krebbel
  1 sibling, 1 reply; 18+ messages in thread
From: Richard Guenther @ 2007-10-31 16:43 UTC (permalink / raw)
  To: tromey; +Cc: Andreas Krebbel, gcc-patches

On 10/31/07, Tom Tromey <tromey@redhat.com> wrote:
> >>>>> "Andreas" == Andreas Krebbel <Andreas.Krebbel@de.ibm.com> writes:
>
> Andreas> could a C front end and/or middle end maintainer please have
> Andreas> a look at this one:
> Andreas> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01407.html
>
> I'm not an FE or ME maintainer but I took a look anyway.
> I only looked at the mechanics of the change, since I really don't
> know whether it is desirable or not.

I also wonder how you can distinguish between "old" and "new" "m" used
in asms.  How do you know which semantics the user chose?  IMHO
"semantics" of asm constrains should never change, but instead you should
introduce new machine specific constraints if necessary.

The patch doesn't come with a testcase or an example showing how this
should work or how it would break without it, so it's hard to tell if you
have a point.  But in general - asm constrains rewriting?? uh no.

Richard.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-31 16:31 ` Tom Tromey
  2007-10-31 16:43   ` Richard Guenther
@ 2007-10-31 17:22   ` Andreas Krebbel
  2007-10-31 17:56     ` Tom Tromey
  1 sibling, 1 reply; 18+ messages in thread
From: Andreas Krebbel @ 2007-10-31 17:22 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gcc-patches

Hi,

> I'm not an FE or ME maintainer but I took a look anyway.
Thanks!

> Wouldn't the C++ FE need a similar patch?  And, if so, why do this
> work in the front ends rather than somewhere more generic?  

Yes, the C++ FE also needs to call that hook.  I've focused on C with
my first patch since this is the most obvious user of inline
assemblies, but you are right this has to be extended.

I've tried at first to implement this as a more generic hook in the
middle end, but dealing with this in recog and reload turned out to be
way more complicated.

> Or, if the
> FE is really the way to go, perhaps a helper function in c-common.c
> would be better.
Ok that might be a good idea.

> I saw a few coding standard violations in the code.
> 
> * Repeated use of strlen in the loop (use an output index)

Ok.

> * Assignments in conditional expressions (frowned on by GNU standards;
>   in any case the comma operators seemed gratuitous to me)
> * No braces around body of do-while.

I've simply copied the loop reload and recog are using.  See
find_reloads and constrain_operands.  I wasn't aware that this isn't
considered good style.  I can certainly change that.

> * Hard-coded lengths without bounds checking (this one is questionable
>   I suppose)

It would be difficult to harden that code against back end hooks
returning huge strings.  I'm not sure thats worth the effort.  But
I'll try to address this.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-31 17:22   ` Andreas Krebbel
@ 2007-10-31 17:56     ` Tom Tromey
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2007-10-31 17:56 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches

>>>>> "Andreas" == Andreas Krebbel <Andreas.Krebbel@de.ibm.com> writes:

Andreas> I've tried at first to implement this as a more generic hook in the
Andreas> middle end, but dealing with this in recog and reload turned out to be
Andreas> way more complicated.

I was thinking more like gimplification or some other early pass.

>> * No braces around body of do-while.

Andreas> I've simply copied the loop reload and recog are using.  See
Andreas> find_reloads and constrain_operands.  I wasn't aware that this isn't
Andreas> considered good style.  I can certainly change that.

See (info "(standards) Formatting"), toward the end.

>> * Hard-coded lengths without bounds checking (this one is questionable
>> I suppose)

Andreas> It would be difficult to harden that code against back end hooks
Andreas> returning huge strings.  I'm not sure thats worth the effort.  But
Andreas> I'll try to address this.

FWIW, a simple assertion would satisfy me.  Or, it isn't hard to use
obstacks or dyn-strings.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-31 16:43   ` Richard Guenther
@ 2007-10-31 18:55     ` Andreas Krebbel
  2007-10-31 19:13       ` Richard Guenther
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Krebbel @ 2007-10-31 18:55 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

Hi,

> I also wonder how you can distinguish between "old" and "new" "m" used
> in asms.  How do you know which semantics the user chose?  IMHO
> "semantics" of asm constrains should never change, but instead you should
> introduce new machine specific constraints if necessary.

Actually the point of the target hook is to prevent the "m" constraint
semantics from changing when the back end changes.

Consider the case you want to add a new instruction to a back
end. This new instruction supports loading a value from memory
locations given by base + index register.  All the existing
instructions just accepted a single base register as memory address.
In order to prevent reload from trying to reload all base + index
addresses into a single base address register you have to change the
GO_IF_LEGITIMATE_ADDRESS hook to accept base + index addresses.  This
change would depend on the availability of the new instruction
supporting base + index like addresses so most likely it would depend
on an architecture flag.

As a consequence the "m" constraint also would accept base + index
addresses for the added architecture.  Unfortunately this breaks all
places where one of the old instructions is coupled to the "m"
constraint.  Instruction patterns in the back end using the "m"
constraint have to be changed.  The straightforward solution would be
to add a new constraint letter accepting memory addresses with just a
single base register and replace all occurrences of "m" with that new
constraint letter.

But unfortunately our internally used constraint letters are exposed
to the GCC user via the GCC inline assembly construct.  So if you have
an inline assembly containing one of the old instructions accepting
just a base as address its operand constraint is probably the "m"
constraint.  If you now would compile the very same inline assembly
setting the architecture flag which makes the back end to enable base
+ index addresses GCC will pass a base + index address for that
operand which would make gas to complain about that instruction.

The problem is that the "m" semantics changed but not the instruction
in the inline assembly.

To distingiush between the different "m" constraint versions the back
end hook has to use the same logic which will be added to
GO_IF_LEGITIMATE_ADDRESS.  The new hook should replace every
occurrence of "m" in an inline assembly operand list with the
constraint letter for the former address format and it only has to do
it for architecture flags which would change the behaviour of
GO_IF_LEGITIMATE_ADDRESS (although it wouldn't be a problem to
always do the rewrite).

> The patch doesn't come with a testcase or an example showing how this
> should work or how it would break without it, so it's hard to tell if you
> have a point.  But in general - asm constrains rewriting?? uh no.

I don't have a testcase yet.  But my explanations above probably give
you an idea why this target hook is necessary sooner or later.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-31 18:55     ` Andreas Krebbel
@ 2007-10-31 19:13       ` Richard Guenther
  2007-10-31 20:40         ` Andreas Krebbel
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Guenther @ 2007-10-31 19:13 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches

On 10/31/07, Andreas Krebbel <Andreas.Krebbel@de.ibm.com> wrote:
> Hi,
>
> > I also wonder how you can distinguish between "old" and "new" "m" used
> > in asms.  How do you know which semantics the user chose?  IMHO
> > "semantics" of asm constrains should never change, but instead you should
> > introduce new machine specific constraints if necessary.
>
> Actually the point of the target hook is to prevent the "m" constraint
> semantics from changing when the back end changes.
>
> Consider the case you want to add a new instruction to a back
> end. This new instruction supports loading a value from memory
> locations given by base + index register.  All the existing
> instructions just accepted a single base register as memory address.
> In order to prevent reload from trying to reload all base + index
> addresses into a single base address register you have to change the
> GO_IF_LEGITIMATE_ADDRESS hook to accept base + index addresses.  This
> change would depend on the availability of the new instruction
> supporting base + index like addresses so most likely it would depend
> on an architecture flag.
>
> As a consequence the "m" constraint also would accept base + index
> addresses for the added architecture.  Unfortunately this breaks all
> places where one of the old instructions is coupled to the "m"
> constraint.  Instruction patterns in the back end using the "m"
> constraint have to be changed.  The straightforward solution would be
> to add a new constraint letter accepting memory addresses with just a
> single base register and replace all occurrences of "m" with that new
> constraint letter.
>
> But unfortunately our internally used constraint letters are exposed
> to the GCC user via the GCC inline assembly construct.  So if you have
> an inline assembly containing one of the old instructions accepting
> just a base as address its operand constraint is probably the "m"
> constraint.  If you now would compile the very same inline assembly
> setting the architecture flag which makes the back end to enable base
> + index addresses GCC will pass a base + index address for that
> operand which would make gas to complain about that instruction.
>
> The problem is that the "m" semantics changed but not the instruction
> in the inline assembly.
>
> To distingiush between the different "m" constraint versions the back
> end hook has to use the same logic which will be added to
> GO_IF_LEGITIMATE_ADDRESS.  The new hook should replace every
> occurrence of "m" in an inline assembly operand list with the
> constraint letter for the former address format and it only has to do
> it for architecture flags which would change the behaviour of
> GO_IF_LEGITIMATE_ADDRESS (although it wouldn't be a problem to
> always do the rewrite).

Why not leave "m" as is, even with the new addressing mode and add a
new constraint allowing the new base + index addressing mode.  This way
existing patterns and inline asm will continue to work (it couldn't know about
the new addressing mode or instructions anyway) and if one wants to use
the new stuff use the new constraint?

Are you suggesting the rewriting for optimization (I think you do)?  Or do you
believe there is no way you can keep correctness without the patch (I don't
believe that)?

> > The patch doesn't come with a testcase or an example showing how this
> > should work or how it would break without it, so it's hard to tell if you
> > have a point.  But in general - asm constrains rewriting?? uh no.
>
> I don't have a testcase yet.  But my explanations above probably give
> you an idea why this target hook is necessary sooner or later.

The only thing I could imagine would be that the new architecture forces
you to use base + index addressing, but only on _some_ insns.  So where

  insn0 %0
  insn1 %1

with both "m" constraints would be valid before, only (and truly only) the
second insn1 is now forced to do base + index addressing?  That is, you
have to split semantics of "m" based on the instruction it is used in?
Which is the root of my doubt that you can automatically rewrite "m"s
in any case - or proves that this is not the situation you need to handle.

Richard.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-31 19:13       ` Richard Guenther
@ 2007-10-31 20:40         ` Andreas Krebbel
  2007-11-04 23:05           ` Richard Sandiford
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Krebbel @ 2007-10-31 20:40 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

Hi,

> Why not leave "m" as is, even with the new addressing mode and add a
> new constraint allowing the new base + index addressing mode.  This way
> existing patterns and inline asm will continue to work (it couldn't know about
> the new addressing mode or instructions anyway) and if one wants to use
> the new stuff use the new constraint?

Unfortunately this is not possible.  The problem is that the
GO_IF_LEGITIMATE_ADDRESS_P hook is coupled to the "m" constraint.
Currently it isn't possible to change one without the other.  But you
*have* to add the new addressing mode the GO_IF_LEGITIMATE_ADDRESS_P
hook otherwise these addresses would never pass the reload step.

The definition of the "m" constraint is to accept every possible
address the back end accepts.  Or in the words of the interals manual:

m: "A memory operand is allowed, with any kind of address that the
machine supports in general."

So you actually can't support new addressing formats in the back end
without changing the semantics of it.  The proper way would be to
disallow the "m" constraint in inline assemblies but thats probably a
bit late ;)

> The only thing I could imagine would be that the new architecture forces
> you to use base + index addressing, but only on _some_ insns.  So where
> 
>   insn0 %0
>   insn1 %1
> 
> with both "m" constraints would be valid before, only (and truly only) the
> second insn1 is now forced to do base + index addressing?  That is, you
> have to split semantics of "m" based on the instruction it is used in?
> Which is the root of my doubt that you can automatically rewrite "m"s
> in any case - or proves that this is not the situation you need to handle.

I must admit that it is probably are rare case that an architecture
changes so fundamentally that new address types are supported.  But
actually we already had such a change for S/390 - with the long
displacement extension.  Originally only Base + Index + 12 bit
displacement addresses were supported.  With the long displacement
facility the discplacement was extended to 20 bit.  Fortunately most
of the instructions dealing with memory were extended transparently so
it was decided to leave it as is but for 100% correctness we would
have needed a mechanism like the "inline asm constraint rewrite hook"
already for that change.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-31 20:40         ` Andreas Krebbel
@ 2007-11-04 23:05           ` Richard Sandiford
  2007-11-05  9:25             ` Andreas Krebbel
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Sandiford @ 2007-11-04 23:05 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: Richard Guenther, gcc-patches

"Andreas Krebbel" <Andreas.Krebbel@de.ibm.com> writes:
> So you actually can't support new addressing formats in the back end
> without changing the semantics of it.  The proper way would be to
> disallow the "m" constraint in inline assemblies but thats probably a
> bit late ;)

Sorry if this has already been suggested, but why not just replace
hard-coded uses of 'm' in constraint-handling code with uses of some
target macro?  You could then treat your current 'm' as an ordinary
define_memory_constraint.

Richard

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-04 23:05           ` Richard Sandiford
@ 2007-11-05  9:25             ` Andreas Krebbel
  2007-11-05  9:43               ` Richard Sandiford
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Krebbel @ 2007-11-05  9:25 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

> Sorry if this has already been suggested, but why not just replace
> hard-coded uses of 'm' in constraint-handling code with uses of some
> target macro?  You could then treat your current 'm' as an ordinary
> define_memory_constraint.

I'm not sure this would work.  I think reload relies on the
relationship between the 'm' constraint and GO_IF_LEGITIMATE_ADDRESS.
There is an early loop invoking find_reloads_address.  This reloads
all addresses which aren't supported by the target at all i.e. all
addresses for which GO_IF_LEGITIMATE_ADDRESS is false (or does not
jump to the given label).  If reload later on sees an 'm' constraint
it considers the address already been fixed by that early loop.  So I
would expect that in order to make such a hook work I would have to
mess around with the reload logic which is not preferable I think.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-05  9:25             ` Andreas Krebbel
@ 2007-11-05  9:43               ` Richard Sandiford
  2007-11-05 10:32                 ` Andreas Krebbel
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Sandiford @ 2007-11-05  9:43 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches

"Andreas Krebbel" <Andreas.Krebbel@de.ibm.com> writes:
>> Sorry if this has already been suggested, but why not just replace
>> hard-coded uses of 'm' in constraint-handling code with uses of some
>> target macro?  You could then treat your current 'm' as an ordinary
>> define_memory_constraint.
>
> I'm not sure this would work.  I think reload relies on the
> relationship between the 'm' constraint and GO_IF_LEGITIMATE_ADDRESS.
> There is an early loop invoking find_reloads_address.  This reloads
> all addresses which aren't supported by the target at all i.e. all
> addresses for which GO_IF_LEGITIMATE_ADDRESS is false (or does not
> jump to the given label).  If reload later on sees an 'm' constraint
> it considers the address already been fixed by that early loop.  So I
> would expect that in order to make such a hook work I would have to
> mess around with the reload logic which is not preferable I think.

I don't understand.  What I'm saying is that we should look through
gcc/*.c for cases where the C construct "'m'" refers to "the constraint
associated with legitimate addresses", and replace those cases with some
target macro that is 'm' by default.  E.g.:

	      case 'm':
		if (force_reload)
		  break;
		if (MEM_P (operand)
		    || (REG_P (operand)
			&& REGNO (operand) >= FIRST_PSEUDO_REGISTER
			&& reg_renumber[REGNO (operand)] < 0))
		  win = 1;
		if (CONST_POOL_OK_P (operand))
		  badop = 0;
		constmemok = 1;
		break;

would become:

	      case TARGET_MEM_CONSTRAINT:
		if (force_reload)
		  break;
		if (MEM_P (operand)
		    || (REG_P (operand)
			&& REGNO (operand) >= FIRST_PSEUDO_REGISTER
			&& reg_renumber[REGNO (operand)] < 0))
		  win = 1;
		if (CONST_POOL_OK_P (operand))
		  badop = 0;
		constmemok = 1;
		break;

and other places would change similarly.  If we do that consistently,
there would no longer be a connection between 'm' and
GO_IF_LEGITIMATE_ADDRESS.

Replacing hard-coded constants seems like good practice for its own
sake and would avoid the need for the front end to rewrite asms.

Richard

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-05  9:43               ` Richard Sandiford
@ 2007-11-05 10:32                 ` Andreas Krebbel
  2007-11-05 11:02                   ` Richard Sandiford
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Krebbel @ 2007-11-05 10:32 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

> I don't understand.  What I'm saying is that we should look through
> gcc/*.c for cases where the C construct "'m'" refers to "the constraint
> associated with legitimate addresses", and replace those cases with some
> target macro that is 'm' by default.  E.g.:
...
> and other places would change similarly.  If we do that consistently,
> there would no longer be a connection between 'm' and
> GO_IF_LEGITIMATE_ADDRESS.

Ok I see.  The new TARGET_MEM_CONSTRAINT would have to be a hook
returning a new constraint letter for the architecture introducing the
new address format and 'm' otherwise - right ?!

That sounds reasonable to me - thanks for explaining it twice.  I
thought you were suggesting a target hook which allows the back end to
limit what is accepted for 'm'.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-05 10:32                 ` Andreas Krebbel
@ 2007-11-05 11:02                   ` Richard Sandiford
  2007-11-05 13:42                     ` Andreas Krebbel
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Sandiford @ 2007-11-05 11:02 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches

"Andreas Krebbel" <Andreas.Krebbel@de.ibm.com> writes:
>> I don't understand.  What I'm saying is that we should look through
>> gcc/*.c for cases where the C construct "'m'" refers to "the constraint
>> associated with legitimate addresses", and replace those cases with some
>> target macro that is 'm' by default.  E.g.:
> ...
>> and other places would change similarly.  If we do that consistently,
>> there would no longer be a connection between 'm' and
>> GO_IF_LEGITIMATE_ADDRESS.
>
> Ok I see.  The new TARGET_MEM_CONSTRAINT would have to be a hook
> returning a new constraint letter for the architecture introducing the
> new address format and 'm' otherwise - right ?!

Well, I was thinking of making it a macro constant; I should have picked
a name other than TARGET_MEM_CONSTRAINT, sorry.

It might be nice to define the macro indirectly using a new constraints.md
construct (define_general_memory_constraint?).  I.e. have genpreds define
the macro for you based on the .md file.  The implementation could then
move away from macros at the same time as the rest of the constraints
stuff does[*].  I think that'd still be easier than converting gcc/*.c
to treat 'm' as a hook (and easier than rewriting asms).  But personally,
I'd be happy enough if the macro was defined in the target .h file.

Richard

[*] E.g. we might eventually generate a constraints parser from the
    .md file.  I can't remember now if that was an explicit goal of
    the constraints.md stuff.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-05 11:02                   ` Richard Sandiford
@ 2007-11-05 13:42                     ` Andreas Krebbel
  2007-11-06 22:00                       ` Richard Sandiford
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Krebbel @ 2007-11-05 13:42 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

> Well, I was thinking of making it a macro constant; I should have picked
> a name other than TARGET_MEM_CONSTRAINT, sorry.
> 
> It might be nice to define the macro indirectly using a new constraints.md
> construct (define_general_memory_constraint?).  I.e. have genpreds define
> the macro for you based on the .md file.  The implementation could then
> move away from macros at the same time as the rest of the constraints
> stuff does[*].  I think that'd still be easier than converting gcc/*.c
> to treat 'm' as a hook (and easier than rewriting asms).  But personally,
> I'd be happy enough if the macro was defined in the target .h file.

One disadvantage of a target macro used in case expressions is that we
can't check the arch flags here.  So the logic would have to go into
the constraint definition.  The new constraint letter will have to
accept the new address format as well as the old one for the new
architecture and just the old address format for all former archs.  If
we would have a target hook here which would return the new mem
constraint letter if the arch flag for the new architecture is set and
'm' otherwise, the new constraint letter would not have to depend on
arch flags what I would personally prefer.

Another disadvantage is that this would prevent usage of multiple
letter constraints for the standard mem constraint.

What about the idea of letting the back end override all the existing
standard constraints using the existing constraint definitions.
Reload and co could check whether a back end accepts one of the
standard constraint letters as extra constraint and would fallback to
the standard behaviour if not.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-10-30 12:54 [PING] Target hook for rewriting inline asm constraints Andreas Krebbel
  2007-10-31 16:31 ` Tom Tromey
@ 2007-11-06 18:20 ` Michael Meissner
  2007-11-07  9:10   ` Andreas Krebbel
  1 sibling, 1 reply; 18+ messages in thread
From: Michael Meissner @ 2007-11-06 18:20 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches

On Tue, Oct 30, 2007 at 12:54:24PM +0100, Andreas Krebbel wrote:
> Hello,
> 
> could a C front end and/or middle end maintainer please have a look at
> this one:
> 
> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01407.html

I'm catching up on my backlog.

Here are the nit-picky comments about the change:

First, fixed size string arrays (result and tmp in c_parser_asm_operands) is
not a good idea.

Second, using repeated strlen calls to grow the string is also not a good idea
(in c_parser_asm_operand).

Third the local variables for c_parser_asm_operand should be moved into the
outer scope.

Fourth, you added the include file tm_p.h to the includes of c-parser.c, but I
did not see a corresponding modification to Makefile.in for this dependency.

Now, getting on the broader scope issues about the patch, I can sympathize with
the desire, but I think this is really opening up a can of worms if we let the
back end rewrite asm constraints like this.  I'm also not sure I understand
what the exact problem is.  I would imagine if you have address modes that are
allowed in some cases, but not others, this is better expressed in
GO_IF_MODE_DEPENDENT_ADDRESS instead of GO_IF_LEGITIMATE_ADDRESS.

-- 
Michael Meissner, AMD
90 Central Street, MS 83-29, Boxborough, MA, 01719, USA
michael.meissner@amd.com


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-05 13:42                     ` Andreas Krebbel
@ 2007-11-06 22:00                       ` Richard Sandiford
  2007-11-07 11:55                         ` Andreas Krebbel
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Sandiford @ 2007-11-06 22:00 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: gcc-patches

"Andreas Krebbel" <Andreas.Krebbel@de.ibm.com> writes:
>> Well, I was thinking of making it a macro constant; I should have picked
>> a name other than TARGET_MEM_CONSTRAINT, sorry.
>> 
>> It might be nice to define the macro indirectly using a new constraints.md
>> construct (define_general_memory_constraint?).  I.e. have genpreds define
>> the macro for you based on the .md file.  The implementation could then
>> move away from macros at the same time as the rest of the constraints
>> stuff does[*].  I think that'd still be easier than converting gcc/*.c
>> to treat 'm' as a hook (and easier than rewriting asms).  But personally,
>> I'd be happy enough if the macro was defined in the target .h file.
>
> One disadvantage of a target macro used in case expressions is that we
> can't check the arch flags here.  So the logic would have to go into
> the constraint definition.  The new constraint letter will have to
> accept the new address format as well as the old one for the new
> architecture and just the old address format for all former archs.

Maybe I'm misunderstanding what you want, but I thought we were
simply talking about renaming the "all valid memory addresses"
constraint from 'm' to some target-specific value.  That renamed
constraint would be treated exactly as 'm' is treated now, without
the need for any special back-end code to handle it.

> If we would have a target hook here which would return the new mem
> constraint letter if the arch flag for the new architecture is set and
> 'm' otherwise, the new constraint letter would not have to depend on
> arch flags what I would personally prefer.
>
> Another disadvantage is that this would prevent usage of multiple
> letter constraints for the standard mem constraint.

True.  Is s390 out of single constraint letters?

> What about the idea of letting the back end override all the existing
> standard constraints using the existing constraint definitions.
> Reload and co could check whether a back end accepts one of the
> standard constraint letters as extra constraint and would fallback to
> the standard behaviour if not.

That sounds OK too, although I'm not the one you need to convince ;)

Richard

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-06 18:20 ` Michael Meissner
@ 2007-11-07  9:10   ` Andreas Krebbel
  0 siblings, 0 replies; 18+ messages in thread
From: Andreas Krebbel @ 2007-11-07  9:10 UTC (permalink / raw)
  To: Michael Meissner; +Cc: gcc-patches

Hi Michael,

thanks for your review.  I'll address the points you mentioned.

> Now, getting on the broader scope issues about the patch, I can sympathize with
> the desire, but I think this is really opening up a can of worms if we let the
> back end rewrite asm constraints like this.  I'm also not sure I understand
> what the exact problem is.  I would imagine if you have address modes that are
> allowed in some cases, but not others, this is better expressed in
> GO_IF_MODE_DEPENDENT_ADDRESS instead of GO_IF_LEGITIMATE_ADDRESS.

In order to support new address formats there is no other way than
extending the GO_IF_LEGITIMATE_ADDRESS hook.  All addresses not
accepted by this hook will be reloaded into a register.

The GO_IF_MODE_DEPENDENT_ADDRESS tells the middle end if an address
should be interpreted depending on the mode of the memory access.
How do you think this can help me here?

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] Target hook for rewriting inline asm constraints
  2007-11-06 22:00                       ` Richard Sandiford
@ 2007-11-07 11:55                         ` Andreas Krebbel
  0 siblings, 0 replies; 18+ messages in thread
From: Andreas Krebbel @ 2007-11-07 11:55 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

> Maybe I'm misunderstanding what you want, but I thought we were
> simply talking about renaming the "all valid memory addresses"
> constraint from 'm' to some target-specific value.  That renamed
> constraint would be treated exactly as 'm' is treated now, without
> the need for any special back-end code to handle it.

I was thinking about the usage of that new "all valid memory
addresses" constraint in the back end.  That constraint letter will
only be usable for instructions which support the old as well as the
new address format which is probably a rare case.  I was hoping that
this could be improved when having a target hook with extra logic
instead of a macro but I don't think anymore that this would help.

Another thing I was worrying about was that for an inline assembly
using the "m" constraint different (maybe worse) code can be generated
when "m" is handled as extra constraint instead of as standard memory
constraint.  That wouldn't be a problem with the target hook returning
"m" for previous archs since the standard "m" codepath would be taken.
But at least for reload this does not seem to be a problem.  The code
handling extra memory constraints matches what is done for the "m"
case.

To sum up I think you've convinced me that the target macro will do
the job and is simpler to implement than the rewriting hook.  As next
step I would like to give the "let the back end redefine standard
constraints" idea a whirl and will come back to your target macro if
this doesn't turn out to be feasible.  Thanks for your suggestions!

> True.  Is s390 out of single constraint letters?

Not yet. But there are not many left.
 
> > What about the idea of letting the back end override all the existing
> > standard constraints using the existing constraint definitions.
> > Reload and co could check whether a back end accepts one of the
> > standard constraint letters as extra constraint and would fallback to
> > the standard behaviour if not.
> 
> That sounds OK too, although I'm not the one you need to convince ;)

I'll post a patch.

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2007-11-07 11:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-30 12:54 [PING] Target hook for rewriting inline asm constraints Andreas Krebbel
2007-10-31 16:31 ` Tom Tromey
2007-10-31 16:43   ` Richard Guenther
2007-10-31 18:55     ` Andreas Krebbel
2007-10-31 19:13       ` Richard Guenther
2007-10-31 20:40         ` Andreas Krebbel
2007-11-04 23:05           ` Richard Sandiford
2007-11-05  9:25             ` Andreas Krebbel
2007-11-05  9:43               ` Richard Sandiford
2007-11-05 10:32                 ` Andreas Krebbel
2007-11-05 11:02                   ` Richard Sandiford
2007-11-05 13:42                     ` Andreas Krebbel
2007-11-06 22:00                       ` Richard Sandiford
2007-11-07 11:55                         ` Andreas Krebbel
2007-10-31 17:22   ` Andreas Krebbel
2007-10-31 17:56     ` Tom Tromey
2007-11-06 18:20 ` Michael Meissner
2007-11-07  9:10   ` Andreas Krebbel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).