Re: Is there a way to look for a tree by its UID?
On Fri, Sep 4, 2020 at 4:36 PM Erick Ochoa
wrote:
>
>
>
> On 04/09/2020 15:19, Richard Biener wrote:
> > On Fri, Sep 4, 2020 at 10:13 AM Erick Ochoa
> > wrote:
> >>
> >>
> >>
> >> On 03/09/2020 12:19, Richard Biener wrote:
> >>> On Thu, Sep 3, 2020 at 10:58 AM Jakub Jelinek via Gcc
> >>> wrote:
>
> On Thu, Sep 03, 2020 at 10:22:52AM +0200, Erick Ochoa wrote:
> > So, I am just wondering is there an interface where I could do something
> > like:
> >
> > ```
> > // vars is the field in pt_solution of type bitmap
> > EXECUTE_IF_SET_IN_BITMAP (vars, 0, uid, bi)
> > {
> > // uid is set
> > tree pointed_to = get_tree_with_uid(uid);
> > }
> > ```
>
> There is not.
> >>>
> >>> And there cannot be since the solution includes UIDs of
> >>> decls that are "fake" and thus never really existed.
> >>
> >> Hi Richard and Jakub,
> >>
> >> thanks, I was looking for why get_tree_with_uid might be a somewhat bad
> >> idea.
> >>
> >> I am thinking about representing an alias set similarly to the
> >> pt_solution. Instead of having bits set in position of points-to
> >> variables UIDs, I was thinking about having bits set in position of
> >> may-alias variables' UIDs. I think an interface similar to the one I
> >> described can provide a good mechanism of jumping to different aliases,
> >> but I do agree that HEAP variables and shadow_variables (and perhaps
> >> other fake variables) might not be a good idea to keep in the interface
> >> to avoid jumping to trees which do not represent something in gimple.
> >>
> >> Richard, you mentioned in another e-mail that I might want to provide
> >> the alias-sets from IPA-PTA to another pass in a similar way to
> >> ipa_escape_pt. I think using a structure similar to pt_solution for
> >> may-alias solution is a good idea. Again, the bitmap to aliasing
> >> variables in UIDs. However, I think for this solution to be general I
> >> need several of these structs not just one. Ideally one per candidate
> >> alias-set, at most one per variable.
> >
> > Sure, you need one per alias-set. Indeed you might want to work
> > with bitmaps of varinfo IDs first when computing alias-sets
> > since ...
>
> Yes, that's what I've been doing :)
>
> >>>
> >>> I think you need to first get a set of candidates you want to
> >>> transform (to limit the work done below), then use the
> >>> internal points-to solutions and compute alias sets for this
> >>> set plus the points-to solution this alias-set aliases. >
> >>> You can then keep the candidate -> alias-set ID -> points-to
> >>> relation (thus candidates should not be "all variables" for
> >>> efficiency reasons).
> >>
> >> I think I can use a relatively simple heuristic: start by looking at
> >> malloc statements and obtain the alias-sets for variables which hold
> >> malloc's return values. This should address most efficiency concerns.
> >>
> >> So, I'm thinking about the following:
> >>
> >> * obtain variables which hold the result of malloc. These are the
> >> initial candidates.
> >
> > ... those would be the is_heapvar ones. Since you can probably
> > only handle the case where all pointers either only point to a
> > single allocation sites result and nothing else or not to it that case
> > looks special and thus easy anyway.
>
> I did a git grep and is_heapvar is gone. But, I believe that I still
> collect these variables as quickly as possible. I iterate over the call
> graph and if I find malloc, then I just look at the callers and collect
> the lhs. This lhs corresponds to the "decl" in the varinfo_t struct.
>
> I then just iterate over the variables in varmap to find matching lhs
> with the decl and computing alias sets by looking at the intersection of
> pt_solution. This seems to work well. I still need to find out whether
> they escape, but it should be simple to do so from here.
>
> >
> >> * for initial candidates compute alias-sets as bitmaps where only "real"
> >> decl UIDs are set. Compute this just before the end of IPA-PTA.
> >> * for each alias_set:
> >> for each alias:
> >> map[alias->decl] = alias_set
> >> * Use this map and the alias-sets bitmaps in pass just after IPA-PTA.
> >> * Potentially use something similar to get_tree_with_uid but that is
> >> only valid for trees which are keys in the map.
> >
> > Hmm, isn't this more than you need? Given a set of candidates C
> > you try to form alias-sets so that all members of an alias set A
> > are members of C, they are layout-compatible and not member
> > of another alias-set. Plus no member escapes and all pointers
> > you can track may only point to a subset of a single alias-set.
>
> Yes? I'm not really sure what you are trying to say here. Can you please
> elaborate more on "this" in the sentence "isn't this more than you need?".
>
> I think I need a way that given some tree (which I know is in
> candidates) I can refer to its alias set. Since the given
Re: A problem with one instruction multiple latencies and pipelines
On Mon, Sep 7, 2020 at 8:10 AM Qian, Jianhua wrote: > > Hi > > I'm adding a new machine model. I have a problem when writing the > "define_insn_reservation" for instruction scheduling. > How to write the "define_insn_reservation" for one instruction that there are > different latencies and pipelines according to parameter. > > For example, the ADD (shifted register) instruction in a64fx > > InstructionOption LatencyPipeline > ADD (shifted register) = 0 1 EX* | EAG* >= [1-4] && =LSL 1+1 (EXA + EXA) > | (EXB + EXB) > 2+1 (EXA + > EXA) | (EXB + EXB) > > In aarch64.md ADD (shifted register) instruction is defined as following． > (define_insn "*add__" > [(set (match_operand:GPI 0 "register_operand" "=r") > (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" "r") > (match_operand:QI 2 "aarch64_shift_imm_" > "n")) > (match_operand:GPI 3 "register_operand" "r")))] > "" > "add\\t%0, %3, %1, %2" > [(set_attr "type" "alu_shift_imm")] > ) > > It could not be distinguished by the type "alu_shift_imm" when writing > "define_insn_reservation" for ADD (shifted register). > What should I do? Just a guess - I'm not very familiar with the pipeline modeling, you probably need to expose two alternatives so you can assign a different type to the second one. Other than that modeling the more restrictive (or permissive?) variant might work good enough in practice. a64fx is probably out-of-order anyway. Richard. > Regards > Qian > > >
Re: Function signatures in extern "C".
Nathan Sidwell wrote: GCC has an extension on machaines with cxx_implicit_extern_c (what used to be !NO_IMPLICIT_EXTERN_C). On such targets we'll treat 'extern "C" void Foo ()' as-if the argument list is variadic. (or something approximating that) perhaps that is confusing things? maybe that’s the underlying reason for failing to diagnose the wrong code. On 9/6/20 4:43 PM, Iain Sandoe wrote: Jonathan Wakely via Gcc wrote: On Sun, 6 Sep 2020 at 16:23, Iain Sandoe wrote: g++.dg/abi/guard3.C has: extern "C" int __cxa_guard_acquire(); Which might not be a suitable declaration, depending on how the ‘extern “C”’ is supposed to affect the function signature generated. IF, the extern C should make this parse as a “K&R” style function - then the TYPE_ARG_TYPES should be NULL (and the testcase is OK). However, we are parsing the decl as int __cxa_guard_acquire(void) (i.e. C++ rules on the empty parens), which makes the testcase not OK. That is the correct parse. Using extern "C" doesn't mean the code is C, it only affects mangling. It still has to follow C++ rules. In practice you can still link to the definition, because its name is just "__cxa_guard_acquire" irrespective of what parameter list is present in the declaration. Linking isn’t the problem in this case. The problem is that we arrive at “expand_call” with a function decl that says f(void) .. and a call parmeter list containing a pointer type. We happily pass the pointer in the place of the ‘void’ - because the code only counts the number of entries and there’s one - so it happens to work. .. that’s not true in the general case and for all calling conventions. that is, “expand_call” does not expect to have to handle the case that the compiler is telling it conflicting information. AFAICT, that’s reasonable, I was unable to find a way to write normal user code [at least, C-family] that the compiler would accept producing this set of conditions (it seems that cases in this category have to be generated by the compiler internally). But PR 45603 is ice-on-invalid triggered by the incorrect declaration of __cxa_guard_acquire. So the incorrect declaration is what originally reproduced the bug, and "fixing" it would make the test useless. Ah OK. So, IIUC we’ve replaced an ICE-on-invalid with an “accepts invalid”, it seems? It's probably worth adding a comment about that in the test. Yes - that would help (will add it to my TODO). Perhaps the PR should be reopened with “accepts invalid”? thanks Iain
RE: A problem with one instruction multiple latencies and pipelines
Hi Richard > -Original Message- > From: Richard Biener > Sent: Monday, September 7, 2020 3:41 PM > To: Qian, Jianhua/钱 建华 > Cc: gcc@gcc.gnu.org > Subject: Re: A problem with one instruction multiple latencies and pipelines > > On Mon, Sep 7, 2020 at 8:10 AM Qian, Jianhua wrote: > > > > Hi > > > > I'm adding a new machine model. I have a problem when writing the > "define_insn_reservation" for instruction scheduling. > > How to write the "define_insn_reservation" for one instruction that there > > are > different latencies and pipelines according to parameter. > > > > For example, the ADD (shifted register) instruction in a64fx > > > > InstructionOption Latency > Pipeline > > ADD (shifted register) = 0 1 EX* > | EAG* > >= [1-4] && =LSL 1+1 > (EXA + EXA) | (EXB + EXB) > > 2+1 (EXA > + EXA) | (EXB + EXB) > > > > In aarch64.md ADD (shifted register) instruction is defined as following． > > (define_insn "*add__" > > [(set (match_operand:GPI 0 "register_operand" "=r") > > (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" > "r") > > (match_operand:QI 2 > "aarch64_shift_imm_" "n")) > > (match_operand:GPI 3 "register_operand" "r")))] > > "" > > "add\\t%0, %3, %1, %2" > > [(set_attr "type" "alu_shift_imm")] > > ) > > > > It could not be distinguished by the type "alu_shift_imm" when writing > "define_insn_reservation" for ADD (shifted register). > > What should I do? > > Just a guess - I'm not very familiar with the pipeline modeling, you probably > need to expose two alternatives so you can assign a different type to the > second > one. I'm considering such method, but if I do that I'm afraid it has side effects on other machine models of aarch64 series. Some instructions' definition will be changed in aarch64.md file. > Other than that modeling the more restrictive (or permissive?) variant might > work good enough in practice. Is your mean that an approximate modeling is good enough? > a64fx is probably out-of-order anyway. Yes, a64fx is an out-of-order architecture. Regards Qian > > Richard. > > > Regards > > Qian > > > > > > >
Re: Function signatures in extern "C".
On Mon, 7 Sep 2020 at 09:18, Iain Sandoe wrote: > > Perhaps the PR should be reopened with “accepts invalid”? My impression from the PR is that the reporter was using a different ABI, where the name isn't reserved. Maybe the testcase should only be accepted with -fno-threadsafe-statics or -ffreestanding or something to say "I'm doing things differently". Or we could just say that G++ reserves the Itanium ABI names unconditionally, even if it doesn't need to use them, in which case it would be accepts-invalid.
Re: Function signatures in extern "C".
On Mon, Sep 07, 2020 at 10:27:13AM +0100, Jonathan Wakely via Gcc wrote: > On Mon, 7 Sep 2020 at 09:18, Iain Sandoe wrote: > > > > Perhaps the PR should be reopened with “accepts invalid”? > > My impression from the PR is that the reporter was using a different > ABI, where the name isn't reserved. Maybe the testcase should only be > accepted with -fno-threadsafe-statics or -ffreestanding or something > to say "I'm doing things differently". > > Or we could just say that G++ reserves the Itanium ABI names > unconditionally, even if it doesn't need to use them, in which case it > would be accepts-invalid. All identifiers starting with two underscores are reserved for the implementation already. Jakub
Re: Function signatures in extern "C".
Jonathan Wakely via Gcc wrote: On Mon, 7 Sep 2020 at 09:18, Iain Sandoe wrote: Perhaps the PR should be reopened with “accepts invalid”? My impression from the PR is that the reporter was using a different ABI, where the name isn't reserved. Maybe the testcase should only be accepted with -fno-threadsafe-statics or -ffreestanding or something to say "I'm doing things differently". Or we could just say that G++ reserves the Itanium ABI names unconditionally, even if it doesn't need to use them, in which case it would be accepts-invalid. Well, it’s a name in the implementation reserved namespace. The majority of GCC platforms executing this test will cause the compiler to generate a call to the function (and that call will have mismatched params). Not sure how many non-itanium ABI platforms we have at present. We say nothing for "-Wall -Wextra -pedantic" (In the end, I don’t have much of an axe to grind here - this fail came up when I added diagnostic code to expand_call to catch cases like this emitted accidentally by the Fortran FE). it seemed worth commenting at least. cheers Iain
Re: Function signatures in extern "C".
On Mon, 7 Sep 2020, 10:34 Jakub Jelinek, wrote: > On Mon, Sep 07, 2020 at 10:27:13AM +0100, Jonathan Wakely via Gcc wrote: > > On Mon, 7 Sep 2020 at 09:18, Iain Sandoe wrote: > > > > > > Perhaps the PR should be reopened with “accepts invalid”? > > > > My impression from the PR is that the reporter was using a different > > ABI, where the name isn't reserved. Maybe the testcase should only be > > accepted with -fno-threadsafe-statics or -ffreestanding or something > > to say "I'm doing things differently". > > > > Or we could just say that G++ reserves the Itanium ABI names > > unconditionally, even if it doesn't need to use them, in which case it > > would be accepts-invalid. > > All identifiers starting with two underscores are reserved for the > implementation already. > Doh, of course. So they'd have to be using some other unsupported ABI which uses that name for a different meaning, which seems like a badly designed ABI given that the "__cxa_" prefix is already claimed by the Itanium ABI. So we shouldn't ICE but any other outcome for that testcase would be ok, including rejecting it.
Re: A problem with one instruction multiple latencies and pipelines
On Mon, Sep 7, 2020 at 10:46 AM Qian, Jianhua wrote: > > Hi Richard > > > -Original Message- > > From: Richard Biener > > Sent: Monday, September 7, 2020 3:41 PM > > To: Qian, Jianhua/钱 建华 > > Cc: gcc@gcc.gnu.org > > Subject: Re: A problem with one instruction multiple latencies and pipelines > > > > On Mon, Sep 7, 2020 at 8:10 AM Qian, Jianhua wrote: > > > > > > Hi > > > > > > I'm adding a new machine model. I have a problem when writing the > > "define_insn_reservation" for instruction scheduling. > > > How to write the "define_insn_reservation" for one instruction that there > > > are > > different latencies and pipelines according to parameter. > > > > > > For example, the ADD (shifted register) instruction in a64fx > > > > > > InstructionOption Latency > > Pipeline > > > ADD (shifted register) = 0 1 EX* > > | EAG* > > >= [1-4] && =LSL 1+1 > > (EXA + EXA) | (EXB + EXB) > > > 2+1 (EXA > > + EXA) | (EXB + EXB) > > > > > > In aarch64.md ADD (shifted register) instruction is defined as following． > > > (define_insn "*add__" > > > [(set (match_operand:GPI 0 "register_operand" "=r") > > > (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" > > "r") > > > (match_operand:QI 2 > > "aarch64_shift_imm_" "n")) > > > (match_operand:GPI 3 "register_operand" "r")))] > > > "" > > > "add\\t%0, %3, %1, %2" > > > [(set_attr "type" "alu_shift_imm")] > > > ) > > > > > > It could not be distinguished by the type "alu_shift_imm" when writing > > "define_insn_reservation" for ADD (shifted register). > > > What should I do? > > > > Just a guess - I'm not very familiar with the pipeline modeling, you > > probably > > need to expose two alternatives so you can assign a different type to the > > second > > one. > I'm considering such method, > but if I do that I'm afraid it has side effects on other machine models of > aarch64 series. > Some instructions' definition will be changed in aarch64.md file. > > > Other than that modeling the more restrictive (or permissive?) variant might > > work good enough in practice. > Is your mean that an approximate modeling is good enough? Yes. > > a64fx is probably out-of-order anyway. > Yes, a64fx is an out-of-order architecture. > > Regards > Qian > > > > > Richard. > > > > > Regards > > > Qian > > > > > > > > > > > > > >
Re: A problem with one instruction multiple latencies and pipelines
"Qian, Jianhua" writes: > Hi > > I'm adding a new machine model. I have a problem when writing the > "define_insn_reservation" for instruction scheduling. > How to write the "define_insn_reservation" for one instruction that there are > different latencies and pipelines according to parameter. > > For example, the ADD (shifted register) instruction in a64fx > > InstructionOption LatencyPipeline > ADD (shifted register) = 0 1 EX* | EAG* >= [1-4] && =LSL 1+1 (EXA + EXA) > | (EXB + EXB) > 2+1 (EXA + > EXA) | (EXB + EXB) > > In aarch64.md ADD (shifted register) instruction is defined as following． > (define_insn "*add__" > [(set (match_operand:GPI 0 "register_operand" "=r") > (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" "r") > (match_operand:QI 2 "aarch64_shift_imm_" > "n")) > (match_operand:GPI 3 "register_operand" "r")))] > "" > "add\\t%0, %3, %1, %2" > [(set_attr "type" "alu_shift_imm")] > ) > > It could not be distinguished by the type "alu_shift_imm" when writing > "define_insn_reservation" for ADD (shifted register). > What should I do? This is just personal opinion, but in general (from the point of view of a new port, or a new subport like SVE), I think the best approach to handling the "type" attribute is to start with the coarsest classification that makes sense, then split these relatively coarse types up whenever there's a specific need. When taking that approach, it's OK (and perhaps even a good sign) for an existing type to sometimes be too coarse for a new CPU. So thanks for asking about this, and please don't feel constrained by the existing "type" classification. IMO we should split existing types wherever that makes sense for new CPUs. In a situation like this, I guess it would make sense to treat alu_shift_imm as a conservative general classification and add alu_shift_imm1to4 (say) as a new type. I'd recommend a two-stage approach, with each stage as a separate patch for ease of review: (1) Add the new type to the definition of the "type" attribute and run: sed -i s/alu_shift_imm/alu_shift_imm1to4,alu_shift_imm/g on all the config/arm and config/aarch64 tuning descriptions. This is a purely mechanical replacement and could be tested in the normal way (i.e. there would be no need to do special testing to make sure that existing descriptions are unaffected: the change is mechanical enough not to need that). (2) Make the define_insns use the new type where appropriate. Like Richard says, this would be handled by splitting the single define_insn alternative above into two separate alternatives. It's also possible to something like: (set (attr "type") (if_then_else (match_operand ...) (const_string "...") (const_string "..."))) which localises the change to the attribute definition. If we end up needing that construct several times, it would also be possible to add a new attribute, e.g.: (define_attr "shift_imm_type" "none,alu_shift_op2") and then change the default value of the type attribute from: (const_string "untyped") to: (cond [(eq_attr "shift_imm_type" "alu_shift_op2") (if_then_else (match_operand 2 ...) (const_string "...") (const_string "..."))] (const_string "untyped")) define_insns like the above could then use: (set_attr "shift_imm_type" "alu_shift_op2") instead of setting "type" directly. That's definitely more complicated, but it can help to reduce cut-&-paste. Thanks, Richard
Why was it important to change "FALLTHROUGH" to "fall through"?
I don't write a lot of code anymore, but this sure seems like a gratuitous irritation to me. I've been using // FALLTHRU and // FALLTHROUGH for *DECADES*, so it's pretty incomprehensible why the compiler should have to invalidate my code because it thinks a different coding comment is better.
Re: Why was it important to change "FALLTHROUGH" to "fall through"?
* Bruce Korb via Gcc: > I don't write a lot of code anymore, but this sure seems like a > gratuitous irritation to me. I've been using > > // FALLTHRU and > // FALLTHROUGH > > for *DECADES*, so it's pretty incomprehensible why the compiler should > have to invalidate my code because it thinks a different coding > comment is better. It's not clear what you are talking about. Presumably you placed the comment before a closing brace, and not immediately before the subsequent case label?
Re: Why was it important to change "FALLTHROUGH" to "fall through"?
On Mon, Sep 7, 2020 at 3:45 PM Florian Weimer wrote: > > * Bruce Korb via Gcc: > > > I don't write a lot of code anymore, but this sure seems like a > > gratuitous irritation to me. I've been using > > > > // FALLTHRU and > > // FALLTHROUGH > > > > for *DECADES*, so it's pretty incomprehensible why the compiler should > > have to invalidate my code because it thinks a different coding > > comment is better. > > It's not clear what you are talking about. > > Presumably you placed the comment before a closing brace, and not > immediately before the subsequent case label? Nope. I had /* FALLTHROUGH */ on the line before a blank line before the case label. After Googling, I found an explicit reference that you had to spell it: // fall through I did that, and it worked. So I'm moving on, but still ...
RE: A problem with one instruction multiple latencies and pipelines
Hi Richard Thanks a lot for your advises and detailed comments. We will discuss which instructions need to be accurately classified, and estimate the workload. Regards Qian > -Original Message- > From: Richard Sandiford > Sent: Tuesday, September 8, 2020 4:21 AM > To: Qian, Jianhua/钱 建华 > Cc: gcc@gcc.gnu.org > Subject: Re: A problem with one instruction multiple latencies and pipelines > > "Qian, Jianhua" writes: > > Hi > > > > I'm adding a new machine model. I have a problem when writing the > "define_insn_reservation" for instruction scheduling. > > How to write the "define_insn_reservation" for one instruction that there > > are > different latencies and pipelines according to parameter. > > > > For example, the ADD (shifted register) instruction in a64fx > > > > InstructionOption Latency > Pipeline > > ADD (shifted register) = 0 1 EX* > | EAG* > >= [1-4] && =LSL 1+1 > (EXA + EXA) | (EXB + EXB) > > 2+1 (EXA > + EXA) | (EXB + EXB) > > > > In aarch64.md ADD (shifted register) instruction is defined as following． > > (define_insn "*add__" > > [(set (match_operand:GPI 0 "register_operand" "=r") > > (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" > "r") > > (match_operand:QI 2 > "aarch64_shift_imm_" "n")) > > (match_operand:GPI 3 "register_operand" "r")))] > > "" > > "add\\t%0, %3, %1, %2" > > [(set_attr "type" "alu_shift_imm")] > > ) > > > > It could not be distinguished by the type "alu_shift_imm" when writing > "define_insn_reservation" for ADD (shifted register). > > What should I do? > > This is just personal opinion, but in general (from the point of view of a > new port, > or a new subport like SVE), I think the best approach to handling the "type" > attribute is to start with the coarsest classification that makes sense, then > split > these relatively coarse types up whenever there's a specific need. > > When taking that approach, it's OK (and perhaps even a good sign) for an > existing type to sometimes be too coarse for a new CPU. > > So thanks for asking about this, and please don't feel constrained by the > existing "type" classification. IMO we should split existing types wherever > that makes sense for new CPUs. > > In a situation like this, I guess it would make sense to treat alu_shift_imm > as a > conservative general classification and add > alu_shift_imm1to4 (say) as a new type. > > I'd recommend a two-stage approach, with each stage as a separate patch for > ease of review: > > (1) Add the new type to the definition of the "type" attribute and run: > > sed -i s/alu_shift_imm/alu_shift_imm1to4,alu_shift_imm/g > > on all the config/arm and config/aarch64 tuning descriptions. > > This is a purely mechanical replacement and could be tested in > the normal way (i.e. there would be no need to do special testing > to make sure that existing descriptions are unaffected: the change > is mechanical enough not to need that). > > (2) Make the define_insns use the new type where appropriate. > Like Richard says, this would be handled by splitting the single > define_insn alternative above into two separate alternatives. > It's also possible to something like: > > (set (attr "type") >(if_then_else (match_operand ...) > (const_string "...") > (const_string "..."))) > > which localises the change to the attribute definition. > > If we end up needing that construct several times, it would also > be possible to add a new attribute, e.g.: > > (define_attr "shift_imm_type" "none,alu_shift_op2") > > and then change the default value of the type attribute from: > > (const_string "untyped") > > to: > > (cond [(eq_attr "shift_imm_type" "alu_shift_op2") > (if_then_else (match_operand 2 ...) >(const_string "...") >(const_string "..."))] > (const_string "untyped")) > > define_insns like the above could then use: > > (set_attr "shift_imm_type" "alu_shift_op2") > > instead of setting "type" directly. That's definitely more > complicated, but it can help to reduce cut-&-paste. > > Thanks, > Richard >
