|
From: Florian K. <br...@ac...> - 2012-07-28 02:26:43
|
On 07/27/2012 06:23 PM, Philippe Waroquiers wrote:
> Find attached a patch which drops --vex-iropt-precise-memory-exns
> and adds another clo
> --vex-iropt-register-updates=minimal|atmemaccess|exact
>
I like it. The old clo had a sufficiently obscure name that required
consulting the manual to figure out what's happening. And who wants to
do that :)
I would like it even better if we named the choices slightly
differently. How is this:
minimal -> at-endof-sb
atmemaccess -> at-mem-access
exact -> at-insn
I find that clearer.
WRT the implementation...
+ else if VG_XACT_CLO(arg, "--vex-iropt-register-updates=minimal",
+ VG_(clo_vex_control).iropt_register_updates,
+ 0);
Can we please use symbolic names instead of magic constants 0,1,2 ?
enum {
VEX_REG_UPDATE_AT_ENDOF_SB,
VEX_REG_UPDATE_AT_MEM_ACCESS,
VEX_REG_AT_INSN
};
That improves comprehension and nobody get hurt. We already have enough
magic constants. Let's not introduce new ones.
Florian
|