Support conversion of clamped double values for pixel arrays in Crankshaft.

src/x64/macro-assembler-x64.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2552 matching lines @@
                               Label* fail,
                               bool is_heap_object) {
   if (!is_heap_object) {
     JumpIfSmi(obj, fail);
   }
   Cmp(FieldOperand(obj, HeapObject::kMapOffset), map);
   j(not_equal, fail);
 }
 
 
+void MacroAssembler::ClampUint8(Register reg) {
+  Label done;
+  testl(reg, Immediate(0xFFFFFF00));
+  j(zero, &done, Label::kNear);
+  setcc(negative, reg);  // 1 if negative, 0 if positive.
+  decb(reg);  // 0 if negative, 255 if positive.
+  bind(&done);
+}
+
+
+void MacroAssembler::ClampDoubleToUint8(XMMRegister input_reg,
+                                        XMMRegister temp_xmm_reg,
+                                        Register result_reg,
+                                        Register temp_reg) {
+  Label above_zero;
+  Label done;
+  Label in_bounds;
+  xorps(temp_xmm_reg, temp_xmm_reg);
+  ucomisd(input_reg, temp_xmm_reg);
+  j(above, &above_zero, Label::kNear);

[Lasse Reichstein, 2011/05/14 09:34:38]

I believe NaN should also be converted to zero (i.e., if the P flag is set after
ucomisd)

[danno, 2011/05/15 05:50:27]

NaN does get set to zero, there is an explicit test for it in the
external_arrays.js. Above jumps when CF=0 and ZF=0, which is not the case for
comparison for NaN (for unordered comparisons ZF,PF,CF are all set to 111), so
the code falls through and return zero path is taken.
On 2011/05/14 09:34:38, Lasse Reichstein wrote:

[Lasse Reichstein, 2011/05/15 10:18:50]

Ack, yes. I didn't consider that "above" would also match NaN (which is great
:).

+  Set(result_reg, 0);

[Lasse Reichstein, 2011/05/13 13:17:24]

How about setting result to 0 prior to the compare, and just jump directly to
done if below_equal. Should give less jumping.

[danno, 2011/05/15 05:50:27]

Done.

+  jmp(&done, Label::kNear);
+  bind(&above_zero);
+  uint64_t max_clamp = BitCast<uint64_t, double>(255.0);

[Lasse Reichstein, 2011/05/13 13:17:24]

How about not capping at 255 until after you have converted to integer?

[danno, 2011/05/15 05:50:27]

Done.

+  Set(temp_reg, max_clamp);
+  movq(temp_xmm_reg, temp_reg);

[Lasse Reichstein, 2011/05/13 13:17:24]

Use movsd here. Movq makes the CPU think the XMM register holds an integer, not
a double. There is a latency for switching between interpretations.

[danno, 2011/05/15 05:50:27]

Done.

+  ucomisd(input_reg, temp_xmm_reg);
+  j(below_equal, &in_bounds, Label::kNear);
+  Set(result_reg, 255);
+  jmp(&done, Label::kNear);
+  bind(&in_bounds);
+  if (CpuFeatures::IsSupported(SSE4_1)) {
+    CpuFeatures::Scope scope(SSE4_1);
+    roundsd(temp_xmm_reg, input_reg, kRoundToNearest);

[Lasse Reichstein, 2011/05/13 13:17:24]

Roundsd's round-to-nearest rounds to even numbers in case of a tie. The code in
the other branch rounds up. Which one is correct?

[Lasse Reichstein, 2011/05/14 09:34:38]

I tried to look up a spec for typed arrays, and if the one I found was correct,
we should just round towards zero, i.e., truncate.

[danno, 2011/05/15 05:50:27]

Done.

[danno, 2011/05/15 05:50:27]

This code is specifically for CanvasPixelArrays (which use the special clamping
semantics that include rounding, look at http://www.w3.org/TR/2dcontext/ and
search for "clamp": "use the value from the nearest edge pixel,"), unlike typed
arrays (which use ToInt32 semantics, which we currently don't implement
correctly, but that's another issue and another CL for another day). 
On 2011/05/14 09:34:38, Lasse Reichstein wrote:

[Lasse Reichstein, 2011/05/15 10:18:50]

I read that as clamping positions to the edge of the available image when a
filtering algorithm needs surrounding pixels that aren't in the image.
I don't see anything about rounding double values to octets for the pixel array
(but I can see that Firefox is using round-to-even on my machine, so they are
probably using roundsd).

+  } else {
+    uint64_t one_half = BitCast<uint64_t, double>(0.5);
+    Set(temp_reg, one_half);
+    movq(temp_xmm_reg, temp_reg);

[Lasse Reichstein, 2011/05/13 13:17:24]

Movsd.

[danno, 2011/05/15 05:50:27]

Done.

+    addsd(temp_xmm_reg, input_reg);

[Lasse Reichstein, 2011/05/13 13:17:24]

Adding 0.5 and truncating doesn't work perfectly as round-to-nearet.
It doesn't round correctly for 0.49999999999999994. Adding 0.5 incorrectly
rounds this up to 1, but round to nearest must round down.

You can instead first compare to 0.5, which you already have, and if below, just
return 0.

(There's also a roundoff problem at 2^53-1, but that doesn't matter here, since
both the actual and the correct result will be capped to 255 anyway).

[danno, 2011/05/15 05:50:27]

I am aware of this imprecision, but this is exactly the roudning algorithm used
in the "slow" case implementation for pixel arrays in objects.cc SetProperty,
and the semantic should be the same. I'll file a separate bug to fix the overall
behavior, but that's beyond the scope of this CL.
On 2011/05/13 13:17:24, Lasse Reichstein wrote:

[Lasse Reichstein, 2011/05/15 10:18:51]

Good :)

+  }
+  cvttsd2si(result_reg, temp_xmm_reg);
+  bind(&done);
+}
+
+
 void MacroAssembler::AbortIfNotNumber(Register object) {
   Label ok;
   Condition is_smi = CheckSmi(object);
   j(is_smi, &ok, Label::kNear);
   Cmp(FieldOperand(object, HeapObject::kMapOffset),
       isolate()->factory()->heap_number_map());
   Assert(equal, "Operand not a number");
   bind(&ok);
 }
 
@@ skipping 1070 matching lines @@
   CPU::FlushICache(address_, size_);
 
   // Check that the code was patched as expected.
   ASSERT(masm_.pc_ == address_ + size_);
   ASSERT(masm_.reloc_info_writer.pos() == address_ + size_ + Assembler::kGap);
 }
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_X64