* [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: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-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 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
* 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-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-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
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).