|
From: Florian K. <br...@ac...> - 2012-07-16 22:21:49
|
Hi,
attached are three patches.
guarded-dirty-patch-1 combines the patches I had sent previously and
addresses these comments:
>> Looks plausible. Couple of things:
>>
>> (trivial)
>> -
>> /*------------------------------------------------------------*/
>>
>> pls can you leave the blank line there? I like 2 blank lines
>> before section breaks (more artistically pleasing :)
>
>> + /* If the complaint is to be issued under a guard condition, AND that
>> + guard condition. */
>> + if (guard && ! isTRUE(di->guard)) {
>>
>> That said, I would much prefer that you removed the " && ! isTRUE(di->guard)",
>> hence always constructing the And against di->guard if it is non-NULL, and
>> rely in ir_opt to tidy up afterwards if di->guard is a constant.
>
>> + We assume here, that the validity of GUARD has already been checked.
>> s/validity/definedness
>
vex-patch is an updated version of the changes to the amd64 insn
selector as suggested in this comment:
>>
>> + addInstr(env, AMD64Instr_Test64(0xFFFFFFFF,reg));
>>
>> Shouldn't that be 0x1 ? IIUC, as you have it, rflags.NZ will get set to 1
>> if any of the bits in reg are nonzero.
>
Finally, there is guarded-dirty-patch-2 which fixes up do_origins_Dirty
to observe guards.
> I have an updated patch which I will post when s390 passes. There is
> currently one testcase that is failing and I need to find out why. Could
> be some latent bug that is exposed by these changes.
This bug was completely unrelated to my patch set (and fixed in r12749).
It just surfaced in this context...
I've reg-tested the patches on x86-64, ppc64, and s390x with no new
regressions.
Florian
|