Open
Bug 1001367
Opened 11 years ago
Updated 2 years ago
ClampIToUint8 could be optimized out but isn't
Categories
(Core :: JavaScript Engine: JIT, defect, P3)
Tracking
()
NEW
People
(Reporter: kael, Unassigned)
References
(Blocks 1 open bug)
Details
For code like this (using typed arrays and properly hinted values):
var p = (i * 4) | 0;
dataBytes[p] = bytes[p] & 0xFF;
The JIT inspector displays the following assembly:
[LoadTypedArrayElement]
movzbl 0(%esi,%ebx,1), %eax
[BitOpI:bitand]
andl $0xff, %eax
[ClampIToUint8]
testl $0xffffff00, %eax
je ((1226))
sarl $31, %eax
notl %eax
andl $0xff, %eax
##link ((1226)) jumps to ((1237))
[StoreTypedArrayElement]
movb %al, 0(%edi,%ebx,1)
If you take out the & 0xFF the BitOpI goes away, but the clamp is unchanged.
I think in this scenario, the ClampI operation can be entirely elided, because the user JS is already ensuring the value is within the range of a byte. This eliminates the need for the branch and for the sarl/notl.
Comment 1•8 years ago
|
||
Seems straightforward (in principle) for range analysis to elide both the bitand and the clamp.
Priority: -- → P3
Comment 2•8 years ago
|
||
Currently, RangeAnalysis only attempt to truncate to int32 operations, to be able to truncate to smaller ranges, we would have to keep the range that we are truncating to, as part of the TruncateKind recorded field.
So, under RangeAnalysis::truncate() function, we would have to modify the computeTruncateKind to account for the truncation size, to recall that we want to truncate to the Uint8 range, instead of the Int32 range. This would be required for removing the "& 0xff" when we load from a *int8 typed array.
Once we have these TruncateKind with the range of the truncation, then we can remove the MClampUint8 as part of RangeAnalysis::removeUnnecessaryBitops().
A simpler way, would be to use MClampUint8::collectRangeInfoPreTrunc() and MBitAnd::collectRangeInfoPreTrunc(), to record a flag to acknowledge that we can lower these instructions to a "redefine()" without generating any LIR node. Note, collectRangeInfoPreTrunc is the only place where we can look for the ranges of inputs to store flag for future lowering optimizations.
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•