|
From: <sv...@va...> - 2012-12-19 15:28:52
|
sewardj 2012-12-19 15:28:43 +0000 (Wed, 19 Dec 2012)
New Revision: 2598
Log:
Constant folder: enable Sub32(x,0) ==> x.
Modified files:
trunk/priv/ir_opt.c
Modified: trunk/priv/ir_opt.c (+5 -5)
===================================================================
--- trunk/priv/ir_opt.c 2012-12-19 08:39:11 +00:00 (rev 2597)
+++ trunk/priv/ir_opt.c 2012-12-19 15:28:43 +00:00 (rev 2598)
@@ -2063,13 +2063,14 @@
}
break;
+ case Iop_Sub32:
case Iop_Sub64:
- /* Sub64(x,0) ==> x */
- if (isZeroU64(e->Iex.Binop.arg2)) {
+ /* Sub32/Sub64(x,0) ==> x */
+ if (isZeroU(e->Iex.Binop.arg2)) {
e2 = e->Iex.Binop.arg1;
break;
}
- /* Sub64(t,t) ==> 0, for some IRTemp t */
+ /* Sub32/Sub64(t,t) ==> 0, for some IRTemp t */
if (sameIRExprs(env, e->Iex.Binop.arg1, e->Iex.Binop.arg2)) {
e2 = mkZeroOfPrimopResultType(e->Iex.Binop.op);
break;
@@ -2133,9 +2134,8 @@
}
break;
- case Iop_Sub32:
case Iop_CmpNE32:
- /* Sub32/CmpNE32(t,t) ==> 0, for some IRTemp t */
+ /* CmpNE32(t,t) ==> 0, for some IRTemp t */
if (sameIRExprs(env, e->Iex.Binop.arg1, e->Iex.Binop.arg2)) {
e2 = mkZeroOfPrimopResultType(e->Iex.Binop.op);
break;
|
|
From: John R. <jr...@bi...> - 2012-12-19 16:25:44
|
> New Revision: 2598 > > Log: > Constant folder: enable Sub32(x,0) ==> x. Beware. Processor status (flag bits) might change, and in "unexpected" ways. PowerPC implements SUB(x, y) as ADDwithCarry(x, ~y, 1), and does not invert the Carry bit when the original opcode was SUB. So, subtracting zero always sets Carry (if status is being recorded.) On x86 SUB records a Borrow in the C status bit, so subtracting zero always clears the C bit. -- |
|
From: Julian S. <js...@ac...> - 2012-12-21 09:24:37
|
On Wednesday, December 19, 2012, John Reiser wrote: > > Constant folder: enable Sub32(x,0) ==> x. > > Beware. Processor status (flag bits) might change, and in "unexpected" > ways. The constant folder operates on the IR, which makes all architected state changes explicit -- there are no "under the table" semantics. If a ppc sub insn sets flag bits then that will have been translated into its own fragment of IR independent of the actual SUB. Hence IR expressions are denote pure values and we can transform them as such. J |
|
From: Florian K. <br...@ac...> - 2012-12-21 15:22:58
|
On 12/19/2012 11:26 AM, John Reiser wrote:
>> New Revision: 2598
>>
>> Log:
>> Constant folder: enable Sub32(x,0) ==> x.
>
> Beware. Processor status (flag bits) might change, and in "unexpected" ways.
>
> PowerPC implements SUB(x, y) as ADDwithCarry(x, ~y, 1),
Interesting. s390 does this, too.
> and does not invert the Carry bit when the original opcode was SUB.
> So, subtracting zero always sets Carry (if status is being recorded.)
>
> On x86 SUB records a Borrow in the C status bit, so subtracting zero
> always clears the C bit.
>
True, but I think this optimisation does not break break anything.
For these reasons:
(1) Performing the optimisation is completely safe when the flags bits
for this subtraction are not examined, as in (x - 0) + y.
For arithmetic operations that are generated during IR generation
(e.g. for address computation) the flag bits are not examined.
(2) From #1 it follows that this optimisation could only make a
difference if
(a) it was written in the source code
(b) it was preserved through the compiler optimisation
(c) the carry bit is tested on the result
While not impossible that looks like a scenario that is quite
unlikely to occur. Probably requires some handwritten assembly.
(3) Even if IR optimisation replaces SUB(x,0) -> x that does not mean
that the flags are computed improperly. Condition code computation
in VEX is performed by putting the operands of an operation into the
flags thunk (guest state) together with a tag that describes the
operation that is to be performed on them. Here, something like:
[SUB32, x, 0]
If it turns out that flags bits are needed, the thunk will be
evaluated. So, if there is a problem here at all, it would imply
that the code that evaluates the flags thunk is incorrect.
If you're interested take a look at function s390_calculate_cc
in guest_s390_helpers.c that does exactly this.
No reason for concern.
Florian
|