Re: RFC: Hack to make restrict more useful
On 9/3/07, Daniel Berlin [EMAIL PROTECTED] wrote: On 9/2/07, Paul Brook [EMAIL PROTECTED] wrote: That said, second, my understanding of restrict, from reading the c99 standard, is that it is perfectly valid for restrict pointers to alias each other during *loads*.. IE you can guarantee any restricted pointer that is stored into can't alias the other restricted pointers. Those only used for loads can alias each other. How does it make a difference? If for two pointers that are only loaded from we say they don't alias I cannot imagine a transformation that would get anything wrong. Easy answer: Dependence testing and then loop transforms. Given p[i] = a[i] + b[i], if you claim a and b can't alias, you will now claim that a[i] and b[i] don't access the same memory on the same iteration. We could easily use this and some profit estimation to decide to say, change the iteration space of a but not b,which will break since they really do alias, and breaking this is bad because they are allowed to. Eh? Maybe I'm blind, but how can a change in iteration space that is valid for the non-aliasing case be invalid for the aliasing case _if we do not modify any data_? Ok, Marks example is the only one we could get wrong. But we simply can not derive value ranges from restrict qualification. Richard.
Re: RFC: Hack to make restrict more useful
On Sun, 2 Sep 2007, Mark Mitchell wrote: Daniel Berlin wrote: Again, I'd love to just ignore this and say we don't care. Ugh. I think you're right that the standard says that we only get to assume non-aliasing when the pointed-to memory is modified, so all-parameters-restrict is actually weaker than -fargument-noalias. How unfortunate. I've CC'd Joseph in the hopes that his C standards knowledge will suggest a different answer. The rules that unmodified memory may alias were a deliberate change in the FDIS relative to the previous public draft; see http://www.open-std.org/jtc1/sc22/wg14/www/docs/n866.htm: 24 1. The FCD specification of restrict forbids aliasing of 25 unmodified objects. Doing so does not promote optimization, 26 and has other disadvantages, which are discussed in examples 27 A-E below. It is also contrary to the prior art in Fortran. -- Joseph S. Myers [EMAIL PROTECTED]
Re: RFC: Hack to make restrict more useful
Joseph S. Myers wrote: The rules that unmodified memory may alias were a deliberate change in the FDIS relative to the previous public draft; see http://www.open-std.org/jtc1/sc22/wg14/www/docs/n866.htm: That explains why I had no memory of this, despite having researched restrict pretty carefully in earlier drafts. That also explains why other compilers in the field implements restrict as meaning does not alias, independent of what's modified. Danny, does your more comprehensive treatment of restrict still optimize test cases like the one in the PR I filed? In any case, I guess we should consider my patch withdrawn. Although, if the new meaning of restrict matches standard Fortran semantics, then our Fortran handling must be wrong, since all my patch did was make us match our current Fortran semantics. Thanks, -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: RFC: Hack to make restrict more useful
In any case, I guess we should consider my patch withdrawn. Although, if the new meaning of restrict matches standard Fortran semantics, then our Fortran handling must be wrong, since all my patch did was make us match our current Fortran semantics. In Fortran the pointers are not exposed at the language level, so it's probably much harder to construct a cases where this matters. Paul
Re: RFC: Hack to make restrict more useful
Mark Mitchell wrote: Joseph S. Myers wrote: The rules that unmodified memory may alias were a deliberate change in the FDIS relative to the previous public draft; see http://www.open-std.org/jtc1/sc22/wg14/www/docs/n866.htm: That explains why I had no memory of this, despite having researched restrict pretty carefully in earlier drafts. That also explains why other compilers in the field implements restrict as meaning does not alias, independent of what's modified. Danny, does your more comprehensive treatment of restrict still optimize test cases like the one in the PR I filed? Test cases of mine, which fail to optimize, involve a scalar argument, performing arithmetic between a scalar and an array. In the C case, -fargument-noalias brings optimization, which is not promoted by restrict keyword. Using an explicit local copy of the argument should work as well. Could that be a reason for avoiding the optimization? I think the likely use of restrict by various compilers comes into play only when a possibly aliased variable is modified, as pointed out previously. Such optimization could result in broken results anyway, wwhen array bounds violations occur, so I guess there's no way to check, other than by extending bounds checking.
Re: RFC: Hack to make restrict more useful
On 9/3/07, Richard Guenther [EMAIL PROTECTED] wrote: On 9/3/07, Daniel Berlin [EMAIL PROTECTED] wrote: On 9/2/07, Paul Brook [EMAIL PROTECTED] wrote: That said, second, my understanding of restrict, from reading the c99 standard, is that it is perfectly valid for restrict pointers to alias each other during *loads*.. IE you can guarantee any restricted pointer that is stored into can't alias the other restricted pointers. Those only used for loads can alias each other. How does it make a difference? If for two pointers that are only loaded from we say they don't alias I cannot imagine a transformation that would get anything wrong. Easy answer: Dependence testing and then loop transforms. Given p[i] = a[i] + b[i], if you claim a and b can't alias, you will now claim that a[i] and b[i] don't access the same memory on the same iteration. We could easily use this and some profit estimation to decide to say, change the iteration space of a but not b,which will break since they really do alias, and breaking this is bad because they are allowed to. Eh? Maybe I'm blind, but how can a change in iteration space that is valid for the non-aliasing case be invalid for the aliasing case _if we do not modify any data_? You may be right, but it just means we have to be very careful where we use the data if there are no modifications. I'm not sure the best way to go about this. Right now, i attached restrict info to SSA_NAME's, and we use it in access_can_touch_variable, may_alias_p, and the dataref version of this. Sadly though, it also means we can't use restricted pointers to say anything about non-restricted pointers unless their is modification either. IE int foo(int *a, restrict *b), doesn't guarantee a and b don't alias unless there is a modification of one of them. --Dan
Re: RFC: Hack to make restrict more useful
On 9/3/07, Daniel Berlin [EMAIL PROTECTED] wrote: On 9/3/07, Richard Guenther [EMAIL PROTECTED] wrote: On 9/3/07, Daniel Berlin [EMAIL PROTECTED] wrote: On 9/2/07, Paul Brook [EMAIL PROTECTED] wrote: That said, second, my understanding of restrict, from reading the c99 standard, is that it is perfectly valid for restrict pointers to alias each other during *loads*.. IE you can guarantee any restricted pointer that is stored into can't alias the other restricted pointers. Those only used for loads can alias each other. How does it make a difference? If for two pointers that are only loaded from we say they don't alias I cannot imagine a transformation that would get anything wrong. Easy answer: Dependence testing and then loop transforms. Given p[i] = a[i] + b[i], if you claim a and b can't alias, you will now claim that a[i] and b[i] don't access the same memory on the same iteration. We could easily use this and some profit estimation to decide to say, change the iteration space of a but not b,which will break since they really do alias, and breaking this is bad because they are allowed to. Eh? Maybe I'm blind, but how can a change in iteration space that is valid for the non-aliasing case be invalid for the aliasing case _if we do not modify any data_? You may be right, but it just means we have to be very careful where we use the data if there are no modifications. I'm not sure the best way to go about this. Right now, i attached restrict info to SSA_NAME's, and we use it in access_can_touch_variable, may_alias_p, and the dataref version of this. I think this should be ok. Sadly though, it also means we can't use restricted pointers to say anything about non-restricted pointers unless their is modification either. IE int foo(int *a, restrict *b), doesn't guarantee a and b don't alias unless there is a modification of one of them. Well, that is probably to make handling of contrieved derivations of a restrict pointer conservatively correct. That is, for example restrict int *p; int pi = (int)p; *(int *)p = 1; just assuming that there are derivations that PTA cannot deal with. The way the standard is written it's just for a safe default. Richard.
Re: RFC: Hack to make restrict more useful
On 9/1/07, Mark Mitchell [EMAIL PROTECTED] wrote: Richard Guenther wrote: OK, great. Here's a draft patch for the trick; this works on the test case I had, and I'll be testing it now. If it passes testing, and I add testcases, does this look OK to you? Thanks for the speedy and accurate review! + bool noalias; it's an int. Doh, fixed. + /* For each incoming pointer argument arg, ARG = ESCAPED_VARS or a + dummy variable if flag_argument_noalias 2. */ What's this comment for? Brain leakage. I cut-and-paste that from an older version; sorry, will remove. + if (POINTER_TYPE_P (TREE_TYPE (t)) noalias) noalias 0 I suppose. If you like. (Since noalias is never negative, this is logically equivalent.) I've made the change. { varinfo_t vi; tree heapvar = heapvar_lookup (t); @@ -4579,7 +4583,7 @@ intra_create_variable_infos (void) heapvar_insert (t, heapvar); ann = get_var_ann (heapvar); - if (flag_argument_noalias == 1) + if (flag_argument_noalias = 1) ann-noalias_state = NO_ALIAS; else if (flag_argument_noalias == 2) ann-noalias_state = NO_ALIAS_GLOBAL; That looks wrong. Shouldn't this just replace flag_argument_noalias for noalias everywhere? Either will work, but you're right; using noalias is clearer. (In an earlier version I'd not organized things in the same way, so that wouldn't have worked.) I'll make those changes too. As you suggest, I'll finish up testing and wait for Danny's comments. Two comments. First, the work I have subsumes this, but will be a week or two. Your work certainly won't hurt in case i get hit by a bus, and i'll happily just remove it when it becomes obsolete :) That said, second, my understanding of restrict, from reading the c99 standard, is that it is perfectly valid for restrict pointers to alias each other during *loads*.. IE you can guarantee any restricted pointer that is stored into can't alias the other restricted pointers. Those only used for loads can alias each other. This comes from 6.7.3.1 #4, which says During each execution of B, let L be any lvalue that has L based on P. If L is used to access the value of the object X that it designates, and X is also *modified* (by any means), ... the following requirements apply:...[part about not being able to point to same object, basically] They even given an example of this (two restricted pointers to arrays aliasing) happening. It this an incorrect reading? I'll happily admit that reading standards makes my brain hurt, it's worse than reading caselaw, so i'd love to be wrong. I'd also love to simply ignore this part, because i don't believe anyone who has actually *used* restrict thinks it means anything other than this can't alias my other restrict pointers. I'm also pretty sure no other compiler implements this restriction anyway, as it removes a *lot* of useful cases of restrict. We certainly get it wrong. :) --Dan
Re: RFC: Hack to make restrict more useful
On 9/3/07, Daniel Berlin [EMAIL PROTECTED] wrote: That said, second, my understanding of restrict, from reading the c99 standard, is that it is perfectly valid for restrict pointers to alias each other during *loads*.. IE you can guarantee any restricted pointer that is stored into can't alias the other restricted pointers. Those only used for loads can alias each other. This comes from 6.7.3.1 #4, which says During each execution of B, let L be any lvalue that has L based on P. If L is used to access the value of the object X that it designates, and X is also *modified* (by any means), ... the following requirements apply:...[part about not being able to point to same object, basically] They even given an example of this (two restricted pointers to arrays aliasing) happening. It this an incorrect reading? I'll happily admit that reading standards makes my brain hurt, it's worse than reading caselaw, so i'd love to be wrong. I'd also love to simply ignore this part, because i don't believe anyone who has actually *used* restrict thinks it means anything other than this can't alias my other restrict pointers. I'm also pretty sure no other compiler implements this restriction anyway, as it removes a *lot* of useful cases of restrict. We certainly get it wrong. :) How does it make a difference? If for two pointers that are only loaded from we say they don't alias I cannot imagine a transformation that would get anything wrong. For example EXAMPLE 2 clearly suggests that not both pointers need to be written to. That is, EXAMPLE 3 is merely to make it defined behavior from a standards point of view, even if it doesn't make a difference in practice. But, IANTL. ;) Richard.
Re: RFC: Hack to make restrict more useful
That said, second, my understanding of restrict, from reading the c99 standard, is that it is perfectly valid for restrict pointers to alias each other during *loads*.. IE you can guarantee any restricted pointer that is stored into can't alias the other restricted pointers. Those only used for loads can alias each other. How does it make a difference? If for two pointers that are only loaded from we say they don't alias I cannot imagine a transformation that would get anything wrong. For example EXAMPLE 2 clearly suggests that not both pointers need to be written to. That is, EXAMPLE 3 is merely to make it defined behavior from a standards point of view, even if it doesn't make a difference in practice. I share Richard's confusion. Fortran aliasing rules also allow two arguments to alias if neither of then are modified. In F90 you have intent attributes to help the compiler enforce this, in older revisions (and if these attributes are not specified) it's up to the user to ensure this is true. Paul
Re: RFC: Hack to make restrict more useful
On 9/2/07, Paul Brook [EMAIL PROTECTED] wrote: That said, second, my understanding of restrict, from reading the c99 standard, is that it is perfectly valid for restrict pointers to alias each other during *loads*.. IE you can guarantee any restricted pointer that is stored into can't alias the other restricted pointers. Those only used for loads can alias each other. How does it make a difference? If for two pointers that are only loaded from we say they don't alias I cannot imagine a transformation that would get anything wrong. Easy answer: Dependence testing and then loop transforms. Given p[i] = a[i] + b[i], if you claim a and b can't alias, you will now claim that a[i] and b[i] don't access the same memory on the same iteration. We could easily use this and some profit estimation to decide to say, change the iteration space of a but not b,which will break since they really do alias, and breaking this is bad because they are allowed to. Again, I'd love to just ignore this and say we don't care. But we should not do it by saying oh, well, this could never effect anything. It could. You can come up with transformations that break, and they all involve the compiler claiming a and b can't alias when they can.
Re: RFC: Hack to make restrict more useful
Daniel Berlin wrote: Again, I'd love to just ignore this and say we don't care. Ugh. I think you're right that the standard says that we only get to assume non-aliasing when the pointed-to memory is modified, so all-parameters-restrict is actually weaker than -fargument-noalias. How unfortunate. I've CC'd Joseph in the hopes that his C standards knowledge will suggest a different answer. But we should not do it by saying oh, well, this could never effect anything. It could. You can come up with transformations that break, and they all involve the compiler claiming a and b can't alias when they can. Indeed. The most obvious example is: return a != b; If the compiler knows the pointers don't alias, the compiler will happily, but wrongly, fold that to 1. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: RFC: Hack to make restrict more useful
On 9/1/07, Mark Mitchell [EMAIL PROTECTED] wrote: This bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33272 is about a situation in which -fargument-noalias works better than putting restrict on all pointer arguments to a function, even though that should be logically equivalent. Using restrict for all arguments to a function is probably one of the most common cases of restrict; that's what you want for things like the test case I posted, and for other Fortran-ish code. I have a prototype hack which changes checks of flag_argument_noalias != 0 to also check for the presence of restrict on all pointer arguments. This fixes the test case, modulo a C front-end bug which Joseph has volunteered to fix. To make that a real patch, here's what I plan to do: (1) Add a flag to struct function to say all pointer arguments are restrict. (2) Lazily set it, when something wants to check it. (3) Change checks of flag_argument_noalias to call a function argument_noalias() which will return an int with the same meaning as flag_argument_noalias. Does that plan sound OK to folks? AFAIK Danny had been fixing restrict on his working agenda lately. No idea what the status on that is, though. Richard.
Re: RFC: Hack to make restrict more useful
Richard Guenther wrote: I have a prototype hack which changes checks of flag_argument_noalias != 0 to also check for the presence of restrict on all pointer arguments. This fixes the test case, modulo a C front-end bug which Joseph has volunteered to fix. AFAIK Danny had been fixing restrict on his working agenda lately. No idea what the status on that is, though. I fully concede that my trick isn't a general solution to making full use of restrict. But, given that I think it'll take about 20-50 lines of code, and that it will get a lot of the common cases, I think it's worth it. Do you agree? Thanks, -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: RFC: Hack to make restrict more useful
On 9/1/07, Mark Mitchell [EMAIL PROTECTED] wrote: Richard Guenther wrote: I have a prototype hack which changes checks of flag_argument_noalias != 0 to also check for the presence of restrict on all pointer arguments. This fixes the test case, modulo a C front-end bug which Joseph has volunteered to fix. AFAIK Danny had been fixing restrict on his working agenda lately. No idea what the status on that is, though. I fully concede that my trick isn't a general solution to making full use of restrict. But, given that I think it'll take about 20-50 lines of code, and that it will get a lot of the common cases, I think it's worth it. Do you agree? Yes, I agree. I just was curious on the status of Dannys work and if it would obsolete what you propose. Richard.
Re: RFC: Hack to make restrict more useful
Richard Guenther wrote: I fully concede that my trick isn't a general solution to making full use of restrict. But, given that I think it'll take about 20-50 lines of code, and that it will get a lot of the common cases, I think it's worth it. Do you agree? Yes, I agree. I just was curious on the status of Dannys work and if it would obsolete what you propose. OK, great. Here's a draft patch for the trick; this works on the test case I had, and I'll be testing it now. If it passes testing, and I add testcases, does this look OK to you? Thanks, -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713 2007-09-01 Mark Mitchell [EMAIL PROTECTED] * function.h (struct function): Add argument_noalias. * alias.h (argument_noalias): Declare it. * alias.c (argument_noalias): New function. (base_alias_check): Use it. * tree-ssa-structalias.c (intra_create_variable_infos): Use argument_noalias. Index: function.h === --- function.h (revision 127950) +++ function.h (working copy) @@ -473,6 +473,12 @@ struct function GTY(()) function has been gimplified, so we can make sure we're not creating non GIMPLE tuples after gimplification. */ unsigned gimplified : 1; + + /* 0 if we have not yet determined whether arguments to this + function might alias each other. 1 if we have determined they + may alias each other. 2 if we have determined they cannot alias + each other. */ + unsigned argument_noalias : 2; }; /* If va_list_[gf]pr_size is set to this, it means we don't know how Index: alias.c === --- alias.c (revision 127950) +++ alias.c (working copy) @@ -1452,6 +1452,56 @@ find_base_term (rtx x) } } +/* Returns an integer indicating to what extent arguments to the + current function may alias one-another or global variables. In + particular, the return value is: + + 0 if pointer arguments may alias each other. + 1 if pointer arguments may not alias each other but may alias + global variables. + 2 if pointer arguments may not alias each other and may not + alias global variables. + 3 if pointer arguments may not alias anything. + + Unlike FLAG_ARGUMENT_NOALIAS, the value returned may differ + depending on the function presently being processed. */ + +int +argument_noalias (void) +{ + tree t; + + /* If FLAG_ARGUMENT_NOALIAS tells us that arguments cannot alias, we + do not have to try to prove that ourselves. And, if we're + outside of a function, there's nothing we can prove. */ + if (flag_argument_noalias || !cfun) +return flag_argument_noalias; + + /* If we have not already determined whether this function's + arguments can alias each other, figure that out now. */ + if (!cfun-argument_noalias) +{ + /* If all the arguments to the current function are pointers with +the restrict qualifier, then none of them can point to one +another. Assume that's the case. */ + cfun-argument_noalias = 2; + for (t = DECL_ARGUMENTS (current_function_decl); t; t = TREE_CHAIN (t)) + { + tree type = TREE_TYPE (t); + /* If we find a pointer argument that does not have the +restrict qualifier, then some of the arguments to this +function might alias other arguments. */ + if (POINTER_TYPE_P (type) !TYPE_RESTRICT (type)) + { + cfun-argument_noalias = 1; + break; + } + } +} + + return cfun-argument_noalias - 1; +} + /* Return 0 if the addresses X and Y are known to point to different objects, 1 if they might be pointers to the same object. */ @@ -1461,6 +1511,7 @@ base_alias_check (rtx x, rtx y, enum mac { rtx x_base = find_base_term (x); rtx y_base = find_base_term (y); + int noalias; /* If the address itself has no known base see if a known equivalent value has one. If either address still has no known base, nothing @@ -1521,10 +1572,11 @@ base_alias_check (rtx x, rtx y, enum mac || (GET_CODE (y_base) == ADDRESS GET_MODE (y_base) == Pmode)) return 0; - if (! flag_argument_noalias) + noalias = argument_noalias (); + if (! noalias) return 1; - if (flag_argument_noalias 1) + if (noalias 1) return 0; /* Weak noalias assertion (arguments are distinct, but may match globals). */ Index: alias.h === --- alias.h (revision 127950) +++ alias.h (working copy) @@ -28,6 +28,7 @@ extern alias_set_type get_varargs_alias_ extern alias_set_type get_frame_alias_set (void); extern bool component_uses_parent_alias_set (const_tree); extern bool alias_set_subset_of (alias_set_type, alias_set_type); +extern int argument_noalias (void); /* This alias set can be used to force
Re: RFC: Hack to make restrict more useful
Richard Guenther wrote: OK, great. Here's a draft patch for the trick; this works on the test case I had, and I'll be testing it now. If it passes testing, and I add testcases, does this look OK to you? Thanks for the speedy and accurate review! + bool noalias; it's an int. Doh, fixed. + /* For each incoming pointer argument arg, ARG = ESCAPED_VARS or a + dummy variable if flag_argument_noalias 2. */ What's this comment for? Brain leakage. I cut-and-paste that from an older version; sorry, will remove. + if (POINTER_TYPE_P (TREE_TYPE (t)) noalias) noalias 0 I suppose. If you like. (Since noalias is never negative, this is logically equivalent.) I've made the change. { varinfo_t vi; tree heapvar = heapvar_lookup (t); @@ -4579,7 +4583,7 @@ intra_create_variable_infos (void) heapvar_insert (t, heapvar); ann = get_var_ann (heapvar); - if (flag_argument_noalias == 1) + if (flag_argument_noalias = 1) ann-noalias_state = NO_ALIAS; else if (flag_argument_noalias == 2) ann-noalias_state = NO_ALIAS_GLOBAL; That looks wrong. Shouldn't this just replace flag_argument_noalias for noalias everywhere? Either will work, but you're right; using noalias is clearer. (In an earlier version I'd not organized things in the same way, so that wouldn't have worked.) I'll make those changes too. As you suggest, I'll finish up testing and wait for Danny's comments. Thanks! -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: RFC: Hack to make restrict more useful
On 9/1/07, Mark Mitchell [EMAIL PROTECTED] wrote: Richard Guenther wrote: I fully concede that my trick isn't a general solution to making full use of restrict. But, given that I think it'll take about 20-50 lines of code, and that it will get a lot of the common cases, I think it's worth it. Do you agree? Yes, I agree. I just was curious on the status of Dannys work and if it would obsolete what you propose. OK, great. Here's a draft patch for the trick; this works on the test case I had, and I'll be testing it now. If it passes testing, and I add testcases, does this look OK to you? --- tree-ssa-structalias.c (revision 127950) +++ tree-ssa-structalias.c (working copy) @@ -4544,7 +4544,12 @@ intra_create_variable_infos (void) { tree t; struct constraint_expr lhs, rhs; + bool noalias; it's an int. + noalias = argument_noalias (); + + /* For each incoming pointer argument arg, ARG = ESCAPED_VARS or a + dummy variable if flag_argument_noalias 2. */ What's this comment for? /* For each incoming pointer argument arg, create the constraint ARG = ANYTHING or a dummy variable if flag_argument_noalias is set. */ for (t = DECL_ARGUMENTS (current_function_decl); t; t = TREE_CHAIN (t)) @@ -4554,11 +4559,10 @@ intra_create_variable_infos (void) if (!could_have_pointers (t)) continue; - /* If flag_argument_noalias is set, then function pointer -arguments are guaranteed not to point to each other. In that -case, create an artificial variable PARM_NOALIAS and the -constraint ARG = PARM_NOALIAS. */ - if (POINTER_TYPE_P (TREE_TYPE (t)) flag_argument_noalias 0) + /* If the arguments are guaranteed not to point to each other, +create an artificial variable PARM_NOALIAS and the constraint +ARG = PARM_NOALIAS. */ + if (POINTER_TYPE_P (TREE_TYPE (t)) noalias) noalias 0 I suppose. { varinfo_t vi; tree heapvar = heapvar_lookup (t); @@ -4579,7 +4583,7 @@ intra_create_variable_infos (void) heapvar_insert (t, heapvar); ann = get_var_ann (heapvar); - if (flag_argument_noalias == 1) + if (flag_argument_noalias = 1) ann-noalias_state = NO_ALIAS; else if (flag_argument_noalias == 2) ann-noalias_state = NO_ALIAS_GLOBAL; That looks wrong. Shouldn't this just replace flag_argument_noalias for noalias everywhere? Otherwise it looks good, but let's wait for Danny to comment. Richard.