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)

30 Branch
x86_64
Windows 7
defect

Tracking

()

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.
Seems straightforward (in principle) for range analysis to elide both the bitand and the clamp.
Priority: -- → P3
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.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.