Re: Optimization of conditional access to globals: thread-unsafe?
> Frankly, you realise the consequences of volatile access, you have > this comment: > > /* Avoid reading __gthread_active twice on the main code path. */ > int __gthread_active_latest_value = __gthread_active; > > > Now, do you really believe that every multithreaded program should use > volatile, and then should copy shared data to temporal storage, just > because volatile is such a hammer? No, I just wanted to point out that "volatile" has a well-defined semantics and can be properly used for shared accesses. In other words, it's not all or nothing like your earlier message[*] seemed to imply. [*] http://gcc.gnu.org/ml/gcc/2007-10/msg00663.html -- Eric Botcazou
Re: Optimization of conditional access to globals: thread-unsafe?
On Tue, Oct 30, 2007 at 10:59:24 +0300, Tomash Brechko wrote: > On Tue, Oct 30, 2007 at 08:56:08 +0100, Eric Botcazou wrote: > > > The use doesn't become proper simply because it appears in the code, > > > even if in the code of GCC. volatile might be used there for > > > completely different reasons. > > > > No, I put it there for this purpose. > > Then you could remove it, if not for unlocked access. Frankly, you realise the consequences of volatile access, you have this comment: /* Avoid reading __gthread_active twice on the main code path. */ int __gthread_active_latest_value = __gthread_active; Now, do you really believe that every multithreaded program should use volatile, and then should copy shared data to temporal storage, just because volatile is such a hammer? You may have to, with current compilers, but that's not what was supposed by POSIX. -- Tomash Brechko
ARM testsuite failures
I compile gcc trunk from two days ago on ARM (OABI, v3, Debian) for all languages and looked at some of the testsuite failures. I wanted to ask whether people are aware of these issues and working on them before I file PRs. Testsuite results: http://gcc.gnu.org/ml/gcc-testresults/2007-10/msg01361.html gcc --- A number of ICEs like this one: | gcc.c-torture/execute/20050121-1.c:33 (set (reg:HI 0 r0) | (subreg:HI (mem/c:CQI (plus:SI (reg/f:SI 11 fp) | (const_int -24 [0xffe8])) [0 S2 A8]) 0)) -1 (nil)) | gcc.c-torture/execute/20050121-1.c:33: internal compiler error: in extract_insn, at recog.c:1990 Many neon testcases fail with: | arm_neon.h: In function 'test_vdupQ_np16': | arm_neon.h:5385: internal compiler error: in copy_to_mode_reg, at explow.c:644 None of them are reported. Paul/Richard, are you aware of those? libffi -- A number of "execution test" test failures. Then some output failures: | FAIL: libffi.call/cls_3byte1.c -O0 -W -Wall output pattern test, is 1 15 58900 157: 58901 172 | FAIL: libffi.call/cls_3byte1.c -O2 output pattern test, is 1 15 34316 224: 34317 239 | FAIL: libffi.call/cls_2byte.c -O3 output pattern test, is 1 13 4 166: 5 179 | FAIL: libffi.call/cls_3byte1.c -O3 output pattern test, is 1 15 46604 137: 46605 152 | FAIL: libffi.call/cls_3byte1.c -Os output pattern test, is 1 15 58896 184: 58897 199 Who might be interested in taking a look? gfortran gfortran has 7442 unexpected failures. Most of them are due to "test for excess errors". Many are simply because of this: | warning: 'const' attribute directive ignored | warning: 'nothrow' attribute directive ignored which seems to be mentioned in PR21185 (comment #20). Is that problem still on the radar of the gfortran developers? But then we also have e.g.: | gfortran.dg/PR19754_1.f90:7.7-12: |x = x + y ! { dg-error "Shapes for operands at" } | 12 | Error: Shapes for operands at (1) and (2) are not conformable for which I cannot find a PR. Also: | access_spec_2.f90:9.13: | public :: x ! { dg-error "was already specified" } | 1 | Error: ACCESS specification at (1) was already specified and: | access_spec_2.f90:18.19: | integer, public :: y ! { dg-error "Fortran 2003: Attribute PUBLIC" } | 1 | Error: Fortran 2003: Attribute PUBLIC at (1) in a TYPE definition Are these known (but unreported) issues or should I file PRs for them? obj-c++ --- There are a number of longstanding but known issue that show up on all or most architectures: ICE: internal compiler error: tree check: expected tree that contains 'decl with RTL' structure, have 'field_decl' in assemble_external_real, at varasm.c:2220 PR31032 ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in setup_one_parameter, at tree-inline.c:1483 PR23716 -- Martin Michlmayr http://www.cyrius.com/
[RFC PING^2] INSN attribute to enable/disable alternatives
Hi Ian, have you had time to look at this? Or does anyone else like to comment? http://gcc.gnu.org/ml/gcc/2007-10/msg00092.html Bye, -Andreas-
Re: Optimization of conditional access to globals: thread-unsafe?
On Tue, Oct 30, 2007 at 09:20:28 +0100, Eric Botcazou wrote: > No, I just wanted to point out that "volatile" has a well-defined semantics > and can be properly used for shared accesses. In other words, it's not all > or nothing like your earlier message[*] seemed to imply. > > [*] http://gcc.gnu.org/ml/gcc/2007-10/msg00663.html I didn't get your point. Sure volatile can be used _along_ with shared data. But we can't say it _has_ to be used _for_ shared data. I.e. if you require all shared data to be volatile, you can't pass pointer to such data to any function without casting away the qualifier. volatile can be properly used _only_ if you also assume atomicity and cache-coherence, and this is beyond POSIX. But anyway, I'm proving the opposite: when you use POSIX locks, you don't have to use volatile, that it. -- Tomash Brechko
Re: ARM testsuite failures
gfortran has 7442 unexpected failures. Most of them are due to "test for excess errors". Many are simply because of this: | warning: 'const' attribute directive ignored | warning: 'nothrow' attribute directive ignored which seems to be mentioned in PR21185 (comment #20). Is that problem still on the radar of the gfortran developers? I thought it was newlib-specific, and thus did not hurry too much (gfortran for newlib targets is currently not a high priority). Can you reduce one of these failures to a short example and file a PR (and CC me)? Is there something target-specific we should know about arm that could explain that kind of warnings? But then we also have e.g.: | gfortran.dg/PR19754_1.f90:7.7-12: |x = x + y ! { dg-error "Shapes for operands at" } | 12 | Error: Shapes for operands at (1) and (2) are not conformable That can't be a "test for excess error". This is perfectly normal: we are checking that an error is emitted and, apparently, it is. Are you sure this is what is failing? Could you post your testsuite log file (${builddir}/gcc/testsuite/gfortran/gfortran.log) somewhere or send it to us? | access_spec_2.f90:9.13: | public :: x ! { dg-error "was already specified" } | 1 | Error: ACCESS specification at (1) was already specified Same here | access_spec_2.f90:18.19: | integer, public :: y ! { dg-error "Fortran 2003: Attribute PUBLIC" } | 1 | Error: Fortran 2003: Attribute PUBLIC at (1) in a TYPE definition and there. FX
Re: ARM testsuite failures
* FX Coudert <[EMAIL PROTECTED]> [2007-10-30 09:04]: > Can you reduce one of these failures to a short example and file a > PR (and CC me)? PR33947 A simple: program main end program main is enough to trigger it. Note that this doesn't happen with 4.1 and 4.2. > Is there something target-specific we should know about arm that > could explain that kind of warnings? I don't know. > > | Error: Shapes for operands at (1) and (2) are not conformable > That can't be a "test for excess error". This is perfectly normal: > we are checking that an error is emitted and, apparently, it is. Are > you sure this is what is failing? You're right - I misread the error. The test that is looking for the error passes, but at the same time the testcase fails with "test for excess error" because of the warnings mentioned above. -- Martin Michlmayr http://www.cyrius.com/
Re: Optimization of conditional access to globals: thread-unsafe?
I'd like to answer one last argument, mostly for the sake of curious reader, because Michael himself has agreed with (at least the part of) the point. On Mon, Oct 29, 2007 at 16:00:18 +0100, Michael Matz wrote: > The issue is, that people want to write this: > > if (condition) > *p = value; > > (i.e. without any synchronization primitive or in fact anything else after > the store in the control region) and expect that the store indeed only > happens in that control region. And this expectation is misguided. Had > they written it like: > > if (condition) { > *p = value; > membarrier(); > } > > it would have worked just fine. Even if we put aside the fact that there's no such membarrier() equivalent in POSIX bindings, this won't help. First of all, let's note that you can't break the program by making it _more_ ordered. Indeed, program correctness doesn't depend on some particular reordering (you can't predict it anyway), it depends only on some particular ordering. So we can rewrite if (condition) { *p = value; membarrier(); } as if (condition) { *p = value; membarrier(); } else { membarrier(); } But this is the same as if (condition) *p = value; membarrier(); and we are back to the start: the store could me moved outside the condition. In general the following would work if (condition) { *p = value; opaque_function(); } because GCC has to assume that the call may access any memory, thus store to *p can't be moved outside of the condition, because the call itself can't be moved outside. But such a construction can't be the requirement for threaded programming. In the original example there _were_ synchronization primitives already, the complete piece is: if (condition) pthread_mutex_lock(&mutex); ... if (condition) *p = value; ... if (condition) pthread_mutex_unlock(&mutex); and POSIX doesn't require any additional ordering between lock() and unlock(). When condition is false, any speculative store to *p is bogus, because any condition is potentially a 'lock acquired' condition (or 'not read-only' condition). And it was shown that the volatile qualifier can't be applied in general case. But perhaps I'm the only one who is still unsure about the outcome of this discussion :). -- Tomash Brechko
Re: Is gcc thread-unsafe?
On Tue, Oct 30, 2007 at 10:20:34AM +, Andrew Haley wrote: > That's what the proposed standard language says, kinda-sorta. There's > an informal description at > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2338.html. > > Anyway, we have fixed this bug and are committing it to all open gcc > branches. Credit to Ian Taylor for writing the patch. To be precise, it was fixed in one of the optimization passes, there are still others (e.g. loop invariant motion). Jakub
Re: Is gcc thread-unsafe?
Jakub Jelinek writes: > On Tue, Oct 30, 2007 at 10:20:34AM +, Andrew Haley wrote: > > That's what the proposed standard language says, kinda-sorta. There's > > an informal description at > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2338.html. > > > > Anyway, we have fixed this bug and are committing it to all open gcc > > branches. Credit to Ian Taylor for writing the patch. > > To be precise, it was fixed in one of the optimization passes, there > are still others (e.g. loop invariant motion). Ah, thanks for clarifying that. Do you have a test case? Andrew.
Re: Is gcc thread-unsafe?
On Tue, Oct 30, 2007 at 10:31:17AM +, Andrew Haley wrote: > Jakub Jelinek writes: > > On Tue, Oct 30, 2007 at 10:20:34AM +, Andrew Haley wrote: > > > That's what the proposed standard language says, kinda-sorta. There's > > > an informal description at > > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2338.html. > > > > > > Anyway, we have fixed this bug and are committing it to all open gcc > > > branches. Credit to Ian Taylor for writing the patch. > > > > To be precise, it was fixed in one of the optimization passes, there > > are still others (e.g. loop invariant motion). > > Ah, thanks for clarifying that. Do you have a test case? Sure, e.g. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31862#c7 Say on x86_64-linux, foo at -O2 is on the trunk: foo: movlv(%rip), %edx xorl%eax, %eax .p2align 4,,10 .p2align 3 .L3: cmpl%eax, %edi cmovl %esi, %edx addl$1, %eax cmpl$100, %eax jne .L3 movl%edx, v(%rip) ret (note that v is read and written unconditionally), which is a data race, the program as written only reads or writes variable v when holding the mutex m. With -O2 -fno-tree-loop-im we got until yesterday foo: xorl%edx, %edx .p2align 4,,10 .p2align 3 .L3: movlv(%rip), %eax cmpl%edx, %edi cmovl %esi, %eax addl$1, %edx cmpl$100, %edx movl%eax, v(%rip) jne .L3 rep ret which has a data race as well, but with today's trunk (thanks to Ian's patch) we now finally get foo: xorl%eax, %eax .p2align 4,,10 .p2align 3 .L3: cmpl%eax, %edi jge .L2 movl%esi, v(%rip) .L2: addl$1, %eax cmpl$100, %eax jne .L3 rep ret at -O2 -fno-tree-loop-im, which doesn't have the data race. Of course if the compiler was smart enough it could transform the loop into just if (x < 99) v = y; but that's quite unlikely we'll be able to do (and it is questionable if it would be a worthwile optimization in real world). Anyway, -ftree-loop-im breaks this by adding v_lsm.12 below: foo (x, y) { int v_lsm.12; int i; : v_lsm.12_11 = v; : # v_lsm.12_1 = PHI # i_12 = PHI if (x_3(D) < i_12) goto ; else goto ; : v_lsm.12_10 = y_4(D); : # v_lsm.12_7 = PHI i_5 = i_12 + 1; if (i_5 <= 99) goto ; else goto ; : goto ; : # v_lsm.12_15 = PHI v = v_lsm.12_15; return; } which should be allowed only if v is not is_global_var and its address has not been taken. Jakub
Re: [RFC PING^2] INSN attribute to enable/disable alternatives
On Tue, Oct 30, 2007 at 09:34:07AM +0100, Andreas Krebbel wrote: > Hi Ian, > > have you had time to look at this? Or does anyone else like to > comment? > > http://gcc.gnu.org/ml/gcc/2007-10/msg00092.html I have no opinion on the attribute functions which took an alternative parameter. But the enabled alternative seems like a reasonable addition; ARM has the same problem. -- Daniel Jacobowitz CodeSourcery
Re: Optimization of conditional access to globals: thread-unsafe?
> volatile can be properly used _only_ if you also assume atomicity and > cache-coherence, and this is beyond POSIX. But anyway, I'm proving > the opposite: when you use POSIX locks, you don't have to use > volatile, that it. We're not talking about locks, see the example you gave in your first message. -- Eric Botcazou
Re: GCC 4.3 release schedule
Benjamin Kosnik wrote: Jason, any thoughts on how to translate "ready to make a .0 release" into "made a .0 release," in terms of a firm schedule, with dates? I'm assuming that the < 100 bugzilla count is an adequate milestone for the release branch to be cut. Or do you think < 100 implies branch implies 4.3.0 release, as originally described by Richard is the way to go? No. Previously we've branched at <100 regressions, but waited for the numbers to get better than that before making the release. I'm suggesting that the release criteria stay about the same as before, just that we delay the branch until the release is ready rather than have branch criteria and then release criteria. I'm willing to try this. It's just that I would like some advance notice before a release, just to check over things. I would expect Mark's status mails to cover that. Jason
Re: Optimization of conditional access to globals: thread-unsafe?
Tomash Brechko <[EMAIL PROTECTED]> writes: > Even if we put aside the fact that there's no such membarrier() > equivalent in POSIX bindings, this won't help. In POSIX, any mutex function must be a membarrier. For example, on x86, mutex lock and unlock more or less have to execute the mfence instruction. If they don't, the program can see inconsistent data structures despite the mutex operations. > if (condition) { > *p = value; > membarrier(); > } else { > membarrier(); > } > > But this is the same as > > if (condition) > *p = value; > membarrier(); No, it isn't. If membarrier is not a general function call, then it has to be a magic function. In gcc it is implemented using a volatile asm. Note that I've committed my patch to avoid speculative stores to all active branches, so this particular case should be a non-issue going forward. However, we all are going to have to take a careful look at gcc to make sure that it generally conforms to the C++0x memory model. Ian
Re: GCC 4.3 release schedule
Jason Merrill wrote: > No. Previously we've branched at <100 regressions, but waited for the > numbers to get better than that before making the release. I'm > suggesting that the release criteria stay about the same as before, just > that we delay the branch until the release is ready rather than have > branch criteria and then release criteria. Yes, that's what I'm imagining too, under this plan. All we'd be doing differently is delaying Stage 1 until after the release, instead of parallelizing Stage 1 with the final release preparation. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: Optimization of conditional access to globals: thread-unsafe?
On Tue, Oct 30, 2007 at 15:33:56 +0100, Eric Botcazou wrote: > We're not talking about locks, see the example you gave in your > first message. Please read the _description_ that comes along with the code example. Anyways, the patch is there. -- Tomash Brechko
Re: Optimization of conditional access to globals: thread-unsafe?
On Tue, Oct 30, 2007 at 07:50:04 -0700, Ian Lance Taylor wrote: > Tomash Brechko <[EMAIL PROTECTED]> writes: > > > Even if we put aside the fact that there's no such membarrier() > > equivalent in POSIX bindings, this won't help. > > In POSIX, any mutex function must be a membarrier. For example, on > x86, mutex lock and unlock more or less have to execute the mfence > instruction. If they don't, the program can see inconsistent data > structures despite the mutex operations. Yes, but you don't imply I should write if (condition) { *p = value; pthread_mutex_lock(&dummy): pthread_mutex_unlock(&dummy): } just to trigger it. > > if (condition) { > > *p = value; > > membarrier(); > > } else { > > membarrier(); > > } > > > > But this is the same as > > > > if (condition) > > *p = value; > > membarrier(); > > No, it isn't. If membarrier is not a general function call, then it > has to be a magic function. In gcc it is implemented using a volatile > asm. I didn't get your point, but probably you didn't get my either. I was talking about memory barriers as a whole, not a particular implementation in GCC. And my point is that you are free to inject them wherever you like. This will affect performance, but not correctness. Hence you can't be sure membarrier() won't be moved from the condition. > Note that I've committed my patch to avoid speculative stores to all > active branches, so this particular case should be a non-issue going > forward. However, we all are going to have to take a careful look at > gcc to make sure that it generally conforms to the C++0x memory model. I'm not against ending this discussion. As I understand the patch (and I don't grok GCC internals), it fixes both read-only memory case, and race case. But it doesn't try to preserve the optimization in the form that was suggested by Michael Matz (i.e. to use pointer to dummy object on the stack), right? -- Tomash Brechko
Cross compiler on Linux
Hi, I have a question. We are in the process of building a cross compiler for our system on Linux. In the documentation I read that we need the GPL file: "gpl-include.tar.bz2" Do you perhaps know where this file can be obtained so that we can proceed with our build? Best regards, Kees Schipper ICT Production Manager ** For information, services and offers, please visit our web site: http://www.klm.com. This e-mail and any attachment may contain confidential and privileged material intended for the addressee only. If you are not the addressee, you are notified that no part of the e-mail or any attachment may be disclosed, copied or distributed, and that any other action related to this e-mail or attachment is strictly prohibited, and may be unlawful. If you have received this e-mail by error, please notify the sender immediately by return e-mail, and delete this message. Koninklijke Luchtvaart Maatschappij NV (KLM), its subsidiaries and/or its employees shall not be liable for the incorrect or incomplete transmission of this e-mail or any attachments, nor responsible for any delay in receipt. Koninklijke Luchtvaart Maatschappij N.V. (also known as KLM Royal Dutch Airlines) is registered in Amstelveen, The Netherlands, with registered number 33014286 **
Re: Cross compiler on Linux
Schipper, K. - SPLXM writes: > For information, services and offers, please visit our web site: > http://www.klm.com. This e-mail and any attachment may contain > confidential and privileged material intended for the addressee > only. If you are not the addressee, you are notified that no part > of the e-mail or any attachment may be disclosed, copied or > distributed, and that any other action related to this e-mail or > attachment is strictly prohibited, and may be unlawful. If you have > received this e-mail by error, please notify the sender immediately > by return e-mail, and delete this message. > > Koninklijke Luchtvaart Maatschappij NV (KLM), its subsidiaries > and/or its employees shall not be liable for the incorrect or > incomplete transmission of this e-mail or any attachments, nor > responsible for any delay in receipt. > Koninklijke Luchtvaart Maatschappij N.V. (also known as KLM Royal > Dutch Airlines) is registered in Amstelveen, The Netherlands, with > registered number 33014286 Please do not include this sort of disclaimer in messages sent to mailing lists hosted at gcc.gnu.org. They are against list policy as described at http://gcc.gnu.org/lists.html. If you can't remove the disclaimers from your outgoing e-mail messages, I suggest using a free web-based e-mail account. Thanks. Additionally, please direct questions like this to [EMAIL PROTECTED] Everyone else: Please do not reply to mails with disclaimers. Andrew.
Re: Cross compiler on Linux
Schipper, K. - SPLXM writes: > > For information, services and offers, please visit our web site: > > http://www.klm.com. This e-mail and any attachment may contain > > confidential and privileged material intended for the addressee > > only. If you are not the addressee, you are notified that no part > > of the e-mail or any attachment may be disclosed, copied or > > distributed, and that any other action related to this e-mail or > > attachment is strictly prohibited, and may be unlawful. If you have > > received this e-mail by error, please notify the sender immediately > > by return e-mail, and delete this message. On Tue, Oct 30, 2007 at 04:24:39PM +, Andrew Haley wrote: > Please do not include this sort of disclaimer in messages sent to > mailing lists hosted at gcc.gnu.org. That's not a disclaimer. A disclaimer would be a statement like "This message is the opinion of the author only and not the opinion of FooCorp". Disclaimers always say that someone is not responsible for something. Rather, it is a legal threat, and one that forbids us to do what we always automatically do with every message on the list: forward it around the world and publish it on the gcc.gnu.org web site, making it available to people who are not addressees. This kind of legal threat is extremely rude, though many organizations seem to think it is a good idea to threaten people they correspond with for some reason.
Re: Optimization of conditional access to globals: thread-unsafe?
Tomash Brechko <[EMAIL PROTECTED]> writes: > > > if (condition) { > > > *p = value; > > > membarrier(); > > > } else { > > > membarrier(); > > > } > > > > > > But this is the same as > > > > > > if (condition) > > > *p = value; > > > membarrier(); > > > > No, it isn't. If membarrier is not a general function call, then it > > has to be a magic function. In gcc it is implemented using a volatile > > asm. > > I didn't get your point, but probably you didn't get my either. I was > talking about memory barriers as a whole, not a particular > implementation in GCC. And my point is that you are free to inject > them wherever you like. This will affect performance, but not > correctness. Hence you can't be sure membarrier() won't be moved from > the condition. My point is that for a memory barrier to work at all, it has to be magic. And if it is magic, then it can not be moved from the condition. To put it another way, if you can move the memory barrier from the condition, then it is not a memory barrier after all. > > Note that I've committed my patch to avoid speculative stores to all > > active branches, so this particular case should be a non-issue going > > forward. However, we all are going to have to take a careful look at > > gcc to make sure that it generally conforms to the C++0x memory model. > > I'm not against ending this discussion. As I understand the patch > (and I don't grok GCC internals), it fixes both read-only memory case, > and race case. But it doesn't try to preserve the optimization in the > form that was suggested by Michael Matz (i.e. to use pointer to dummy > object on the stack), right? I don't know which suggestion you are referring to. The patch I wrote will retain the optimization in the case where the memory location is unconditionally written later in the function. This is most relevant in that the optimization can take place in a loop, if somewhere after the loop the memory location is unconditionally written. Ian
Re: Optimization of conditional access to globals: thread-unsafe?
On Tue, Oct 30, 2007 at 09:49:00 -0700, Ian Lance Taylor wrote: > I don't know which suggestion you are referring to. The patch I wrote > will retain the optimization in the case where the memory location is > unconditionally written later in the function. This is most relevant > in that the optimization can take place in a loop, if somewhere after > the loop the memory location is unconditionally written. OK, thanks for the description, I just couldn't build GCC after update to see what result looks like. And big Thank You for the patch! -- Tomash Brechko
Re: Optimization of conditional access to globals: thread-unsafe?
> Please read the _description_ that comes along with the code example. I did. > Anyways, the patch is there. The one for ifcvt.c, yes; more will be needed though, see for example http://gcc.gnu.org/ml/gcc/2007-10/msg00754.html -- Eric Botcazou
Fwd: g++ source
Dear Sir, I am a student at the university of Mauritius and doing some research about C++ Programming language... Could you please help me on the source Code??? Could you give us the source code or perhaps help in finding how does the Compiler captures and outputs the errors??? Thaking you in Advance... -- Regards <--Pramod-->
Re: Fwd: g++ source
> I am a student at the university of Mauritius and doing some research > about C++ Programming language... Could you please help me on the > source Code??? Could you give us the source code or perhaps help in > finding how does the Compiler captures and outputs the errors??? You can find the source code linked off the GCC home page, gcc.gnu.org. You might just want to download a release or check out the current source tree using Subversion. Cheers, Ben