| Message ID | 1267739110-26400-4-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
> TCG internal helpers only access to the values passed in arguments, and > do not modify the CPU internal state. Thus they can be declared as > const and pure. I think this needs an explanatory comment. It's not immediately obvious that tcg_gen_helperN and tcg_gen_helper{32,64} have significantly different semantics. tcg/README also needs updating, specifically: "* Helpers: Using the tcg_gen_helper_x_y it is possible to call any function taking i32, i64 or pointer types. Before calling an helper, all globals are stored at their canonical location and it is assumed that the function can modify them. In the future, function modifiers will be allowed to tell that the helper does not read or write some globals. " Paul
On Fri, Mar 05, 2010 at 11:15:45AM +0000, Paul Brook wrote: > > TCG internal helpers only access to the values passed in arguments, and > > do not modify the CPU internal state. Thus they can be declared as > > const and pure. > > I think this needs an explanatory comment. It's not immediately obvious that > tcg_gen_helperN and tcg_gen_helper{32,64} have significantly different > semantics. What do you mean exactly? Mentioning explicitly tcg_gen_helper{32,64} instead of "TCG internal helpers". > tcg/README also needs updating, specifically: > > "* Helpers: > > Using the tcg_gen_helper_x_y it is possible to call any function > taking i32, i64 or pointer types. Before calling an helper, all > globals are stored at their canonical location and it is assumed that > the function can modify them. In the future, function modifiers will > be allowed to tell that the helper does not read or write some globals. > " > Fully agreed, but it is not a change introduce by this patch series. I'll submit a patch later.
> On Fri, Mar 05, 2010 at 11:15:45AM +0000, Paul Brook wrote: > > > TCG internal helpers only access to the values passed in arguments, and > > > do not modify the CPU internal state. Thus they can be declared as > > > const and pure. > > > > I think this needs an explanatory comment. It's not immediately obvious > > that tcg_gen_helperN and tcg_gen_helper{32,64} have significantly > > different semantics. > > What do you mean exactly? Mentioning explicitly tcg_gen_helper{32,64} > instead of "TCG internal helpers". I think the difference between tcg_gen_helperN and tcg_gen_helper{32,64} is sufficiently subtle that it deserves documenting. It's not obvious that the latter may only be used for cont/pure helpers. My guess is that the FIXME you're removing was added precisely because there was uncertainty whether this assumption was reasonable, and under which circumstances they are used. Paul
diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h index c71e1a8..30a7a73 100644 --- a/tcg/tcg-op.h +++ b/tcg/tcg-op.h @@ -364,7 +364,6 @@ static inline void tcg_gen_helperN(void *func, int flags, int sizemask, tcg_temp_free_ptr(fn); } -/* FIXME: Should this be pure? */ static inline void tcg_gen_helper32(void *func, TCGv_i32 ret, TCGv_i32 a, TCGv_i32 b) { @@ -373,11 +372,11 @@ static inline void tcg_gen_helper32(void *func, TCGv_i32 ret, fn = tcg_const_ptr((tcg_target_long)func); args[0] = GET_TCGV_I32(a); args[1] = GET_TCGV_I32(b); - tcg_gen_callN(&tcg_ctx, fn, 0, 0, GET_TCGV_I32(ret), 2, args); + tcg_gen_callN(&tcg_ctx, fn, TCG_CALL_CONST | TCG_CALL_PURE, + 0, GET_TCGV_I32(ret), 2, args); tcg_temp_free_ptr(fn); } -/* FIXME: Should this be pure? */ static inline void tcg_gen_helper64(void *func, TCGv_i64 ret, TCGv_i64 a, TCGv_i64 b) { @@ -386,7 +385,8 @@ static inline void tcg_gen_helper64(void *func, TCGv_i64 ret, fn = tcg_const_ptr((tcg_target_long)func); args[0] = GET_TCGV_I64(a); args[1] = GET_TCGV_I64(b); - tcg_gen_callN(&tcg_ctx, fn, 0, 7, GET_TCGV_I64(ret), 2, args); + tcg_gen_callN(&tcg_ctx, fn, TCG_CALL_CONST | TCG_CALL_PURE, + 7, GET_TCGV_I64(ret), 2, args); tcg_temp_free_ptr(fn); }
TCG internal helpers only access to the values passed in arguments, and do not modify the CPU internal state. Thus they can be declared as const and pure. Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- tcg/tcg-op.h | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)