[v8-dev] Fix the V8_GNUC_PREREQ macro. (issue 1003383004 by alexandre.ra...@arm.com)
Reviewers: danno, Description: Fix the V8_GNUC_PREREQ macro. BUG= Please review this at https://codereview.chromium.org/1003383004/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+3, -3 lines): M include/v8config.h Index: include/v8config.h diff --git a/include/v8config.h b/include/v8config.h index 46d2c3ce71ff43f827461e128f4e9001e3c704dd..846cd64286b0e487346b0c1647f6585c57dbb1f1 100644 --- a/include/v8config.h +++ b/include/v8config.h @@ -42,9 +42,9 @@ ((__GNUC__ * 1 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) = \ ((major) * 1 + (minor) * 100 + (patchlevel))) #elif defined(__GNUC__) defined(__GNUC_MINOR__) -# define V8_GNUC_PREREQ(major, minor, patchlevel) \ -((__GNUC__ * 1 + __GNUC_MINOR__) = \ - ((major) * 1 + (minor) * 100 + (patchlevel))) +#define V8_GNUC_PREREQ(major, minor, patchlevel) \ +((__GNUC__ * 1 + __GNUC_MINOR__ * 100) = \ + ((major)*1 + (minor)*100 + (patchlevel))) #else # define V8_GNUC_PREREQ(major, minor, patchlevel) 0 #endif -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Fix the V8_GNUC_PREREQ macro. (issue 1003383004 by alexandre.ra...@arm.com)
https://codereview.chromium.org/1003383004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Revert ARM: Use the shifter operand to merge in previous shift instructions. (issue 359713004 by ma...@chromium.org)
On 2014/06/27 09:13:48, Michael Achenbach wrote: FYI I saw that. Looking at it. https://codereview.chromium.org/359713004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM: Use the shifter operand to merge in previous shift instructions. (issue 312173002 by alexandre.ra...@arm.com)
Committed patchset #2 manually as r22017 (presubmit successful). https://codereview.chromium.org/312173002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM/ARM64: Optimise HLoadNamedField and HStoreNamedField. (issue 324093002 by alexandre.ra...@arm.com)
We tried to go down this route before, but by simplifying all object accesses into a either two loads or a load and a store. That had the disadvantage of reducing the accuracy of GVN and load/store elimination. This approach doesn't that have the disadvantage of reducing the accuracy of those phases, but it further entangles the whole graph with these extra uses, that are only used on two backends. That seems like added complexity that I'm not sure is worth it. Maybe the patch could be simplified to have the second load take the newly introduced first load as its object input rather than as a separate input. If that works the modifications to the graphs could be acceptable; what do you think? We have another similar patch extracting the 'load map' outside of 'compare map' instructions. Like this patch it doesn't impact the benchmarks, but cleans the generated code. The arch-dependent code in hydrogen.h and hydrogen-instructions.h could also be moved to separate arm64 headers, but I don't think it is preferable. https://codereview.chromium.org/324093002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid regeneration of constants. (issue 308313002)
Any progress on this topic? https://codereview.chromium.org/308313002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM: Use the shifter operand to merge in previous shift instructions. (issue 312173002)
https://codereview.chromium.org/312173002/diff/1/src/arm/lithium-arm.cc File src/arm/lithium-arm.cc (right): https://codereview.chromium.org/312173002/diff/1/src/arm/lithium-arm.cc#newcode809 src/arm/lithium-arm.cc:809: constant_value = constant-Integer32Value() 0x1f; On 2014/06/10 11:53:37, ulan wrote: Use JSShiftAmountFromHConstant? Done. https://codereview.chromium.org/312173002/diff/1/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): https://codereview.chromium.org/312173002/diff/1/src/arm/lithium-codegen-arm.cc#newcode1731 src/arm/lithium-codegen-arm.cc:1731: right = ToOperand(right_op); On 2014/06/10 11:53:37, ulan wrote: Doesn't ToShiftedRightOperand take care of this case? It does. Removed the constant case. https://codereview.chromium.org/312173002/diff/1/src/arm/lithium-codegen-arm.cc#newcode1793 src/arm/lithium-codegen-arm.cc:1793: uint8_t shift_count = static_castuint8_t(value 0x1F); On 2014/06/10 11:53:37, ulan wrote: The original CL used JSShiftAmountFromLConstant here and in other places. Done. https://codereview.chromium.org/312173002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM: Use the shifter operand to merge in previous shift instructions. (issue 312173002)
Uploaded the patch with your comments addressed. Could you confirm it is still ok to go in? Thanks https://codereview.chromium.org/312173002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM/ARM64: Optimise HLoadNamedField and HStoreNamedField. (issue 324093002)
Reviewers: ulan, jochen traveling until Jun 23, Message: This does not show a noticeable impact on benchmarks, but the code generated is cleaner. Description: ARM/ARM64: Optimise HLoadNamedField and HStoreNamedField. The access to the object properties is abstracted in via a separate HLoadNamedField to be shared across accesses to the same object. Please review this at https://codereview.chromium.org/324093002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+174, -29 lines): M src/arm/lithium-arm.h M src/arm/lithium-arm.cc M src/arm/lithium-codegen-arm.cc M src/arm64/lithium-arm64.h M src/arm64/lithium-arm64.cc M src/arm64/lithium-codegen-arm64.cc M src/hydrogen.h M src/hydrogen-instructions.h -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM: Use the shifter operand to merge in previous shift instructions. (issue 312173002)
Reviewers: ulan, jochen, Message: This is a port of the arm64 similar commit. Description: ARM: Use the shifter operand to merge in previous shift instructions. When possible, we transform sequences of code of the form lsl r1, r2, #imm add r0, r5, r1 into add r0, r5, r2 LSL #imm This is an adaptation of r21161. Please review this at https://codereview.chromium.org/312173002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+269, -15 lines): M src/arm/assembler-arm.h M src/arm/assembler-arm.cc M src/arm/constants-arm.h M src/arm/lithium-arm.h M src/arm/lithium-arm.cc M src/arm/lithium-codegen-arm.h M src/arm/lithium-codegen-arm.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid regeneration of constants. (issue 308313002)
On 2014/06/03 08:01:16, danno wrote: Actually, the right thing to do is to treat constants as normal values and teach the register allocator how to split constant ranges and rematerialize on demand without spilling. This is something that Jaro and Benedikt have thought about recently, and I think this is generally a cleaner way to approach the problem than the ad-hoc EmitAsUses approach which is just a brittle heuristic. I did a bit of work on this as a follow-up for this patch: I augmented lithium instructions with a method indicating if it was better to regenerate the value (re-visit the instruction) instead of spilling and restoring it. LiveRanges and LOperands kept track of which lithium instruction output they referred to. The gap resolver skips spilling moves when appropriate, and instead of moving from the stack re-visits the lithium instruction. I think there is some more work to do around that, but if you are interested I could upload this so you can have a look. https://codereview.chromium.org/308313002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Regenerate instead of spilling and restoring cheap lithium instructions. (issue 309373002)
Reviewers: , Message: Following the discussion at https://codereview.chromium.org/308313002/ , here is a patch (applicable on top of the patch for the previously mentioned issue) with some work on avoiding spilling/restoring. To make things clear again, this is *not* a review request. Short description: LOperands and LiveRanges are augmented with a pointer to the Lithium instruction that generated them. Gap moves are computed as usual, but code generation skips spilling and unspilling moves (reg from/to stack) for values generated by a 'cheap' Lithium instruction, and instead regenerates (re-visits) the value. Description: ARM64: Regenerate instead of spilling and restoring cheap lithium instructions. FOR INFORMATION PURPOSES ONLY. NOT TO BE COMMITTED! Please review this at https://codereview.chromium.org/309373002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+168, -48 lines): M src/arm64/lithium-arm64.h M src/arm64/lithium-arm64.cc M src/arm64/lithium-gap-resolver-arm64.cc M src/hydrogen-instructions.h M src/hydrogen-instructions.cc M src/lithium.h M src/lithium.cc M src/lithium-allocator.h M src/lithium-allocator.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Avoid regeneration of constants. (issue 308313002)
Reviewers: jochen, ulan,
Message:
This shows some improvement (up to ~5%) for some of the Octane benchmarks
(up to
~5% for Raytrace and Richards). The impact depends on the CPU benchmarked.
Ideally the same result should be achieved by modifying
HConstant::EmitAtUses(),
but an attempt to do so triggered crashes.
For example trying:
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -2870,6 +2870,7 @@ bool HConstant::EmitAtUses() {
if (IsCell()) return false;
if (representation().IsDouble()) return false;
if (representation().IsExternal()) return false;
+ if (representation().IsTagged()) return false;
return true;
}
yields
# Fatal error in ../src/arm64/lithium-codegen-arm64.cc, line 1856
# CHECK(!info()-IsStub()) failed
Any pointer or ideas on what may be going wrong?
Description:
ARM64: Avoid regeneration of constants.
At the lithium level, HConstants are cached and their virtual register
reused to
avoid regenerating them.
Please review this at https://codereview.chromium.org/308313002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+67, -1 lines):
M src/arm64/lithium-arm64.h
M src/arm64/lithium-arm64.cc
M src/hydrogen-osr.h
Index: src/arm64/lithium-arm64.cc
diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc
index
02127066536aa8dead082292d77f04a50b4f21de..6f60e8ae0d7b7f436a7800155d59326fd9b81b32
100644
--- a/src/arm64/lithium-arm64.cc
+++ b/src/arm64/lithium-arm64.cc
@@ -375,7 +375,12 @@ LUnallocated*
LChunkBuilder::ToUnallocated(DoubleRegister reg) {
LOperand* LChunkBuilder::Use(HValue* value, LUnallocated* operand) {
if (value-EmitAtUses()) {
HInstruction* instr = HInstruction::cast(value);
-VisitInstruction(instr);
+if (value-IsConstant() IsCachedConstant(HConstant::cast(value),
+current_block_)) {
+ // We will reuse the cached value.
+} else {
+ VisitInstruction(instr);
+}
}
operand-set_virtual_register(value-id());
return operand;
@@ -586,7 +591,13 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block) {
ASSERT(is_building());
current_block_ = block;
+ if (graph()-has_osr()
+ (block == graph()-osr()-osr_entry())) {
+constant_cache_.Clear();
+ }
+
if (block-IsStartBlock()) {
+constant_cache_.Clear();
block-UpdateEnvironment(graph_-start_environment());
argument_count_ = 0;
} else if (block-predecessors()-length() == 1) {
@@ -1328,6 +1339,7 @@ LInstruction*
LChunkBuilder::DoCompareMap(HCompareMap* instr) {
LInstruction* LChunkBuilder::DoConstant(HConstant* instr) {
+ CacheConstant(instr, current_block_);
Representation r = instr-representation();
if (r.IsSmi()) {
return DefineAsRegister(new(zone()) LConstantS);
Index: src/arm64/lithium-arm64.h
diff --git a/src/arm64/lithium-arm64.h b/src/arm64/lithium-arm64.h
index
be053ddbbda026ab18a97d07433d04b43f8fbcc3..fc6e667c6061295788eea765721d57f7616cff96
100644
--- a/src/arm64/lithium-arm64.h
+++ b/src/arm64/lithium-arm64.h
@@ -3012,6 +3012,50 @@ class LWrapReceiver V8_FINAL : public
LTemplateInstruction1, 2, 0 {
};
+class ConstantCache {
+ public:
+ ConstantCache() {}
+
+ class ConstantCacheLevel {
+ public:
+void Cache(HConstant* constant) { cached_constants_.insert(constant); }
+bool IsCached(HConstant* constant) {
+ return cached_constants_.find(constant) != cached_constants_.end();
+}
+
+ private:
+std::setHConstant* cached_constants_;
+ };
+
+ void Clear() { cache_levels_.clear(); }
+
+ void Cache(HConstant* constant, HBasicBlock* block) {
+cache_levels_[block].Cache(constant);
+ }
+
+ bool IsCached(HConstant* constant, HBasicBlock* block) {
+HBasicBlock* base_block = block;
+std::mapHBasicBlock*, ConstantCacheLevel::iterator it;
+while (block != NULL) {
+ it = cache_levels_.find(block);
+ if (it != cache_levels_.end() it-second.IsCached(constant)) {
+if (base_block != block) {
+ // Avoid traversing the dominator blocks multiple times for the
same
+ // constant.
+ cache_levels_[block].Cache(constant);
+}
+return true;
+ }
+ block = block-dominator();
+}
+return false;
+ }
+
+ private:
+ std::mapHBasicBlock*, ConstantCacheLevel cache_levels_;
+};
+
+
class LChunkBuilder;
class LPlatformChunk V8_FINAL : public LChunk {
public:
@@ -3055,6 +3099,13 @@ class LChunkBuilder V8_FINAL : public
LChunkBuilderBase {
static bool HasMagicNumberForDivision(int32_t divisor);
+ bool IsCachedConstant(HConstant* constant, HBasicBlock* block) {
+return constant_cache_.IsCached(constant, block);
+ }
+ void CacheConstant(HConstant* constant, HBasicBlock* block) {
+constant_cache_.Cache(constant, block);
+ }
+
private:
enum Status {
UNUSED,
@@ -3205,6 +3256,7 @@ class LChunkBuilder V8_FINAL : public
LChunkBuilderBase {
[v8-dev] Re: ARM64: Avoid regeneration of constants. (issue 308313002)
On 2014/06/02 09:28:25, danno wrote: DBC: Changes EmitAtUses to allocate Tagged constants in registers is generally a very bad thing for i32, so you will need more platform-specific machinery to abstract the decision. Of course! This is just to highlight the bug met. Could you provide more context for the ASSERT, perhaps the first few frames of the stack crawl? Below is a backtrace. Is this something 'expected' (we know we need to emit at uses for tagged constants and some others)? Or would we expect to be able to modify EmitAtUses() and everything to just work (let's be optimistic!)? $ make arm64.debug -j45 snapshot=off [...] $ gdb --args ./out/arm64.debug/d8 (gdb) run # # Fatal error in ../src/arm64/lithium-codegen-arm64.cc, line 1856 # CHECK(!info()-IsStub()) failed # [...] Program received signal SIGILL, Illegal instruction. v8::internal::OS::Abort () at ../src/platform-posix.cc:253 253 V8_IMMEDIATE_CRASH(); (gdb) bt #0 v8::internal::OS::Abort () at ../src/platform-posix.cc:253 #1 0x008c3b28 in V8_Fatal (file=0x1088560 ../src/arm64/lithium-codegen-arm64.cc, line=1856, format=0x10869a8 CHECK(%s) failed) at ../src/checks.cc:88 #2 0x00d0f6f8 in v8::internal::LCodeGen::DoBranch (this=0x7fff2a30, instr=0x1ffcd28) at ../src/arm64/lithium-codegen-arm64.cc:1856 #3 0x00cea091 in v8::internal::LBranch::CompileToNative (this=0x1ffcd28, generator=0x7fff2a30) at ../src/arm64/lithium-arm64.cc:20 #4 0x00da3e90 in v8::internal::LCodeGenBase::GenerateBody (this=0x7fff2a30) at ../src/lithium-codegen.cc:95 #5 0x00d09377 in v8::internal::LCodeGen::GenerateCode (this=0x7fff2a30) at ../src/arm64/lithium-codegen-arm64.cc:601 #6 0x00ab5fe4 in v8::internal::LChunk::Codegen (this=0x1ffc788) at ../src/lithium.cc:441 #7 0x008d9808 in v8::internal::DoGenerateCodev8::internal::CompareNilICStub (stub=0x7fffd0e0) at ../src/code-stubs-hydrogen.cc:274 #8 0x008d2b9e in v8::internal::CompareNilICStub::GenerateCode (this=0x7fffd0e0) at ../src/code-stubs-hydrogen.cc:849 #9 0x008c5ec2 in v8::internal::CodeStub::GetCode (this=0x7fffd0e0) at ../src/code-stubs.cc:122 #10 0x008c5af4 in v8::internal::CodeStub::GetCodeCopy (this=0x7fffd0e0, pattern=...) at ../src/code-stubs.cc:69 #11 0x00c5398d in v8::internal::StubCache::ComputeCompareNil (this=0x1ebef40, receiver_map=..., stub=...) at ../src/stub-cache.cc:315 #12 0x00a7205f in v8::internal::CompareNilIC::CompareNil (this=0x7fffd180, object=...) at ../src/ic.cc:2963 #13 0x00a721da in v8::internal::__RT_impl_CompareNilIC_Miss (args=..., isolate=0x1ea5130) at ../src/ic.cc:2976 #14 0x00a7214c in v8::internal::CompareNilIC_Miss (args_length=1, args_object=0x76dcd010, isolate=0x1ea5130) at ../src/ic.cc:2972 #15 0x00d53621 in v8::internal::Simulator::DoRuntimeCall (this=0x1ed02d0, instr=0x1eecc38) at ../src/arm64/simulator-arm64.cc:589 #16 0x00d5db4b in v8::internal::Simulator::VisitException (this=0x1ed02d0, instr=0x1eecc38) at ../src/arm64/simulator-arm64.cc:3584 #17 0x00d64d26 in v8::internal::Decoderv8::internal::Simulator::DecodeBranchSystemException (this=0x1ed02d0, instr=0x1eecc38) at ../src/arm64/decoder-arm64-inl.h:156 #18 0x00d63f3b in v8::internal::Decoderv8::internal::Simulator::Decode (this=0x1ed02d0, instr=0x1eecc38) at ../src/arm64/decoder-arm64-inl.h:62 #19 0x00d51204 in v8::internal::Simulator::ExecuteInstruction (this=0x1ed02d0) at ../src/arm64/simulator-arm64.h:320 #20 0x00d53081 in v8::internal::Simulator::Run (this=0x1ed02d0) at ../src/arm64/simulator-arm64.cc:436 #21 0x00d52584 in v8::internal::Simulator::CheckPCSComplianceAndRun (this=0x1ed02d0) at ../src/arm64/simulator-arm64.cc:248 #22 0x00d52101 in v8::internal::Simulator::CallVoid (this=0x1ed02d0, entry=0x5461eec0 ?, args=0x7fffd670) at ../src/arm64/simulator-arm64.cc:162 #23 0x00d5214d in v8::internal::Simulator::CallInt64 (this=0x1ed02d0, entry=0x5461eec0 ?, args=0x7fffd670) at ../src/arm64/simulator-arm64.cc:169 #24 0x00d52275 in v8::internal::Simulator::CallJS (this=0x1ed02d0, entry=0x5461eec0 ?, function_entry=0x54666c60 \234\203, func=0x32f51559, revc=0x32f10c21, argc=0, argv=0x0) at ../src/arm64/simulator-arm64.cc:194 #25 0x0094ff67 in v8::internal::Invoke (is_construct=false, function=..., receiver=..., argc=0, args=0x0) at ../src/execution.cc:94 #26 0x00950319 in v8::internal::Execution::Call (isolate=0x1ea5130, callable=..., receiver=..., argc=0, argv=0x0, convert_receiver=false) at ../src/execution.cc:149 #27 0x008abd39 in v8::internal::Genesis::CompileScriptCached (isolate=0x1ea5130, name=..., source=..., cache=0x0, extension=0x0, top_context=..., use_runtime_context=true) at ../src/bootstrapper.cc:1551 #28 0x008ab9bf in v8::internal::Genesis::CompileNative (isolate=0x1ea5130,
[v8-dev] Re: Allow HPushArgument to handle more than one argument. (issue 296113008)
On 2014/05/23 10:31:58, ulan wrote: LGTM with one nit: please use int instead of unsigned where possible to avoid mixing them. Uploaded the fixed patch. Can you confirm it's still ok? https://codereview.chromium.org/296113008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Provide a helper to generate multiple Lithium instructions for one Hydrogen instruction. (issue 296993002)
Committed patchset #2 manually as r21465 (presubmit successful). https://codereview.chromium.org/296993002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Allow HPushArgument to handle more than one argument. (issue 296113008)
Committed patchset #3 manually as r21468 (presubmit successful). https://codereview.chromium.org/296113008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Provide a helper to generate multiple Lithium instructions for one Hydrogen instruction. (issue 296993002)
Reviewers: ulan, Jakob,
Message:
This is a pre-requisite commit for issue 296113008.
Description:
Provide a helper to generate multiple Lithium instructions for one Hydrogen
instruction.
Please review this at https://codereview.chromium.org/296993002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+65, -25 lines):
M src/arm/lithium-arm.h
M src/arm/lithium-arm.cc
M src/arm64/lithium-arm64.h
M src/arm64/lithium-arm64.cc
M src/ia32/lithium-ia32.h
M src/ia32/lithium-ia32.cc
M src/mips/lithium-mips.h
M src/mips/lithium-mips.cc
M src/x64/lithium-x64.h
M src/x64/lithium-x64.cc
Index: src/arm/lithium-arm.cc
diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index
1e54695d3ffec4eb969015abdae659f842a8e835..a35470ec209aced9012c4a455337c533d5f529f8
100644
--- a/src/arm/lithium-arm.cc
+++ b/src/arm/lithium-arm.cc
@@ -834,10 +834,18 @@ void LChunkBuilder::VisitInstruction(HInstruction*
current) {
argument_count_ += current-argument_delta();
ASSERT(argument_count_ = 0);
+ CheckAndAddInstruction(instr, current);
+
+ current_instruction_ = old_current;
+}
+
+
+void LChunkBuilder::CheckAndAddInstruction(LInstruction* instr,
+ HInstruction* hydrogen_val) {
if (instr != NULL) {
// Associate the hydrogen instruction first, since we may need it for
// the ClobbersRegisters() or ClobbersDoubleRegisters() calls below.
-instr-set_hydrogen_value(current);
+instr-set_hydrogen_value(hydrogen_val);
#if DEBUG
// Make sure that the lithium instruction has either no fixed register
@@ -878,10 +886,10 @@ void LChunkBuilder::VisitInstruction(HInstruction*
current) {
chunk_-AddInstruction(instr, current_block_);
if (instr-IsCall()) {
- HValue* hydrogen_value_for_lazy_bailout = current;
+ HValue* hydrogen_value_for_lazy_bailout = hydrogen_val;
LInstruction* instruction_needing_environment = NULL;
- if (current-HasObservableSideEffects()) {
-HSimulate* sim = HSimulate::cast(current-next());
+ if (hydrogen_val-HasObservableSideEffects()) {
+HSimulate* sim = HSimulate::cast(hydrogen_val-next());
instruction_needing_environment = instr;
sim-ReplayEnvironment(current_block_-last_environment());
hydrogen_value_for_lazy_bailout = sim;
@@ -897,7 +905,6 @@ void LChunkBuilder::VisitInstruction(HInstruction*
current) {
}
}
}
- current_instruction_ = old_current;
}
Index: src/arm/lithium-arm.h
diff --git a/src/arm/lithium-arm.h b/src/arm/lithium-arm.h
index
714ec55e2a984f0b58ec2600e1accfccb4046511..aae2ac98812dc5e0889c986844eb83aa5a1923be
100644
--- a/src/arm/lithium-arm.h
+++ b/src/arm/lithium-arm.h
@@ -2837,6 +2837,7 @@ class LChunkBuilder V8_FINAL : public
LChunkBuilderBase {
CanDeoptimize can_deoptimize = CANNOT_DEOPTIMIZE_EAGERLY);
void VisitInstruction(HInstruction* current);
+ void CheckAndAddInstruction(LInstruction* instr, HInstruction* current);
void DoBasicBlock(HBasicBlock* block, HBasicBlock* next_block);
LInstruction* DoShift(Token::Value op, HBitwiseBinaryOperation* instr);
Index: src/arm64/lithium-arm64.cc
diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc
index
d18808a587b1cd2a50dc9a58120786de46a978ea..3d3f724fe23ce8c73f9b1102dd29de5432c85282
100644
--- a/src/arm64/lithium-arm64.cc
+++ b/src/arm64/lithium-arm64.cc
@@ -678,10 +678,18 @@ void LChunkBuilder::VisitInstruction(HInstruction*
current) {
argument_count_ += current-argument_delta();
ASSERT(argument_count_ = 0);
+ CheckAndAddInstruction(instr, current);
+
+ current_instruction_ = old_current;
+}
+
+
+void LChunkBuilder::CheckAndAddInstruction(LInstruction* instr,
+ HInstruction* hydrogen_val) {
if (instr != NULL) {
// Associate the hydrogen instruction first, since we may need it for
// the ClobbersRegisters() or ClobbersDoubleRegisters() calls below.
-instr-set_hydrogen_value(current);
+instr-set_hydrogen_value(hydrogen_val);
#if DEBUG
// Make sure that the lithium instruction has either no fixed register
@@ -722,10 +730,10 @@ void LChunkBuilder::VisitInstruction(HInstruction*
current) {
chunk_-AddInstruction(instr, current_block_);
if (instr-IsCall()) {
- HValue* hydrogen_value_for_lazy_bailout = current;
+ HValue* hydrogen_value_for_lazy_bailout = hydrogen_val;
LInstruction* instruction_needing_environment = NULL;
- if (current-HasObservableSideEffects()) {
-HSimulate* sim = HSimulate::cast(current-next());
+ if (hydrogen_val-HasObservableSideEffects()) {
+HSimulate* sim = HSimulate::cast(hydrogen_val-next());
instruction_needing_environment = instr;
sim-ReplayEnvironment(current_block_-last_environment());
hydrogen_value_for_lazy_bailout = sim;
@@ -741,7 +749,6 @@ void
[v8-dev] Allow HPushArgument to handle more than one argument. (issue 296113008)
Reviewers: , Message: The diff includes the diff for issue 296993002 since git cl uploaded everything, but that can be committed separately. Description: Allow HPushArgument to handle more than one argument. Please review this at https://codereview.chromium.org/296113008/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+269, -105 lines): M src/arm/lithium-arm.h M src/arm/lithium-arm.cc M src/arm64/lithium-arm64.h M src/arm64/lithium-arm64.cc M src/arm64/lithium-codegen-arm64.cc M src/arm64/macro-assembler-arm64.h M src/arm64/macro-assembler-arm64.cc M src/code-stubs-hydrogen.cc M src/hydrogen.cc M src/hydrogen-instructions.h M src/hydrogen-instructions.cc M src/ia32/lithium-ia32.h M src/ia32/lithium-ia32.cc M src/lithium.cc M src/mips/lithium-mips.h M src/mips/lithium-mips.cc M src/x64/lithium-x64.h M src/x64/lithium-x64.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Allow HPushArgument to handle more than one argument. (issue 296113008)
https://codereview.chromium.org/296113008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Allow HPushArgument to handle more than one argument. (issue 296113008)
On 2014/05/22 13:01:09, Jakob wrote: On 2014/05/22 12:57:03, Alexandre Rames wrote: The diff includes the diff for issue 296993002 since git cl uploaded everything, DBC: You can use git cl upload my_other_branch to upload the diff to my_other_branch (which can be any commit descriptor git understands). Thanks. Just did this to upload only the patch of interest. https://codereview.chromium.org/296113008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Provide a helper to generate multiple Lithium instructions for one Hydrogen instruction. (issue 296993002)
Yes, all platforms have the same changes.
https://codereview.chromium.org/296993002/diff/1/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):
https://codereview.chromium.org/296993002/diff/1/src/arm/lithium-arm.cc#newcode843
src/arm/lithium-arm.cc:843: void
LChunkBuilder::CheckAndAddInstruction(LInstruction* instr,
On 2014/05/22 12:59:46, Jakob wrote:
I'd call this just AddInstruction...
Done.
https://codereview.chromium.org/296993002/diff/1/src/arm/lithium-arm.cc#newcode845
src/arm/lithium-arm.cc:845: if (instr != NULL) {
On 2014/05/22 12:59:46, Jakob wrote:
...and move this check to the caller. I don't think other callers need
the
flexibility to pass in NULL.
Done.
https://codereview.chromium.org/296993002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Provide a helper to generate multiple Lithium instructions for one Hydrogen instruction. (issue 296993002)
On 2014/05/22 14:56:20, Jakob wrote: Please don't forget to upload the updated patch set for future reference. Just did. I was waiting for our internal green flag. https://codereview.chromium.org/296993002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Use the shifter operand to merge in previous shift instructions. (issue 257203002)
On 2014/05/05 07:30:45, ulan wrote: lgtm Was landed by Martyn in r21161. https://codereview.chromium.org/257203002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Use the shifter operand to merge in previous shift instructions. (issue 257203002)
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc File src/arm64/lithium-arm64.cc (right): https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc#newcode2047 src/arm64/lithium-arm64.cc:2047: if (!val-IsBitwise() !(val-IsAdd() || val-IsSub())) return NULL; On 2014/05/02 10:01:18, ulan wrote: Simpler condition: !(val-IsBitwise() || val-IsAdd() || val-IsSub()) Done. https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc#newcode2139 src/arm64/lithium-arm64.cc:2139: bool can_overflow = hinstr-CheckFlag(HValue::kCanOverflow); On 2014/05/02 10:01:18, ulan wrote: You can inline hinstr-CheckFlag(HValue::kCanOverflow) below without can_overflow Done. https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc#newcode2148 src/arm64/lithium-arm64.cc:2148: UNREACHABLE(); On 2014/05/02 10:01:18, ulan wrote: nit: I'd ASSERT(hinstr-IsSub()) above. Done. https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h File src/arm64/lithium-arm64.h (right): https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h#newcode563 src/arm64/lithium-arm64.h:563: public: On 2014/05/02 10:01:18, ulan wrote: Google style guide doesn't allow non-pure multiple inheritance. One possible solution is to turn each function that accepts LShiftedRightOpInterface* into a template with L-instructions as a template parameter. Done. https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h#newcode3098 src/arm64/lithium-arm64.h:3098: return Assembler::IsImmAddSub(imm) || Assembler::IsImmAddSub(-imm); On 2014/05/02 10:01:18, ulan wrote: Did you mean instead of ||? '||' was intended. IsImmAddSub() works with unsigned values. https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h#newcode3104 src/arm64/lithium-arm64.h:3104: UNREACHABLE(); On 2014/05/02 10:01:18, ulan wrote: nit: I'd slightly prefer a more compact ASSERT(instr-IsBitwise()) above without this branch. Done. https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-codegen-arm64.cc File src/arm64/lithium-codegen-arm64.cc (right): https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-codegen-arm64.cc#newcode1268 src/arm64/lithium-codegen-arm64.cc:1268: ToInteger32(LConstantOperand::cast(shift_info-shift_amount())) 0x1f); On 2014/05/02 10:01:18, ulan wrote: Could you name the 0x1f constant since it is used in other places too? Abstracted the constant and used a helper. https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-codegen-arm64.cc#newcode1270 src/arm64/lithium-codegen-arm64.cc:1270: } On 2014/05/02 10:01:18, ulan wrote: Two empty lines after function. Done. https://codereview.chromium.org/257203002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Use the shifter operand to merge in previous shift instructions. (issue 257203002)
Uploaded the updated patch. https://codereview.chromium.org/257203002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
https://codereview.chromium.org/258793002/diff/50001/src/arm64/lithium-arm64.h File src/arm64/lithium-arm64.h (right): https://codereview.chromium.org/258793002/diff/50001/src/arm64/lithium-arm64.h#newcode1912 src/arm64/lithium-arm64.h:1912: /* Math.floor with a double result. */ On 2014/04/30 09:18:41, ulan wrote: Please use // comment to be consistent with other code. Done. https://codereview.chromium.org/258793002/diff/50001/src/arm64/lithium-arm64.h#newcode1920 src/arm64/lithium-arm64.h:1920: /* Math.floor with an integer result. */ On 2014/04/30 09:18:41, ulan wrote: ditto Done. https://codereview.chromium.org/258793002/diff/50001/src/arm64/lithium-arm64.h#newcode2020 src/arm64/lithium-arm64.h:2020: /* Math.round with an integer result. */ On 2014/04/30 09:18:41, ulan wrote: ditto Done. https://codereview.chromium.org/258793002/diff/50001/src/arm64/lithium-arm64.h#newcode2031 src/arm64/lithium-arm64.h:2031: /* Math.round with an integer result. */ On 2014/04/30 09:18:41, ulan wrote: ditto Done. https://codereview.chromium.org/258793002/diff/50001/src/arm64/lithium-codegen-arm64.cc File src/arm64/lithium-codegen-arm64.cc (right): https://codereview.chromium.org/258793002/diff/50001/src/arm64/lithium-codegen-arm64.cc#newcode4019 src/arm64/lithium-codegen-arm64.cc:4019: __ B(eq, done); On 2014/04/30 09:18:41, ulan wrote: I see how it handles 0.0 and negative integers, but how does this handle positive input? If the input is greater or equal to zero, the 'lt' (less than) condition is not met, so FCCMP sets the Z condition flag, and the following branch is taken. https://codereview.chromium.org/258793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
Committed patchset #5 manually as r21091 (presubmit successful). https://codereview.chromium.org/258793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Use the shifter operand to merge in previous shift instructions. (issue 257203002)
Reviewers: ulan, jochen, Description: ARM64: Use the shifter operand to merge in previous shift instructions. When possible, we transform sequences of code of the form lsl x8, x9, #imm add x0, x1, x8 into add x0, x1, x9 LSL #imm Please review this at https://codereview.chromium.org/257203002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+252, -13 lines): M src/arm64/assembler-arm64.h M src/arm64/lithium-arm64.h M src/arm64/lithium-arm64.cc M src/arm64/lithium-codegen-arm64.h M src/arm64/lithium-codegen-arm64.cc M src/hydrogen-instructions.h -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
Reworked the type inference. https://codereview.chromium.org/258793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Use the shifter operand to merge in previous shift instructions. (issue 257203002)
On 2014/04/29 15:01:44, ulan wrote: Alexandre, first question before reviewing: does this optimization trigger in benchmarks? If so how does it affect performance? The patch gives a ~15% performance boost on v8/Crypto. https://codereview.chromium.org/257203002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
Just uploaded an updated patch. I have extended the existing testing to also test the 'double output' path. https://codereview.chromium.org/258793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
On 2014/04/28 13:53:31, Jakob wrote:
Any reason you're not looking at both required and observed
representations as
I
suggested? If the only use is e.g. another unary math function (which
requires
double inputs and, being a call, doesn't have any observed
representation), it
would be nice to avoid deopts too. (If in my previous example you use
function
f(x) { return Math.log(Math.round(x)); } that should exhibit what I
mean.)
I had misunderstood your first comment. Updated the code to use both
oberved and
required representations, but prefer the observed representation (fixes the
`Math.log(Math.round())` test-case) and also use a double output
representation
for tagged inputs (fixes the `Math.round(x) + .` test-case).
https://codereview.chromium.org/258793002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
I was confused. I understood the double representation part, but somehow just messed up the code. While looking at the first example I confused 'input' and 'use' representations... I'll get some rest and update the patch tomorrow! https://codereview.chromium.org/258793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
Reviewers: ulan, jochen, Description: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. Please review this at https://codereview.chromium.org/258793002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+154, -27 lines): M src/arm64/constants-arm64.h M src/arm64/lithium-arm64.h M src/arm64/lithium-arm64.cc M src/arm64/lithium-codegen-arm64.cc M src/hydrogen-instructions.h M src/hydrogen-instructions.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Generate optimized code for Math.floor and Math.round with double outputs. (issue 258793002)
I will update the patch to address your comments, and update the round and
floor
testing for the new paths.
https://codereview.chromium.org/258793002/diff/1/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/258793002/diff/1/src/hydrogen-instructions.cc#newcode4211
src/hydrogen-instructions.cc:4211: if (!rep_required) {
Removed consideration of the required representation.
On 2014/04/25 14:12:03, Jakob wrote:
I'm not sure I like the way you're prioritizing required over observed
representations. If any of the uses (ideally, including transitive
uses through
Phis, but that's clearly outside the scope of this CL) has observed
doubles,
that should always be enough of a reason to choose double
representation.
Example:
function f(x) { return Math.round(x) + .; }
f(0x8000);
f(0x8000);
%OptimizeFunctionOnNextCall(f);
f(0x8000);
This will deopt with the current patch, because the addition requires
tagged
inputs, so the Round ignores it and chooses int32 representation. If
you honor
the observed double feedback, you can avoid the deopt.
I think you'll just have to look at both required and observed
representations.
https://codereview.chromium.org/258793002/diff/1/src/hydrogen-instructions.cc#newcode4224
src/hydrogen-instructions.cc:4224: break;
On 2014/04/25 14:12:03, Jakob wrote:
nit: indentation. Also, a comment would be nice (e.g. Having seen one
double
use is enough.), because changing control flow based on a tracing
flag looks
quite surprising at first.
Done.
https://codereview.chromium.org/258793002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix the atomic type width in the 64bit Acquire_Load(). (issue 220203012)
On 2014/04/17 12:33:56, rmcilroy wrote: On 2014/04/03 14:28:47, Alexandre Rames wrote: Committed patchset #1 manually as r20486 (presubmit successful). Could you let me know if you fix any bugs in atomicops_internals_arm64_gcc.h in the future so that I can include the fix in Chrome's base/ implementation. I just spent quite a while debugging a crash in Chrome which turned out to be due to this bug, but I didn't know you'd already fixed in V8. Ouch. Sorry about that. I will keep it in mind. https://codereview.chromium.org/220203012/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix EnsureSpace to not emit pools. (issue 230223005)
Committed patchset #1 manually as r20714 (presubmit successful). https://codereview.chromium.org/230223005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid iterating through all unresolved branch infos when many are pending. (issue 227043010)
Committed patchset #2 manually as r20715 (presubmit successful). https://codereview.chromium.org/227043010/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Reland A64: Now that we have veneers, fix a couple of branches to directly jump to their target. (issue 236163006)
Reviewers: ulan, jochen,
Description:
Reland A64: Now that we have veneers, fix a couple of branches to directly
jump
to their target.
This was reverted by revision 20187.
make -j45 arm64.release.check TESTFLAGS=--time
TESTJOBS=mozilla/js1_5/Regress/regress-280769-2
Without the patch:
--- Total time: 00:07.039 ---
1 (00:07.016) mozilla/js1_5/Regress/regress-280769-2
2 (00:01.814) mozilla/js1_5/Regress/regress-280769-2
3 (00:01.812) mozilla/js1_5/Regress/regress-280769-2
With the patch:
--- Total time: 00:12.252 ---
1 (00:12.226) mozilla/js1_5/Regress/regress-280769-2
2 (00:04.217) mozilla/js1_5/Regress/regress-280769-2
3 (00:04.213) mozilla/js1_5/Regress/regress-280769-2
Please review this at https://codereview.chromium.org/236163006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+3, -12 lines):
M src/arm64/regexp-macro-assembler-arm64.cc
Index: src/arm64/regexp-macro-assembler-arm64.cc
diff --git a/src/arm64/regexp-macro-assembler-arm64.cc
b/src/arm64/regexp-macro-assembler-arm64.cc
index
decf314fa798d58194e11ad569e3b95fa1456b30..1cce0a66a2b24e2a0b1b5d43815c512715e78fa3
100644
--- a/src/arm64/regexp-macro-assembler-arm64.cc
+++ b/src/arm64/regexp-macro-assembler-arm64.cc
@@ -1481,12 +1481,7 @@ void
RegExpMacroAssemblerARM64::BranchOrBacktrack(Condition condition,
if (to == NULL) {
to = backtrack_label_;
}
- // TODO(ulan): do direct jump when jump distance is known and fits in
imm19.
- Condition inverted_condition = InvertCondition(condition);
- Label no_branch;
- __ B(inverted_condition, no_branch);
- __ B(to);
- __ Bind(no_branch);
+ __ B(condition, to);
}
void RegExpMacroAssemblerARM64::CompareAndBranchOrBacktrack(Register reg,
@@ -1497,15 +1492,11 @@ void
RegExpMacroAssemblerARM64::CompareAndBranchOrBacktrack(Register reg,
if (to == NULL) {
to = backtrack_label_;
}
-// TODO(ulan): do direct jump when jump distance is known and fits in
imm19.
-Label no_branch;
if (condition == eq) {
- __ Cbnz(reg, no_branch);
+ __ Cbz(reg, to);
} else {
- __ Cbz(reg, no_branch);
+ __ Cbnz(reg, to);
}
-__ B(to);
-__ Bind(no_branch);
} else {
__ Cmp(reg, immediate);
BranchOrBacktrack(condition, to);
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Reland A64: Now that we have veneers, fix a couple of branches to directly jump to their target. (issue 236163006)
On 2014/04/14 12:15:20, ulan wrote: On 2014/04/14 12:09:42, Alexandre Rames wrote: Do you know how this affects octane/regexp benchmark? Good point. This shows a ~8% performance penalty, so maybe it is better not to push it. I note that I should profile the regexp benchmark once we have 'perf', so I can check there are no other sites where we would do better without veneers. https://codereview.chromium.org/236163006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Fix EnsureSpace to not emit pools. (issue 230223005)
Reviewers: ulan, jochen,
Description:
ARM64: Fix EnsureSpace to not emit pools.
The EnsureSpace scope must not trigger any code emission, so it should only
grow
the buffer, and not check for pools.
Please review this at https://codereview.chromium.org/230223005/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+8, -2 lines):
M src/arm64/assembler-arm64.h
M src/arm64/assembler-arm64-inl.h
Index: src/arm64/assembler-arm64-inl.h
diff --git a/src/arm64/assembler-arm64-inl.h
b/src/arm64/assembler-arm64-inl.h
index
b56e3ed2a18a3aff20778a4240c25af67596d170..3d12ac076c39c150e8f5f2bdba79be53eac9ad8d
100644
--- a/src/arm64/assembler-arm64-inl.h
+++ b/src/arm64/assembler-arm64-inl.h
@@ -1199,11 +1199,16 @@ void Assembler::LoadRelocated(const CPURegister
rt, const Operand operand) {
}
-inline void Assembler::CheckBuffer() {
+inline void Assembler::CheckBufferSpace() {
ASSERT(pc_ (buffer_ + buffer_size_));
if (buffer_space() kGap) {
GrowBuffer();
}
+}
+
+
+inline void Assembler::CheckBuffer() {
+ CheckBufferSpace();
if (pc_offset() = next_veneer_pool_check_) {
CheckVeneerPool(false, true);
}
Index: src/arm64/assembler-arm64.h
diff --git a/src/arm64/assembler-arm64.h b/src/arm64/assembler-arm64.h
index
9a8cd467a0f7b8425c6dabe754f9f7f89cc54686..dcf83078709593fce60785d6af0c34eb385d2cc6
100644
--- a/src/arm64/assembler-arm64.h
+++ b/src/arm64/assembler-arm64.h
@@ -2040,6 +2040,7 @@ class Assembler : public AssemblerBase {
}
void GrowBuffer();
+ void CheckBufferSpace();
void CheckBuffer();
// Pc offset of the next constant pool check.
@@ -2238,7 +2239,7 @@ class PatchingAssembler : public Assembler {
class EnsureSpace BASE_EMBEDDED {
public:
explicit EnsureSpace(Assembler* assembler) {
-assembler-CheckBuffer();
+assembler-CheckBufferSpace();
}
};
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid iterating through all unresolved branch infos when many are pending. (issue 227043010)
https://codereview.chromium.org/227043010/diff/1/src/arm64/assembler-arm64.cc File src/arm64/assembler-arm64.cc (right): https://codereview.chromium.org/227043010/diff/1/src/arm64/assembler-arm64.cc#newcode612 src/arm64/assembler-arm64.cc:612: DeleteUnresolvedBranchInfoForLabelTraverse(label); On 2014/04/07 13:24:41, ulan wrote: Ignoring ADR instructions, chain length = unresolved_branches_.size(), right? Yes. Then why not always use this function? I initially kept it as I was worried that digging through the chain could be more expensive than a simple iteration. Removed the iterative function to always traverse the label chain. https://codereview.chromium.org/227043010/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix disassembly of branch targets. (issue 225743004)
Committed patchset #1 manually as r20543 (presubmit successful). https://codereview.chromium.org/225743004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Introduce a version of ADR handling distant targets. (issue 222433002)
Committed patchset #2 manually as r20545 (presubmit successful). https://codereview.chromium.org/222433002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Avoid iterating through all unresolved branch infos when many are pending. (issue 227043010)
Reviewers: ulan, jochen,
Message:
This patch diminishes the regression observed at
https://codereview.chromium.org/209333004 .
With this patch applied, the timings for
make -j32 arm64.release.check TESTFLAGS=--time
TESTJOBS=mozilla/js1_5/Regress/regress-280769-2
become:
Without the regexp veneer patch:
1 (00:06.818) mozilla/js1_5/Regress/regress-280769-2
2 (00:01.910) mozilla/js1_5/Regress/regress-280769-2
3 (00:01.711) mozilla/js1_5/Regress/regress-280769-2
With the regexp veneer patch:
1 (00:11.922) mozilla/js1_5/Regress/regress-280769-2
2 (00:04.014) mozilla/js1_5/Regress/regress-280769-2
3 (00:04.014) mozilla/js1_5/Regress/regress-280769-2
Description:
ARM64: Avoid iterating through all unresolved branch infos when many are
pending.
Instead, inspect the label chain and delete pending information for every
branch
in the chain.
Please review this at https://codereview.chromium.org/227043010/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+75, -9 lines):
M src/arm64/assembler-arm64.h
M src/arm64/assembler-arm64.cc
src/arm64/instructions-arm64.h
Index: src/arm64/assembler-arm64.cc
diff --git a/src/arm64/assembler-arm64.cc b/src/arm64/assembler-arm64.cc
index
5695623afd54e033ca6ec9ebfb349cef159b14c4..43bea9f160f8e0e32ed151fda5517e42be164dd5
100644
--- a/src/arm64/assembler-arm64.cc
+++ b/src/arm64/assembler-arm64.cc
@@ -545,6 +545,53 @@ int Assembler::LinkAndGetByteOffsetTo(Label* label) {
}
+void Assembler::DeleteUnresolvedBranchInfoForLabelIterate(Label* label) {
+ std::multimapint, FarBranchInfo::iterator it_tmp, it;
+ it = unresolved_branches_.begin();
+ while (it != unresolved_branches_.end()) {
+it_tmp = it++;
+if (it_tmp-second.label_ == label) {
+ CHECK(it_tmp-first = pc_offset());
+ unresolved_branches_.erase(it_tmp);
+}
+ }
+}
+
+
+void Assembler::DeleteUnresolvedBranchInfoForLabelTraverse(Label* label) {
+ ASSERT(label-is_linked());
+ CheckLabelLinkChain(label);
+
+ int link_offset = label-pos();
+ int link_pcoffset;
+ bool end_of_chain = false;
+
+ while (!end_of_chain) {
+Instruction * link = InstructionAt(link_offset);
+link_pcoffset = link-ImmPCOffset();
+
+// ADR instructions are not handled by veneers.
+if (link-IsImmBranch()) {
+ int max_reachable_pc = InstructionOffset(link) +
+ Instruction::ImmBranchRange(link-BranchType());
+ typedef std::multimapint, FarBranchInfo::iterator
unresolved_info_it;
+ std::pairunresolved_info_it, unresolved_info_it range;
+ range = unresolved_branches_.equal_range(max_reachable_pc);
+ unresolved_info_it it;
+ for (it = range.first; it != range.second; ++it) {
+if (it-second.pc_offset_ == link_offset) {
+ unresolved_branches_.erase(it);
+ break;
+}
+ }
+}
+
+end_of_chain = (link_pcoffset == 0);
+link_offset = link_offset + link_pcoffset;
+ }
+}
+
+
void Assembler::DeleteUnresolvedBranchInfoForLabel(Label* label) {
if (unresolved_branches_.empty()) {
ASSERT(next_veneer_pool_check_ == kMaxInt);
@@ -552,15 +599,17 @@ void
Assembler::DeleteUnresolvedBranchInfoForLabel(Label* label) {
}
if (label-is_linked()) {
-// Branches to this label will be resolved when the label is bound
below.
-std::multimapint, FarBranchInfo::iterator it_tmp, it;
-it = unresolved_branches_.begin();
-while (it != unresolved_branches_.end()) {
- it_tmp = it++;
- if (it_tmp-second.label_ == label) {
-CHECK(it_tmp-first = pc_offset());
-unresolved_branches_.erase(it_tmp);
- }
+// Branches to this label will be resolved when the label is bound,
normally
+// just after all the associated info has been deleted.
+
+// If there are too many unresolved branches pending, avoid iterating
+// through all of them to find the information to delete.
+static const int kMaxNInfoIterate = 128;
+
+if (unresolved_branches_.size() kMaxNInfoIterate) {
+ DeleteUnresolvedBranchInfoForLabelIterate(label);
+} else {
+ DeleteUnresolvedBranchInfoForLabelTraverse(label);
}
}
if (unresolved_branches_.empty()) {
Index: src/arm64/assembler-arm64.h
diff --git a/src/arm64/assembler-arm64.h b/src/arm64/assembler-arm64.h
index
9a8cd467a0f7b8425c6dabe754f9f7f89cc54686..b0dd3b84addd1a32cd059d09a78646e292f3d08b
100644
--- a/src/arm64/assembler-arm64.h
+++ b/src/arm64/assembler-arm64.h
@@ -1694,6 +1694,10 @@ class Assembler : public AssemblerBase {
return reinterpret_castInstruction*(buffer_ + offset);
}
+ ptrdiff_t InstructionOffset(Instruction* instr) const {
+return reinterpret_castbyte*(instr) - buffer_;
+ }
+
// Register encoding.
static Instr Rd(CPURegister rd) {
ASSERT(rd.code() != kSPRegInternalCode);
@@ -2182,6 +2186,15 @@ class Assembler : public AssemblerBase {
// not later attempt (likely unsuccessfully) to patch it to branch
directly to
//
[v8-dev] ARM64: Use direct deoptimization exits. (issue 225703002)
Reviewers: ulan, jochen, Message: This gives about 5% performance improvement on the v8 and kraken benchmarks. Description: ARM64: Use direct deoptimization exits. Please review this at https://codereview.chromium.org/225703002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+47, -69 lines): M src/arm64/lithium-codegen-arm64.cc M src/arm64/macro-assembler-arm64.h -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Use direct deoptimization exits. (issue 225703002)
https://codereview.chromium.org/225703002/diff/1/src/arm64/lithium-codegen-arm64.cc
File src/arm64/lithium-codegen-arm64.cc (right):
https://codereview.chromium.org/225703002/diff/1/src/arm64/lithium-codegen-arm64.cc#newcode2153
src/arm64/lithium-codegen-arm64.cc:2153: }
On 2014/04/04 12:04:13, ulan wrote:
Instead of checking for map_set.size() == 1, how about always doing
this
optimization for the last element of the map_set?
Something along the lines:
ASSERT(map_set.size() 0);
int i = 0;
for (; i + 1 map_set.size(); i++) {
...
__ B(eq, success);
}
HandleMap map = map_set.at(i).handle();
__ CompareMap(map_reg, map);
deoptimize if ne
Done.
https://codereview.chromium.org/225703002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Use direct deoptimization exits. (issue 225703002)
On 2014/04/04 12:04:12, ulan wrote: This gives about 5% performance improvement on the v8 and kraken benchmarks. Does arm64 deopt more than arm? This patch spares the cpu from taking unconditional branches to jump over deopt sites. That's where the improvement comes from. https://codereview.chromium.org/225703002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Fix disassembly of branch targets. (issue 225743004)
Reviewers: ulan, jochen,
Description:
ARM64: Fix disassembly of branch targets.
Please review this at https://codereview.chromium.org/225743004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+1, -2 lines):
M src/arm64/disasm-arm64.cc
Index: src/arm64/disasm-arm64.cc
diff --git a/src/arm64/disasm-arm64.cc b/src/arm64/disasm-arm64.cc
index
ed3e928796bde139229ac22ff5c7f8476b65bd6a..9886d20d26e2af7124ca5e97f25fd24885c025c8
100644
--- a/src/arm64/disasm-arm64.cc
+++ b/src/arm64/disasm-arm64.cc
@@ -1632,10 +1632,9 @@ int
Disassembler::SubstituteBranchTargetField(Instruction* instr,
offset = kInstructionSizeLog2;
char sign = '+';
if (offset 0) {
-offset = -offset;
sign = '-';
}
- AppendToOutput(#%c0x% PRIx64 (addr %p), sign, offset,
+ AppendToOutput(#%c0x% PRIx64 (addr %p), sign, Abs(offset),
instr-InstructionAtOffset(offset),
Instruction::NO_CHECK);
return 8;
}
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Use direct deoptimization exits. (issue 225703002)
Committed patchset #2 manually as r20515 (presubmit successful). https://codereview.chromium.org/225703002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Fix the atomic type width in the 64bit Acquire_Load(). (issue 220203012)
Reviewers: ulan, jochen, JF,
Description:
ARM64: Fix the atomic type width in the 64bit Acquire_Load().
Please review this at https://codereview.chromium.org/220203012/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+1, -1 lines):
M src/atomicops_internals_arm64_gcc.h
Index: src/atomicops_internals_arm64_gcc.h
diff --git a/src/atomicops_internals_arm64_gcc.h
b/src/atomicops_internals_arm64_gcc.h
index
356b89734303b62b3ce5774e27e3d80f49f644d8..f6ed5de602695239e943a96ecb8744d22ae7cbff
100644
--- a/src/atomicops_internals_arm64_gcc.h
+++ b/src/atomicops_internals_arm64_gcc.h
@@ -309,7 +309,7 @@ inline Atomic64 NoBarrier_Load(volatile const Atomic64*
ptr) {
}
inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) {
- Atomic32 value;
+ Atomic64 value;
__asm__ __volatile__ ( // NOLINT
ldar %x[value], %[ptr] \n\t
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Fixes and more support for FRINTX instructions. (issue 223843002)
Reviewers: ulan, jochen, Description: ARM64: Fixes and more support for FRINTX instructions. Fix simulation and tests for the [-0.5, -0.0[ range for FRINTA and FRINTN, and add support for FRINTM. Please review this at https://codereview.chromium.org/223843002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+174, -42 lines): M src/arm64/assembler-arm64.h M src/arm64/assembler-arm64.cc M src/arm64/macro-assembler-arm64.h M src/arm64/macro-assembler-arm64-inl.h M src/arm64/simulator-arm64.cc M test/cctest/test-assembler-arm64.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix the atomic type width in the 64bit Acquire_Load(). (issue 220203012)
Committed patchset #1 manually as r20486 (presubmit successful). https://codereview.chromium.org/220203012/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fixes and more support for FRINTX instructions. (issue 223843002)
Committed patchset #1 manually as r20487 (presubmit successful). https://codereview.chromium.org/223843002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix and improve atomic operations. (issue 220793002)
Following JF's comment, I updated the clobber list of NoBarrier operations to include memory, and added a comment explaining why. Tests showed no difference in the code generated between integrating the barrier in the inline asm and using a function MemoryBarrier(), so I prefer not duplicating the assembly code. https://codereview.chromium.org/220793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Fix cctest/test-disasm-arm64/debug on real hardware. (issue 221903004)
Reviewers: ulan, jochen,
Description:
ARM64: Fix cctest/test-disasm-arm64/debug on real hardware.
Without the 'BREAK' parameter, no instruction is generated when we are not
running with the simulator.
Please review this at https://codereview.chromium.org/221903004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+7, -7 lines):
M test/cctest/test-disasm-arm64.cc
Index: test/cctest/test-disasm-arm64.cc
diff --git a/test/cctest/test-disasm-arm64.cc
b/test/cctest/test-disasm-arm64.cc
index
3343175e9362ae07176c16b14088525798c4191e..23f7b6daf26a7c9d551aa84a9e0d5282406df3fb
100644
--- a/test/cctest/test-disasm-arm64.cc
+++ b/test/cctest/test-disasm-arm64.cc
@@ -1605,13 +1605,13 @@ TEST_(debug) {
// All debug codes should produce the same instruction, and the debug
code
// can be any uint32_t.
- COMPARE(debug(message, 0, NO_PARAM), hlt #0xdeb0);
- COMPARE(debug(message, 1, NO_PARAM), hlt #0xdeb0);
- COMPARE(debug(message, 0x, NO_PARAM), hlt #0xdeb0);
- COMPARE(debug(message, 0x1, NO_PARAM), hlt #0xdeb0);
- COMPARE(debug(message, 0x7fff, NO_PARAM), hlt #0xdeb0);
- COMPARE(debug(message, 0x8000u, NO_PARAM), hlt #0xdeb0);
- COMPARE(debug(message, 0xu, NO_PARAM), hlt #0xdeb0);
+ COMPARE(debug(message, 0, BREAK), hlt #0xdeb0);
+ COMPARE(debug(message, 1, BREAK), hlt #0xdeb0);
+ COMPARE(debug(message, 0x, BREAK), hlt #0xdeb0);
+ COMPARE(debug(message, 0x1, BREAK), hlt #0xdeb0);
+ COMPARE(debug(message, 0x7fff, BREAK), hlt #0xdeb0);
+ COMPARE(debug(message, 0x8000u, BREAK), hlt #0xdeb0);
+ COMPARE(debug(message, 0xu, BREAK), hlt #0xdeb0);
CLEANUP();
}
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix cctest/test-disasm-arm64/debug on real hardware. (issue 221903004)
Committed patchset #1 manually as r20443 (presubmit successful). https://codereview.chromium.org/221903004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Introduce a version of ADR handling distant targets. (issue 222433002)
Reviewers: ulan, jochen, Description: ARM64: Introduce a version of ADR handling distant targets. This fixes an out-of-range label error for an ADR instruction in the mozilla/data/js1_5/Regress/regress-280769-2.js test. Please review this at https://codereview.chromium.org/222433002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+217, -15 lines): M src/arm64/assembler-arm64.h M src/arm64/assembler-arm64.cc M src/arm64/instructions-arm64.h M src/arm64/instructions-arm64.cc M src/arm64/macro-assembler-arm64.h M src/arm64/macro-assembler-arm64.cc M src/arm64/macro-assembler-arm64-inl.h M src/arm64/regexp-macro-assembler-arm64.cc M test/cctest/test-assembler-arm64.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix and improve atomic operations. (issue 220793002)
On 2014/04/02 16:03:49, JF wrote: lgtm (Still need approval from an owner of the file (Ulan and Jochen for example) to allow committing.) https://codereview.chromium.org/220793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid iterating through unresolved branches information when unnecessary. (issue 217343007)
Committed patchset #1 manually as r20444 (presubmit successful). https://codereview.chromium.org/217343007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix and improve atomic operations. (issue 220793002)
Committed patchset #2 manually as r20446 (presubmit successful). https://codereview.chromium.org/220793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Fix and improve atomic operations. (issue 220793002)
Reviewers: ulan, jochen, rmcilroy, Description: A64: Fix and improve atomic operations. * The 'compare and swap' operations should enforce memory ordering even when the exchange does not occur. * The exclusive monitor does not need to be cleared by CLREX if a LDRX was not followed by a matching STREX. * Use LDAR and STLR where possible. * Use the 'Q' constraint where possible. This Aarch64 constraint indicates a memory address which uses a single base register with no offset. It allows us to remove 'memory' from the clobber list. * Use the 'I' and 'J' constraints to hint for constants valid for immediate values. * Avoid code duplication. Please review this at https://codereview.chromium.org/220793002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+53, -102 lines): M src/atomicops_internals_arm64_gcc.h -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix and improve atomic operations. (issue 220793002)
https://codereview.chromium.org/220793002/diff/1/src/atomicops_internals_arm64_gcc.h File src/atomicops_internals_arm64_gcc.h (right): https://codereview.chromium.org/220793002/diff/1/src/atomicops_internals_arm64_gcc.h#newcode57 src/atomicops_internals_arm64_gcc.h:57: : [old_value]IJr (old_value), On 2014/04/01 10:41:19, rmcilroy wrote: Do the I and J constraints require that the value fit in an immediate (12 bits) or is it just a hint that an immediate should be used if it does fit? It is just a hint. If the value does not fit it will use a register. (Without them it uses always uses a register.) https://codereview.chromium.org/220793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Avoid iterating through unresolved branches information when unnecessary. (issue 217343007)
Reviewers: ,
Message:
This is the first of a 2 patches that address the regression performance
spotted
in r20187.
Description:
ARM64: Avoid iterating through unresolved branches information when
unnecessary.
Please review this at https://codereview.chromium.org/217343007/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+12, -10 lines):
M src/arm64/assembler-arm64.cc
Index: src/arm64/assembler-arm64.cc
diff --git a/src/arm64/assembler-arm64.cc b/src/arm64/assembler-arm64.cc
index
8bee92ccc273a87b1e319481ef0697063c7f4075..ad2997c316070600fba82c707a0fd07ab0a162ff
100644
--- a/src/arm64/assembler-arm64.cc
+++ b/src/arm64/assembler-arm64.cc
@@ -456,6 +456,8 @@ void Assembler::bind(Label* label) {
ASSERT(!label-is_near_linked());
ASSERT(!label-is_bound());
+ DeleteUnresolvedBranchInfoForLabel(label);
+
// If the label is linked, the link chain looks something like this:
//
// |--II---I---L
@@ -497,8 +499,6 @@ void Assembler::bind(Label* label) {
ASSERT(label-is_bound());
ASSERT(!label-is_linked());
-
- DeleteUnresolvedBranchInfoForLabel(label);
}
@@ -551,14 +551,16 @@ void
Assembler::DeleteUnresolvedBranchInfoForLabel(Label* label) {
return;
}
- // Branches to this label will be resolved when the label is bound below.
- std::multimapint, FarBranchInfo::iterator it_tmp, it;
- it = unresolved_branches_.begin();
- while (it != unresolved_branches_.end()) {
-it_tmp = it++;
-if (it_tmp-second.label_ == label) {
- CHECK(it_tmp-first = pc_offset());
- unresolved_branches_.erase(it_tmp);
+ if (label-is_linked()) {
+// Branches to this label will be resolved when the label is bound
below.
+std::multimapint, FarBranchInfo::iterator it_tmp, it;
+it = unresolved_branches_.begin();
+while (it != unresolved_branches_.end()) {
+ it_tmp = it++;
+ if (it_tmp-second.label_ == label) {
+CHECK(it_tmp-first = pc_offset());
+unresolved_branches_.erase(it_tmp);
+ }
}
}
if (unresolved_branches_.empty()) {
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Avoid iterating through unresolved branches information when unnecessary. (issue 217343007)
https://codereview.chromium.org/217343007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix and improve atomic operations. (issue 220793002)
On 2014/04/01 16:08:09, JF wrote: I'm not sure I understand why memory is kept in some places and removed in others. lgtm otherwise. From http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html : If your assembler instructions access memory in an unpredictable fashion, add 'memory' to the list of clobbered registers. This causes GCC to not keep memory values cached in registers across the assembler instruction and not optimize stores or loads to that memory. You also should add the volatile keyword if the memory affected is not listed in the inputs or outputs of the asm, as the 'memory' clobber does not count as a side-effect of the asm. So memory is needed when we have barriers, but not when we use 'simple' load/store instructions. https://codereview.chromium.org/220793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix and improve atomic operations. (issue 220793002)
On 2014/04/01 16:40:25, JF wrote: On 2014/04/01 16:28:59, JF wrote: On 2014/04/01 16:19:36, Alexandre Rames wrote: On 2014/04/01 16:08:09, JF wrote: I'm not sure I understand why memory is kept in some places and removed in others. lgtm otherwise. From http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html : If your assembler instructions access memory in an unpredictable fashion, add 'memory' to the list of clobbered registers. This causes GCC to not keep memory values cached in registers across the assembler instruction and not optimize stores or loads to that memory. You also should add the volatile keyword if the memory affected is not listed in the inputs or outputs of the asm, as the 'memory' clobber does not count as a side-effect of the asm. So memory is needed when we have barriers, but not when we use 'simple' load/store instructions. I see, you want to ensure no memory values are registerized across acquire/release? In that case I think memory should be kept in functions that have a single barriers, and the barriers should probably be moved into the asm statements, otherwise the compiler could technically move memory operations between a barrier and the asm statement. e.g. Acquire_CompareAndSwap uses NoBarrier_CompareAndSwap followed by MemoryBarrier, and nothing technically prevents memory operations to move around. Yes! Forgot about that. I'll do a test to see what the compiler generates in both cases. Unless that's on purpose (i.e. you're telling the compiler to acquire or release at the time it finds most convenient, taking the barrier into account), in which case this should be documented. https://codereview.chromium.org/220793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Optimize AllocateHeapNumber to use STP. (issue 216933003)
Committed patchset #1 manually as r20356 (presubmit successful). https://codereview.chromium.org/216933003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] ARM64: Optimize AllocateHeapNumber to use STP. (issue 216933003)
Reviewers: ulan (offline till April 1), jochen, Description: ARM64: Optimize AllocateHeapNumber to use STP. Please review this at https://codereview.chromium.org/216933003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+101, -59 lines): M src/arm64/assembler-arm64.h M src/arm64/code-stubs-arm64.cc M src/arm64/codegen-arm64.cc M src/arm64/full-codegen-arm64.cc M src/arm64/macro-assembler-arm64.h M src/arm64/macro-assembler-arm64.cc M src/arm64/macro-assembler-arm64-inl.h M src/arm64/stub-cache-arm64.cc M src/objects.h -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Reland ARM64: Add overflow checking support for multiplications by constant powers of 2. (issue 212573002)
Reviewers: ulan, jochen,
Message:
Created a new issue for the re-land.
Description:
Reland ARM64: Add overflow checking support for multiplications by constant
powers of 2.
This includes fixes for the overflow case spotted in
mjsunit/mul-exhaustive-part4.
Please review this at https://codereview.chromium.org/212573002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+52, -23 lines):
M src/arm64/lithium-arm64.cc
M src/arm64/lithium-codegen-arm64.cc
Index: src/arm64/lithium-arm64.cc
diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc
index
c6f547f6284db0d091f52f21b02f3f9c8688..3d3ae97b0caab0f3c79e9d030a0737502b137a18
100644
--- a/src/arm64/lithium-arm64.cc
+++ b/src/arm64/lithium-arm64.cc
@@ -1891,29 +1891,36 @@ LInstruction* LChunkBuilder::DoMul(HMul* instr) {
HValue* least_const = instr-BetterLeftOperand();
HValue* most_const = instr-BetterRightOperand();
-LOperand* left = UseRegisterAtStart(least_const);
+LOperand* left;
// LMulConstI can handle a subset of constants:
// With support for overflow detection:
//-1, 0, 1, 2
-// Without support for overflow detection:
//2^n, -(2^n)
+// Without support for overflow detection:
//2^n + 1, -(2^n - 1)
if (most_const-IsConstant()) {
int32_t constant = HConstant::cast(most_const)-Integer32Value();
- int32_t constant_abs = (constant = 0) ? constant : -constant;
-
- if (((constant = -1) (constant = 2)) ||
- (!can_overflow (IsPowerOf2(constant_abs) ||
- IsPowerOf2(constant_abs + 1) ||
- IsPowerOf2(constant_abs - 1 {
+ bool small_constant = (constant = -1) (constant = 2);
+ bool end_range_constant = (constant = -kMaxInt) || (constant ==
kMaxInt);
+ int32_t constant_abs = Abs(constant);
+
+ if (!end_range_constant
+ (small_constant ||
+ (IsPowerOf2(constant_abs)) ||
+ (!can_overflow (IsPowerOf2(constant_abs + 1) ||
+ IsPowerOf2(constant_abs - 1) {
LConstantOperand* right = UseConstant(most_const);
+bool need_register = IsPowerOf2(constant_abs) !small_constant;
+left = need_register ? UseRegister(least_const)
+ : UseRegisterAtStart(least_const);
LMulConstIS* mul = new(zone()) LMulConstIS(left, right);
if (needs_environment) AssignEnvironment(mul);
return DefineAsRegister(mul);
}
}
+left = UseRegisterAtStart(least_const);
// LMulI/S can handle all cases, but it requires that a register is
// allocated for the second operand.
LInstruction* result;
Index: src/arm64/lithium-codegen-arm64.cc
diff --git a/src/arm64/lithium-codegen-arm64.cc
b/src/arm64/lithium-codegen-arm64.cc
index
d61151eec3824aac9b725a31aef1c8cc61548d93..b1dca1f13d3b95d7ce5cd0ab43c8cd51d0995dae
100644
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/src/arm64/lithium-codegen-arm64.cc
@@ -4253,6 +4253,7 @@ void LCodeGen::DoMulConstIS(LMulConstIS* instr) {
Register left =
is_smi ? ToRegister(instr-left()) : ToRegister32(instr-left()) ;
int32_t right = ToInteger32(instr-right());
+ ASSERT((right -kMaxInt) || (right kMaxInt));
bool can_overflow = instr-hydrogen()-CheckFlag(HValue::kCanOverflow);
bool bailout_on_minus_zero =
@@ -4296,20 +4297,45 @@ void LCodeGen::DoMulConstIS(LMulConstIS* instr) {
}
break;
-// All other cases cannot detect overflow, because it would probably
be no
-// faster than using the smull method in LMulI.
-// TODO(jbramley): Investigate this, and add overflow support if it
would
-// be useful.
default:
- ASSERT(!can_overflow);
-
// Multiplication by constant powers of two (and some related values)
// can be done efficiently with shifted operands.
- if (right = 0) {
-if (IsPowerOf2(right)) {
+ int32_t right_abs = Abs(right);
+
+ if (IsPowerOf2(right_abs)) {
+int right_log2 = WhichPowerOf2(right_abs);
+
+if (can_overflow) {
+ Register scratch = result;
+ ASSERT(!AreAliased(scratch, left));
+ __ Cls(scratch, left);
+ __ Cmp(scratch, right_log2);
+ DeoptimizeIf(lt, instr-environment());
+}
+
+if (right = 0) {
// result = left log2(right)
- __ Lsl(result, left, WhichPowerOf2(right));
-} else if (IsPowerOf2(right - 1)) {
+ __ Lsl(result, left, right_log2);
+} else {
+ // result = -left log2(-right)
+ if (can_overflow) {
+__ Negs(result, Operand(left, LSL, right_log2));
+DeoptimizeIf(vs, instr-environment());
+ } else {
+__ Neg(result, Operand(left, LSL, right_log2));
+ }
+}
+return;
+ }
+
+
+ // For the following cases, we could perform a
[v8-dev] Re: Reland ARM64: Add overflow checking support for multiplications by constant powers of 2. (issue 212573002)
Committed patchset #1 manually as r20267 (presubmit successful). https://codereview.chromium.org/212573002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Add overflow checking support for multiplications by constant powers of 2. (issue 210253002)
https://codereview.chromium.org/210253002/diff/1/src/arm64/lithium-arm64.cc File src/arm64/lithium-arm64.cc (right): https://codereview.chromium.org/210253002/diff/1/src/arm64/lithium-arm64.cc#newcode1914 src/arm64/lithium-arm64.cc:1914: !((constant = -1) (constant = 2)); On 2014/03/25 08:26:35, ulan wrote: Since (constant = -1) (constant = 2) used twice consider giving it a name like small_constant to improve readability. Will do before landing. https://codereview.chromium.org/210253002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Optimize RegList::IsValid() to speed up simulation in (opt)debug mode. (issue 209353010)
Committed patchset #1 manually as r20232 (presubmit successful). https://codereview.chromium.org/209353010/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Add overflow checking support for multiplications by constant powers of 2. (issue 210253002)
Committed patchset #1 manually as r20246 (presubmit successful). https://codereview.chromium.org/210253002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Revert A64: Now that we have veneers, fix a couple of branches to directly jump to their target. (issue 209333004)
I am looking at it more in details. On Monday, March 24, 2014 4:13:46 PM UTC, alexand...@arm.com wrote: From some quick profiling, it appears that the test generates so many unresolved branches (forward branches) that it puts a lot fo pressure on `std::multimapint, FarBranchInfo unresolved_branches_`. Without the patch the conditional branches are resolved locally and the unconditional branch does not require an entry in unresolved_branches_. So I think it is not a correctness issue. I could spend some time looking at solutions to improve that (maybe a custom allocator for the multimap?). On Monday, March 24, 2014 3:59:55 PM UTC, ul...@chromium.org wrote: lgtm Alexandre, any idea why this regressed performance? https://codereview.chromium.org/209333004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: block veneer pool in InstructionAccurateScope. (issue 211323003)
On 2014/03/25 15:47:28, ulan wrote: PTAL. This fixes mozilla/ecma_3/RegExp/regress-169497 that started after r20248. LGTM https://codereview.chromium.org/211323003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Add overflow checking support for multiplications by constant powers of 2. (issue 210253002)
Updated the patch to fix the failure observed in mjsunit/mul-exhaustive-part4. Checked that optdebug tests pass both with and without snapshot. https://codereview.chromium.org/210253002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] A64: Fix Register and FPRegister copy constructors. (issue 207743003)
Reviewers: jochen, ulan,
Description:
A64: Fix Register and FPRegister copy constructors.
This issue was introduced in commit
4e27429b1bc295631842cce7e8f6c404f7f77c50.
Please review this at https://codereview.chromium.org/207743003/
SVN Base: git://github.com/v8/v8.git@master
Affected files (+21, -6 lines):
M src/arm64/assembler-arm64.h
M src/arm64/stub-cache-arm64.cc
Index: src/arm64/assembler-arm64.h
diff --git a/src/arm64/assembler-arm64.h b/src/arm64/assembler-arm64.h
index
79f957b91cb744f8ea4822f3cc87dd8d7086c66c..bc0c84181da2ec4ee6f15c0e34fc8a53211ba396
100644
--- a/src/arm64/assembler-arm64.h
+++ b/src/arm64/assembler-arm64.h
@@ -115,7 +115,7 @@ struct CPURegister {
struct Register : public CPURegister {
static Register Create(unsigned code, unsigned size) {
-return CPURegister::Create(code, size, CPURegister::kRegister);
+return Register(CPURegister::Create(code, size,
CPURegister::kRegister));
}
Register() {
@@ -124,7 +124,14 @@ struct Register : public CPURegister {
reg_type = CPURegister::kNoRegister;
}
- Register(const CPURegister r) { // NOLINT(runtime/explicit)
+ explicit Register(const CPURegister r) {
+reg_code = r.reg_code;
+reg_size = r.reg_size;
+reg_type = r.reg_type;
+ASSERT(IsValidOrNone());
+ }
+
+ Register(const Register r) { // NOLINT(runtime/explicit)
reg_code = r.reg_code;
reg_size = r.reg_size;
reg_type = r.reg_type;
@@ -236,7 +243,8 @@ struct Register : public CPURegister {
struct FPRegister : public CPURegister {
static FPRegister Create(unsigned code, unsigned size) {
-return CPURegister::Create(code, size, CPURegister::kFPRegister);
+return FPRegister(
+CPURegister::Create(code, size, CPURegister::kFPRegister));
}
FPRegister() {
@@ -245,7 +253,14 @@ struct FPRegister : public CPURegister {
reg_type = CPURegister::kNoRegister;
}
- FPRegister(const CPURegister r) { // NOLINT(runtime/explicit)
+ explicit FPRegister(const CPURegister r) {
+reg_code = r.reg_code;
+reg_size = r.reg_size;
+reg_type = r.reg_type;
+ASSERT(IsValidOrNone());
+ }
+
+ FPRegister(const FPRegister r) { // NOLINT(runtime/explicit)
reg_code = r.reg_code;
reg_size = r.reg_size;
reg_type = r.reg_type;
Index: src/arm64/stub-cache-arm64.cc
diff --git a/src/arm64/stub-cache-arm64.cc b/src/arm64/stub-cache-arm64.cc
index
0711acae0b9fd7668f182a2916bc94a72ca608e1..1b2e9599368877a666a93c0a7abacd934a22cc67
100644
--- a/src/arm64/stub-cache-arm64.cc
+++ b/src/arm64/stub-cache-arm64.cc
@@ -395,7 +395,7 @@ void
StoreStubCompiler::GenerateStoreTransition(MacroAssembler* masm,
__ JumpIfSmi(value_reg, miss_label);
} else if (representation.IsDouble()) {
UseScratchRegisterScope temps(masm);
-Register temp_double = temps.AcquireD();
+DoubleRegister temp_double = temps.AcquireD();
__ SmiUntagToDouble(temp_double, value_reg, kSpeculativeUntag);
Label do_store, heap_number;
@@ -546,7 +546,7 @@ void
StoreStubCompiler::GenerateStoreField(MacroAssembler* masm,
__ JumpIfSmi(value_reg, miss_label);
} else if (representation.IsDouble()) {
UseScratchRegisterScope temps(masm);
-Register temp_double = temps.AcquireD();
+DoubleRegister temp_double = temps.AcquireD();
__ SmiUntagToDouble(temp_double, value_reg, kSpeculativeUntag);
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Revert A64: Now that we have veneers, fix a couple of branches to directly jump to their target. (issue 209333004)
From some quick profiling, it appears that the test generates so many unresolved branches (forward branches) that it puts a lot fo pressure on `std::multimapint, FarBranchInfo unresolved_branches_`. Without the patch the conditional branches are resolved locally and the unconditional branch does not require an entry in unresolved_branches_. So I think it is not a correctness issue. I could spend some time looking at solutions to improve that (maybe a custom allocator for the multimap?). On Monday, March 24, 2014 3:59:55 PM UTC, ul...@chromium.org wrote: lgtm Alexandre, any idea why this regressed performance? https://codereview.chromium.org/209333004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] A64: Optimize RegList::IsValid() to speed up simulation in (opt)debug mode. (issue 209353010)
Reviewers: ulan, jochen,
Description:
A64: Optimize RegList::IsValid() to speed up simulation in (opt)debug mode.
On my machine this improves the simulation speed noticeably.
| test | base | patched |
||---|-|
| optdebug | 15:37 | 11:07 |
| debug --quickcheck | 33:53 | 26:14 |
Please review this at https://codereview.chromium.org/209353010/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+12, -16 lines):
M src/arm64/assembler-arm64.h
Index: src/arm64/assembler-arm64.h
diff --git a/src/arm64/assembler-arm64.h b/src/arm64/assembler-arm64.h
index
e3e0277294d59c9e636790e69bb56519d7630470..8836e28547b1371a2e36c24259168c08f84fe77a
100644
--- a/src/arm64/assembler-arm64.h
+++ b/src/arm64/assembler-arm64.h
@@ -573,22 +573,18 @@ class CPURegList {
CPURegister::RegisterType type_;
bool IsValid() const {
-if ((type_ == CPURegister::kRegister) ||
-(type_ == CPURegister::kFPRegister)) {
- bool is_valid = true;
- // Try to create a CPURegister for each element in the list.
- for (int i = 0; i kRegListSizeInBits; i++) {
-if (((list_ i) 1) != 0) {
- is_valid = CPURegister::Create(i, size_, type_).IsValid();
-}
- }
- return is_valid;
-} else if (type_ == CPURegister::kNoRegister) {
- // The kNoRegister type is valid only for empty lists.
- // We can't use IsEmpty here because that asserts IsValid().
- return list_ == 0;
-} else {
- return false;
+const RegList kValidRegisters = 0x800;
+const RegList kValidFPRegisters = 0x000;
+switch (type_) {
+ case CPURegister::kRegister:
+return (list_ kValidRegisters) == list_;
+ case CPURegister::kFPRegister:
+return (list_ kValidFPRegisters) == list_;
+ case CPURegister::kNoRegister:
+return list_ == 0;
+ default:
+UNREACHABLE();
+return false;
}
}
};
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: ARM64: Fix Register and FPRegister copy constructors. (issue 207743003)
Committed patchset #1 manually as r20212 (presubmit successful). https://codereview.chromium.org/207743003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] A64: Add overflow checking support for multiplications by constant powers of 2. (issue 210253002)
Reviewers: ulan, jochen,
Description:
A64: Add overflow checking support for multiplications by constant powers
of 2.
Please review this at https://codereview.chromium.org/210253002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+47, -23 lines):
M src/arm64/lithium-arm64.cc
M src/arm64/lithium-codegen-arm64.cc
Index: src/arm64/lithium-arm64.cc
diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc
index
c6f547f6284db0d091f52f21b02f3f9c8688..f85135f1029633cb02c70a548146da5095adc832
100644
--- a/src/arm64/lithium-arm64.cc
+++ b/src/arm64/lithium-arm64.cc
@@ -1891,29 +1891,36 @@ LInstruction* LChunkBuilder::DoMul(HMul* instr) {
HValue* least_const = instr-BetterLeftOperand();
HValue* most_const = instr-BetterRightOperand();
-LOperand* left = UseRegisterAtStart(least_const);
+LOperand* left;
// LMulConstI can handle a subset of constants:
// With support for overflow detection:
//-1, 0, 1, 2
-// Without support for overflow detection:
//2^n, -(2^n)
+// Without support for overflow detection:
//2^n + 1, -(2^n - 1)
if (most_const-IsConstant()) {
int32_t constant = HConstant::cast(most_const)-Integer32Value();
- int32_t constant_abs = (constant = 0) ? constant : -constant;
-
- if (((constant = -1) (constant = 2)) ||
- (!can_overflow (IsPowerOf2(constant_abs) ||
- IsPowerOf2(constant_abs + 1) ||
- IsPowerOf2(constant_abs - 1 {
+ bool end_range_constant = (constant = -kMaxInt) || (constant ==
kMaxInt);
+ int32_t constant_abs = Abs(constant);
+
+ if (!end_range_constant
+ (((constant = -1) (constant = 2)) ||
+ (IsPowerOf2(constant_abs)) ||
+ (!can_overflow (IsPowerOf2(constant_abs + 1) ||
+ IsPowerOf2(constant_abs - 1) {
LConstantOperand* right = UseConstant(most_const);
+bool need_register = IsPowerOf2(constant_abs)
+!((constant = -1) (constant = 2));
+left = need_register ? UseRegister(least_const)
+ : UseRegisterAtStart(least_const);
LMulConstIS* mul = new(zone()) LMulConstIS(left, right);
if (needs_environment) AssignEnvironment(mul);
return DefineAsRegister(mul);
}
}
+left = UseRegisterAtStart(least_const);
// LMulI/S can handle all cases, but it requires that a register is
// allocated for the second operand.
LInstruction* result;
Index: src/arm64/lithium-codegen-arm64.cc
diff --git a/src/arm64/lithium-codegen-arm64.cc
b/src/arm64/lithium-codegen-arm64.cc
index
7fdd94fee11f1beda5688c146720f55b19b23a83..2f344e5c0e0f500ab5edcba16806eb3bd97a9452
100644
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/src/arm64/lithium-codegen-arm64.cc
@@ -4238,6 +4238,7 @@ void LCodeGen::DoMulConstIS(LMulConstIS* instr) {
Register left =
is_smi ? ToRegister(instr-left()) : ToRegister32(instr-left()) ;
int32_t right = ToInteger32(instr-right());
+ ASSERT((right -kMaxInt) || (right kMaxInt));
bool can_overflow = instr-hydrogen()-CheckFlag(HValue::kCanOverflow);
bool bailout_on_minus_zero =
@@ -4281,20 +4282,40 @@ void LCodeGen::DoMulConstIS(LMulConstIS* instr) {
}
break;
-// All other cases cannot detect overflow, because it would probably
be no
-// faster than using the smull method in LMulI.
-// TODO(jbramley): Investigate this, and add overflow support if it
would
-// be useful.
default:
- ASSERT(!can_overflow);
-
// Multiplication by constant powers of two (and some related values)
// can be done efficiently with shifted operands.
- if (right = 0) {
-if (IsPowerOf2(right)) {
+ int32_t right_abs = Abs(right);
+
+ if (IsPowerOf2(right_abs)) {
+int right_log2 = WhichPowerOf2(right_abs);
+
+if (can_overflow) {
+ Register scratch = result;
+ ASSERT(!AreAliased(scratch, left));
+ __ Cls(scratch, left);
+ __ Cmp(scratch, right_log2);
+ DeoptimizeIf(lt, instr-environment());
+}
+
+if (right = 0) {
// result = left log2(right)
- __ Lsl(result, left, WhichPowerOf2(right));
-} else if (IsPowerOf2(right - 1)) {
+ __ Lsl(result, left, right_log2);
+} else {
+ // result = -left log2(-right)
+ __ Neg(result, Operand(left, LSL, right_log2));
+}
+return;
+ }
+
+
+ // For the following cases, we could perform a conservative overflow
check
+ // with CLS as above. However the few cycles saved are likely not
worth
+ // the risk of deoptimizing more often than required.
+ ASSERT(!can_overflow);
+
+ if (right = 0) {
+if (IsPowerOf2(right - 1)) {
// result = left + left log2(right - 1)
__
[v8-dev] A64: Now that we have veneers, fix a couple of branches to directly jump to their target. (issue 207883002)
Reviewers: ulan, jochen,
Description:
A64: Now that we have veneers, fix a couple of branches to directly jump to
their target.
Please review this at https://codereview.chromium.org/207883002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+3, -12 lines):
M src/a64/regexp-macro-assembler-a64.cc
Index: src/a64/regexp-macro-assembler-a64.cc
diff --git a/src/a64/regexp-macro-assembler-a64.cc
b/src/a64/regexp-macro-assembler-a64.cc
index
0b6ade8509f1510f0216182016f961a59c0c392f..06716e1723b435139da936ee9f3258cd29f3d1a7
100644
--- a/src/a64/regexp-macro-assembler-a64.cc
+++ b/src/a64/regexp-macro-assembler-a64.cc
@@ -1483,12 +1483,7 @@ void
RegExpMacroAssemblerA64::BranchOrBacktrack(Condition condition,
if (to == NULL) {
to = backtrack_label_;
}
- // TODO(ulan): do direct jump when jump distance is known and fits in
imm19.
- Condition inverted_condition = InvertCondition(condition);
- Label no_branch;
- __ B(inverted_condition, no_branch);
- __ B(to);
- __ Bind(no_branch);
+ __ B(condition, to);
}
void RegExpMacroAssemblerA64::CompareAndBranchOrBacktrack(Register reg,
@@ -1499,15 +1494,11 @@ void
RegExpMacroAssemblerA64::CompareAndBranchOrBacktrack(Register reg,
if (to == NULL) {
to = backtrack_label_;
}
-// TODO(ulan): do direct jump when jump distance is known and fits in
imm19.
-Label no_branch;
if (condition == eq) {
- __ Cbnz(reg, no_branch);
+ __ Cbz(reg, to);
} else {
- __ Cbz(reg, no_branch);
+ __ Cbnz(reg, to);
}
-__ B(to);
-__ Bind(no_branch);
} else {
__ Cmp(reg, immediate);
BranchOrBacktrack(condition, to);
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Now that we have veneers, fix a couple of branches to directly jump to their target. (issue 207883002)
Committed patchset #1 manually as r20169 (presubmit successful). https://codereview.chromium.org/207883002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Abstract colour definitions for the Simulator tracing. (issue 203563002)
Committed patchset #1 manually as r20106 (presubmit successful). https://codereview.chromium.org/203563002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Abstract simulation of runtime calls in a separate function. (issue 203703002)
https://codereview.chromium.org/203703002/diff/1/src/a64/simulator-a64.cc
File src/a64/simulator-a64.cc (right):
https://codereview.chromium.org/203703002/diff/1/src/a64/simulator-a64.cc#newcode473
src/a64/simulator-a64.cc:473: T external_function() { return
reinterpret_castT(external_function_); }
On 2014/03/19 08:53:19, ulan wrote:
Indentation is off
Fixing before landing.
https://codereview.chromium.org/203703002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Abstract simulation of runtime calls in a separate function. (issue 203703002)
Committed patchset #1 manually as r20107 (presubmit successful). https://codereview.chromium.org/203703002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: In the Simulator, corrupt caller-saved registers after runtime printf call. (issue 203603002)
Committed patchset #1 manually as r20108 (presubmit successful). https://codereview.chromium.org/203603002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] A64: Remove Operand constructors where an implicit constructor can be used. (issue 204293004)
Reviewers: ulan, jochen, Description: A64: Remove Operand constructors where an implicit constructor can be used. Please review this at https://codereview.chromium.org/204293004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+176, -181 lines): M src/a64/builtins-a64.cc M src/a64/code-stubs-a64.cc M src/a64/codegen-a64.cc M src/a64/debug-a64.cc M src/a64/deoptimizer-a64.cc M src/a64/full-codegen-a64.cc M src/a64/ic-a64.cc M src/a64/lithium-codegen-a64.cc M src/a64/lithium-gap-resolver-a64.cc M src/a64/macro-assembler-a64.cc M src/a64/regexp-macro-assembler-a64.cc M src/a64/stub-cache-a64.cc -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] A64: Minor cleaning in StoreStubCompiler::GenerateStoreField. (issue 206183005)
Reviewers: ulan, jochen,
Description:
A64: Minor cleaning in StoreStubCompiler::GenerateStoreField.
Abstract a register to simplify code generation.
Please review this at https://codereview.chromium.org/206183005/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+5, -10 lines):
M src/a64/stub-cache-a64.cc
Index: src/a64/stub-cache-a64.cc
diff --git a/src/a64/stub-cache-a64.cc b/src/a64/stub-cache-a64.cc
index
291da65d44f4113e2c9ff9f226a766571238237a..0d1ecd2d4e83caf51aa6ac9a15b1be3b2fd8750c
100644
--- a/src/a64/stub-cache-a64.cc
+++ b/src/a64/stub-cache-a64.cc
@@ -461,16 +461,15 @@ void
StoreStubCompiler::GenerateStoreTransition(MacroAssembler* masm,
// TODO(verwaest): Share this code as a code stub.
SmiCheck smi_check = representation.IsTagged()
? INLINE_SMI_CHECK : OMIT_SMI_CHECK;
+ Register prop_reg =
+(FLAG_track_double_fields representation.IsDouble()) ? storage_reg
+: value_reg;
if (index 0) {
// Set the property straight into the object.
int offset = object-map()-instance_size() + (index * kPointerSize);
// TODO(jbramley): This construct appears in several places in this
// function. Try to clean it up, perhaps using a result_reg.
-if (representation.IsDouble()) {
- __ Str(storage_reg, FieldMemOperand(receiver_reg, offset));
-} else {
- __ Str(value_reg, FieldMemOperand(receiver_reg, offset));
-}
+__ Str(prop_reg, FieldMemOperand(receiver_reg, offset));
if (!representation.IsSmi()) {
// Update the write barrier for the array address.
@@ -492,11 +491,7 @@ void
StoreStubCompiler::GenerateStoreTransition(MacroAssembler* masm,
// Get the properties array
__ Ldr(scratch1,
FieldMemOperand(receiver_reg, JSObject::kPropertiesOffset));
-if (representation.IsDouble()) {
- __ Str(storage_reg, FieldMemOperand(scratch1, offset));
-} else {
- __ Str(value_reg, FieldMemOperand(scratch1, offset));
-}
+__ Str(prop_reg, FieldMemOperand(scratch1, offset));
if (!representation.IsSmi()) {
// Update the write barrier for the array address.
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Minor cleaning in StoreStubCompiler::GenerateStoreField. (issue 206183005)
On 2014/03/20 10:39:36, ulan wrote: https://codereview.chromium.org/206183005/diff/1/src/a64/stub-cache-a64.cc File src/a64/stub-cache-a64.cc (right): https://codereview.chromium.org/206183005/diff/1/src/a64/stub-cache-a64.cc#newcode465 src/a64/stub-cache-a64.cc:465: (FLAG_track_double_fields representation.IsDouble()) ? storage_reg why FLAG_track_double_fields? Oh sorry, this was part of the test, but was changed very recently and I didn't see it while rebasing. Updating the patch. https://codereview.chromium.org/206183005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Minor cleaning in StoreStubCompiler::GenerateStoreField. (issue 206183005)
Updated... https://codereview.chromium.org/206183005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Minor cleaning in StoreStubCompiler::GenerateStoreField. (issue 206183005)
Committed patchset #2 manually as r20116 (presubmit successful). https://codereview.chromium.org/206183005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] A64: Restore the stack limit protection to 1KB. (issue 206393002)
Reviewers: ulan, jochen,
Description:
A64: Restore the stack limit protection to 1KB.
Please review this at https://codereview.chromium.org/206393002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+1, -7 lines):
M src/a64/simulator-a64.h
M src/a64/simulator-a64.cc
Index: src/a64/simulator-a64.cc
diff --git a/src/a64/simulator-a64.cc b/src/a64/simulator-a64.cc
index
d414bf5426e3c83ba509effb5c1f8d342d17d0d8..a401c62d2ebdf6d287b3c265a8c5ef56d92a171f
100644
--- a/src/a64/simulator-a64.cc
+++ b/src/a64/simulator-a64.cc
@@ -359,12 +359,7 @@ uintptr_t Simulator::PopAddress() {
uintptr_t Simulator::StackLimit() const {
// Leave a safety margin of 1024 bytes to prevent overrunning the stack
when
// pushing values.
- // TODO(all): Increase the stack limit protection.
-
- // The margin was decreased to 256 bytes, because we are intensively
using
- // the stack. The stack usage should decrease when our code improves.
Then
- // we can set it to 1024 again.
- return reinterpret_castuintptr_t(stack_limit_) + 256;
+ return reinterpret_castuintptr_t(stack_limit_) + 1024;
}
Index: src/a64/simulator-a64.h
diff --git a/src/a64/simulator-a64.h b/src/a64/simulator-a64.h
index
0d973e835e14bbb8f58a963c25909c7683d63c2a..1ef891a151ecee375870f5e6073f01cc93d1d773
100644
--- a/src/a64/simulator-a64.h
+++ b/src/a64/simulator-a64.h
@@ -833,7 +833,6 @@ class Simulator : public DecoderVisitor {
static const intptr_t stack_protection_size_ = KB;
intptr_t stack_size_;
byte* stack_limit_;
- // TODO(aleram): protect the stack.
DecoderDispatchingDecoderVisitor* decoder_;
DecoderDispatchingDecoderVisitor* disassembler_decoder_;
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Restore the stack limit protection to 1KB. (issue 206393002)
Committed patchset #1 manually as r20131 (presubmit successful). https://codereview.chromium.org/206393002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Handle a few TODOs. (issue 196473021)
I will split the next TODOs into separate patches. We did so to avoid the review cycle overhead as there are quite a few. https://codereview.chromium.org/196473021/diff/1/src/a64/macro-assembler-a64.cc File src/a64/macro-assembler-a64.cc (right): https://codereview.chromium.org/196473021/diff/1/src/a64/macro-assembler-a64.cc#newcode2467 src/a64/macro-assembler-a64.cc:2467: Cbz(bulk_length, short_copy); On 2014/03/18 12:51:14, ulan wrote: You probably wanted to use flags set by Bics. Yes. Fixed that. https://codereview.chromium.org/196473021/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Handle a few TODOs. (issue 196473021)
https://codereview.chromium.org/196473021/diff/1/src/a64/macro-assembler-a64.cc#newcode2467 src/a64/macro-assembler-a64.cc:2467: Cbz(bulk_length, short_copy); On 2014/03/18 12:51:14, ulan wrote: You probably wanted to use flags set by Bics. Yes. Fixed that. Actually, following our current rule of thumb, we prefer not setting the flags with BIC, and using CBZ. https://codereview.chromium.org/196473021/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: A64: Handle a few TODOs. (issue 196473021)
Committed patchset #1 manually as r20034 (presubmit successful). https://codereview.chromium.org/196473021/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] A64: Force emission of the veneer pool emission when required. (issue 203443003)
Reviewers: ulan,
Description:
A64: Force emission of the veneer pool emission when required.
Please review this at https://codereview.chromium.org/203443003/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+20, -12 lines):
M src/a64/assembler-a64-inl.h
M src/a64/assembler-a64.h
M src/a64/assembler-a64.cc
M src/a64/full-codegen-a64.cc
M src/a64/lithium-codegen-a64.cc
M src/a64/macro-assembler-a64-inl.h
Index: src/a64/assembler-a64-inl.h
diff --git a/src/a64/assembler-a64-inl.h b/src/a64/assembler-a64-inl.h
index
2a930b2e7dcef3b76413f0290b232f03d650d616..feb6bbb467fbc14c875afbe382fd33256f61da85
100644
--- a/src/a64/assembler-a64-inl.h
+++ b/src/a64/assembler-a64-inl.h
@@ -1207,7 +1207,7 @@ inline void Assembler::CheckBuffer() {
GrowBuffer();
}
if (pc_offset() = next_veneer_pool_check_) {
-CheckVeneerPool(true);
+CheckVeneerPool(false, true);
}
if (pc_offset() = next_constant_pool_check_) {
CheckConstPool(false, true);
Index: src/a64/assembler-a64.cc
diff --git a/src/a64/assembler-a64.cc b/src/a64/assembler-a64.cc
index
51542b27da0d1cdc5230cc5a6d3f8c149644cdfe..2bffc587f28e8069457c3c08a1d0751beaa66217
100644
--- a/src/a64/assembler-a64.cc
+++ b/src/a64/assembler-a64.cc
@@ -2548,7 +2548,7 @@ void Assembler::CheckConstPool(bool force_emit, bool
require_jump) {
// Emit veneers for branches that would go out of range during emission
of the
// constant pool.
- CheckVeneerPool(require_jump, kVeneerDistanceMargin + pool_size);
+ CheckVeneerPool(false, require_jump, kVeneerDistanceMargin + pool_size);
Label size_check;
bind(size_check);
@@ -2641,7 +2641,7 @@ void Assembler::RecordVeneerPool(int location_offset,
int size) {
}
-void Assembler::EmitVeneers(bool need_protection, int margin) {
+void Assembler::EmitVeneers(bool force_emit, bool need_protection, int
margin) {
BlockPoolsScope scope(this);
RecordComment([ Veneers);
@@ -2667,7 +2667,7 @@ void Assembler::EmitVeneers(bool need_protection, int
margin) {
it = unresolved_branches_.begin();
while (it != unresolved_branches_.end()) {
-if (ShouldEmitVeneer(it-first, margin)) {
+if (force_emit || ShouldEmitVeneer(it-first, margin)) {
Instruction* branch = InstructionAt(it-second.pc_offset_);
Label* label = it-second.label_;
@@ -2710,7 +2710,7 @@ void Assembler::EmitVeneers(bool need_protection, int
margin) {
}
-void Assembler::CheckVeneerPool(bool require_jump,
+void Assembler::CheckVeneerPool(bool force_emit, bool require_jump,
int margin) {
// There is nothing to do if there are no pending veneer pool entries.
if (unresolved_branches_.empty()) {
@@ -2724,6 +2724,7 @@ void Assembler::CheckVeneerPool(bool require_jump,
// emission, such sequences are protected by calls to BlockVeneerPoolFor
and
// BlockVeneerPoolScope.
if (is_veneer_pool_blocked()) {
+ASSERT(!force_emit);
return;
}
@@ -2731,8 +2732,8 @@ void Assembler::CheckVeneerPool(bool require_jump,
// Prefer emitting veneers protected by an existing instruction.
margin *= kVeneerNoProtectionFactor;
}
- if (ShouldEmitVeneers(margin)) {
-EmitVeneers(require_jump, margin);
+ if (force_emit || ShouldEmitVeneers(margin)) {
+EmitVeneers(force_emit, require_jump, margin);
} else {
next_veneer_pool_check_ =
unresolved_branches_first_limit() - kVeneerDistanceCheckMargin;
Index: src/a64/assembler-a64.h
diff --git a/src/a64/assembler-a64.h b/src/a64/assembler-a64.h
index
31d7f17e071612e503c8fbfba88fdf27f1a5b018..47f9694e054c3b7ed5bef92205fa7f87e41b4a66
100644
--- a/src/a64/assembler-a64.h
+++ b/src/a64/assembler-a64.h
@@ -1808,10 +1808,13 @@ class Assembler : public AssemblerBase {
// Emits veneers for branches that are approaching their maximum range.
// If need_protection is true, the veneers are protected by a branch
jumping
// over the code.
- void EmitVeneers(bool need_protection, int margin =
kVeneerDistanceMargin);
+ void EmitVeneers(bool force_emit, bool need_protection,
+ int margin = kVeneerDistanceMargin);
void EmitVeneersGuard() { EmitPoolGuard(); }
// Checks whether veneers need to be emitted at this point.
- void CheckVeneerPool(bool require_jump, int margin =
kVeneerDistanceMargin);
+ // If force_emit is set, a veneer is generated for *all* unresolved
branches.
+ void CheckVeneerPool(bool force_emit, bool require_jump,
+ int margin = kVeneerDistanceMargin);
class BlockPoolsScope {
Index: src/a64/full-codegen-a64.cc
diff --git a/src/a64/full-codegen-a64.cc b/src/a64/full-codegen-a64.cc
index
dcab182e6770f3b6999302cdd0d9e250c6a559f6..ed6153c23a843d0a1fdfab5e6d65878909a4f9b5
100644
--- a/src/a64/full-codegen-a64.cc
+++ b/src/a64/full-codegen-a64.cc
@@ -314,8 +314,9 @@ void FullCodeGenerator::Generate() {
}
EmitReturnSequence();
- // Force emit the
