Thank you for looking at the change!
Good point about the unnecessary Set method, removed it and using Immediate
wrapper instead.
As for the flags... I tracked down all pairs of <test> ... <conditional
branch> that V8 generates and concluded that nowhere did the new Set insert
itself between the two instructions, nor its earlier occurrence can affect
the conditional branch across multiple conditional branches (in case of
conditional branch that checks one of flags not set by the immediately
preceding test instruction). The details on my process are below.
Thank you!
Martin
Tracking callers of the new Set method didn't lead to certainty because the
Set method is called while walking arbitrarily complex data structures,
especially the Lithium ones, making it hard to isolate what code was emitted
before, or right after the call to Set.
So instead I tracked all places where V8 emits conditional branches and
checked that the call to Set doesn't happen between the test instruction and
the conditional branch.
The functions that start by emitting a conditional branch are the tricky
ones because they have to be preceded by call to something that sets flags
accordingly. I found these methods as beginning with emit of a conditional
jump:
* Assembler::j
* DeferredCode::Branch
* DeferredInlineBinaryOperation::JumpToAnswerOutOfRange
* DeferredInlineBinaryOperation::JumpToConstantRhs
* JumpPatchSite::EmitJump
* FullCodeGenerator::Split
* JumpTarget::DoBranch
* JumpTarget::Branch (incl. BreakTarget::Branch)
* LCodeGen::DeoptimizeIf
* LCodeGen::DoDeoptimize
* LCodeGen::EmitBranch
* MacroAssembler::IncrementCounter(Condition ...)
* MacroAssembler::DecrementCounter(Condition ...)
* MacroAssembler::Check
* MacroAssembler::Assert
* RegExpMacroAssemblerIA32::BranchOrBacktrack
* ControlDestination::Split
Then I tracked down all their callers to make sure that before the call to
any of the above, test instruction is emitted and that there is no possible
intervening call that can lead to Set.
In totality, calls to the above are preceded by emitting one of the
following instructions (or short sequences of instructions):
add
and
cmp
cmp; mov // two instructions emitted, followed by the conditional jmp
cmp; j<c>; ffree(0); fincstp (the first cmp sets flags, they stay untouched)
cmpb
cmpb_al
cmpb_al
cmpw_ax
dec
dec_b
fucomi
imul
neg
or
sub
sub; mov; mov; // followed by conditional jmp
sar
shr
test
test; mulsd
test; mov
test; popad
test_b
ucomisd
xor
or by a call to one of the following:
MacroAssembler::CmpInstanceType()
- emits cmpb
MacroAssembler::CmpObjectType()
- calls CmpInstanceType(), therefore emits cmpb
LCodeGen::EmitClassOfTest()
- ends with emitting "cmp"
LCodeGen::EmitCmpI()
- emits cmp
LCodeGen::EmitIsObject()
- emits cmp as last instruction
LCodeGen::EmitTypeofIs()
- ends by emitting test, cmp or
an unconditional jmp (so the following conditional jmp never executes)
MacroAssembler::FCmp()
- generates either fucomip or sahf, sets ZF, CF, PF which are the only
flags
tested thereafter
MacroAssembler::IsObjectStringType()
- emits "test" last
MacroAssembler::SmiTag()
- emits "add"
SmiUntag()
- "sar"
FullCodeGenerator::PrepareForBailoutBeforeSplit()
- Only interesting in few contexts when used between test and
FullCodeGenerator::Split. In those cases it is either noop or generates
unconditional branch which skips to the code being emitted by Split that
follows, ie no flags are affected here, moreover, call to Set from this
context is never possible.
All of the functions emiting conditional branch are preceded by emitting one
one of the test instructions. Most of them affect the same flags as xor
(OF SF ZF AF PF CF)
* add, and, cmp, imul, neg, or, sub, sar, shr, test, xor, ucomisd
...so even if there was a new xor instruction emitted previously, all flags
set by it would be destroyed and thust the xor would have no effect on any
conditional branch following one of these instructions.
Two instructions affect only some of the flags so if there is new XOR
followed by one of these 2 instructions and later by j<c> that checks one of
the flags set by XOR but not affected by the instruction in question
(basically a bug) the code execution would be affected.
* dec (only affects OF SF ZF AF PF so CF is left unchanged to what it was
previously)
- used in 17 places, and never followed by conditional jump using CF
(either
correct jump or another instruction that clobbers all relevant flags)
* fucomi (only affects ZF, PF and CF) so OF, SF and AF are unchanged.
- only used once and followed by correct j(parity_even)
Some tricky places were
* StringCompareStub::GenerateCompareFlatAsciiStrings
- j<c> is emitted after a label ... so I tracked branches to that label
and all of those were preceded by "test" instruction.
* RegExpMacroAssemblerIA32::BranchOrBacktrack
- whenver called with condition code, it is preceded by test. There are
few calls with "no_condition" which do not generate conditional branch and
thus are not of interest.
(*1) Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume
1: Basic Architecture Appendix A - EFLAGS Cross Reference
On Mon, Dec 20, 2010 at 11:22 PM, <[email protected]> wrote:
> It is very important to make sure that Set() is not used in any place where
> we
> want to preserve the flags - make sure that no new uses of Set() can occur
> between the test and the branch in generated code.
>
> I don't see the need for introducing a new Set(Register, int32_t).
> The one place where it is used, you can simply wrap the int32 in an
> Immediate().
>
>
>
> On 2010/12/21 01:56:39, Martin Maly wrote:
>
>> Emit literal zero as xor is used in few places but while browsing through
>> the
>> code I found that it is not so everywhere.
>>
>
> I ran the benchmarks (crypto, deltablue and friends) with and without my
>>
> change
>
>> and detected no performance regression.
>>
>
> Thank you!
>> Martin
>>
>
>
>
> http://codereview.chromium.org/6016007/
>
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
