How to remove CheckMaps instructions in an optimization phase

[Gabriel Southern]

I'm experimenting with removing deoptimization checks and I have a question about how to remove hydrogen instructions.

I'm looking at a benchmark where the CheckMaps deoptimization checks are never triggered and I'm trying to remove them.  I know this is not safe in the general case, but when I traced the deoptimizations for this benchmark there were not any that were triggered because of CheckMaps.

I've tried to follow the HDeadCodeEliminationPhase as a guide because what I want to do is delete instructions that match a certain criteria, so I thought that pass might be a good example.  The main loop in my pass is:

  for (int i = 0; i < graph()->blocks()->length(); ++i) {   

    HBasicBlock* block = graph()->blocks()->at(i);         

    for (HInstructionIterator it(block); !it.Done(); it.Advance()) {   

      HInstruction* instr = it.Current();  

      if (instr->opcode() == HValue::kCheckMaps) {     

        instr->DeleteAndReplaceWith(NULL);

      }                                                

    } 

  }  

When I run this and just print the list of instructions that will be removed the list looks okay.  However if I actually delete the instruction I get a runtime error as follows:

#

# Fatal error in ../src/objects.cc, line 10380

# unreachable code

#

==== C stack trace ===============================

 1: V8_Fatal

 2: v8::internal::Code::FindAndReplace(v8::internal::Code::FindAndReplacePattern const&)

 3: v8::internal::CodeStub::GetCodeCopy(v8::internal::Code::FindAndReplacePattern const&)

 4: v8::internal::PropertyICCompiler::ComputeCompareNil(v8::internal::Handle<v8::internal::Map>, v8::internal::CompareNilICStub*)

 5: v8::internal::CompareNilIC::CompareNil(v8::internal::Handle<v8::internal::Object>)

 6: ??

 7: v8::internal::CompareNilIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*)

 8: ??

Segmentation fault (core dumped)

I'm wondering if anyone has suggestions for what I can look at it understand what's going on and debug the problem.  Obviously the specific thing I'm trying to do of removing CheckMaps is not something that should work in general.  But I think it should be possible to remove a hydrogen instruction during the optimization phase.  I've tried to pattern my attempt off of the existing code, but obviously I'm missing something.  If anyone has suggestions about what I should try that is appreciated.

Thanks,

Gabriel

[Jakob Kummerow]

Removing check instructions is so utterly wrong and dangerous that I can't bring myself to even try to help you. Just don't do it!

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to the Google Groups "v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Gabriel Southern]

Perhaps it would help if I explained my motivation.  I'm trying to evaluate the overhead of conditional deoptimization checks.  One way is by running workloads that have the checks in their normal configuration and measuring the runtime.  Then removing checks that were never triggered and rerunning the workload and comparing the runtime.  Obviously I understand this is not safe in the general case.

For some workloads I was able to remove the call to DeoptimizeIf in LCodeGen::DoCheckMaps and the benchmark still ran correctly.  But if I removed the call to CompareMap(reg,map) I would get an error about unreachable code similar to what I posted earlier when I remove the CompareMaps hydrogen instructions.

Aside from that I think that I want to be able to choose when to remove checks at the hydrogen instruction level because later I will want to pick which functions to remove the checks from.  I would profile a benchmark first and see which functions have conditional deopts that are never triggered and then remove the deopts from those functions.

Again this is all part of a performance evaluation study, not something to be used for production code.  I hope this makes sense, but if you think there's something I'm overlooking for why this won't work I'd be interested to know why.  From looking at the assembly code sequences that are generated I think this should be okay, but there's also obviously something I'm missing that is leading to the unreachable code error that I've seen.

Thanks,

-Gabriel

[Vyacheslav Egorov]

Hydrogen is also used to generate stubs and there CheckMaps "deopt"  has a different semantics from a normal deopt - and it is not reported in --trace-deopt (which reports all normal JS function deopts). You have to accommodate for that in your experiments.

I however think that your experiment does not provide any actionable data. Knowing that checks introduce X% overhead is kinda useless - unless you also know an algorithm to eliminate all of them and retain correctness. 

Vyacheslav Egorov

[Gabriel Southern]

On Wed, Nov 5, 2014 at 9:03 AM, Vyacheslav Egorov <veg...@chromium.org> wrote:

Hydrogen is also used to generate stubs and there CheckMaps "deopt"  has a different semantics from a normal deopt - and it is not reported in --trace-deopt (which reports all normal JS function deopts). You have to accommodate for that in your experiments.

Thanks for this comment.  I have noticed that there is a --trace_stub_failures that traces deoptimizations for stubs in addition to the --trace_deopt flag.  I admit that I don't have a very good understanding of what the semantic differences are for stubs is, but that is something I'm looking at and trying to understand the code.

 

I however think that your experiment does not provide any actionable data. Knowing that checks introduce X% overhead is kinda useless - unless you also know an algorithm to eliminate all of them and retain correctness. 

There are some research proposals for hardware support to reduce the cost of type checks.  For instance the paper Checked Load: Architectural Support for JavaScript Type-Checking on Mobile Processors from HPCA 2011 (https://homes.cs.washington.edu/~luisceze/publications/anderson-hpca2011.pdf).

I think that type of idea could be adapted to reduce the overhead of deoptimization checks.  I don't know if the idea is worthwhile or not, but I think getting some estimate of the % overhead that deoptimization checks introduce could be useful for analyzing whether hardware support to reduce this overhead could be useful.  So evaluating workloads with the deoptimization checks removed could be a form of limit study to see what the maximum potential benefit of hardware support to reduce their cost is.

-Gabriel

 

You received this message because you are subscribed to a topic in the Google Groups "v8-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/v8-users/EY3cvrOw-Us/unsubscribe.
To unsubscribe from this group and all its topics, send an email to v8-users+u...@googlegroups.com.

[Jakob Kummerow]

Don't touch stubs, they're not what you're interested in.

Regarding your original question, try replacing an HCheckMaps instruction with its value() rather than NULL.

[Gabriel Southern]

Thanks for the information about the stubs and using value() rather than NULL.

Unfortunately I'm still have a problem where v8 crashes when I my pass runs.  To help debug the problem I tried printing out the instruction in my pass (without removing them) and I was still getting a crash that seemed to be related to an IsDereferenceAllowed() check failing when I tried to print one of the instructions.  I was using a technique similar to what the HDeadCodeEliminationPhase pass does.  Since my pass seemed very similar to what the DCE pass does I tried running with --trace_dead_code_elimination flag and I noticed that I'm getting similar results. 

I was able to reproduce the problem in v8 mainline without any of the changes I'm experimenting with so I've filed a bug:

https://code.google.com/p/v8/issues/detail?id=3679

Basically v8 crashes and dumps the following stack trace.

#

# Fatal error in .././src/handles-inl.h, line 43

# CHECK(!v8::internal::FLAG_enable_slow_asserts || (IsDereferenceAllowed(INCLUDE_DEFERRED_CHECK))) failed

#

==== C stack trace ===============================

 1: V8_Fatal

 2: v8::internal::Handle<v8::internal::Map>::operator*() const

 3: v8::internal::HConstant::PrintDataTo(std::ostream&) const

 4: v8::internal::HInstruction::PrintTo(std::ostream&) const

 5: v8::internal::operator<<(std::ostream&, v8::internal::HValue const&)

 6: v8::internal::HDeadCodeEliminationPhase::PrintLive(v8::internal::HValue*, v8::internal::HValue*)

 7: v8::internal::HDeadCodeEliminationPhase::MarkLive(v8::internal::HValue*, v8::internal::ZoneList<v8::internal::HValue*>*)

 8: v8::internal::HDeadCodeEliminationPhase::MarkLiveInstructions()

 9: v8::internal::HDeadCodeEliminationPhase::Run()

10: void v8::internal::HGraph::Run<v8::internal::HDeadCodeEliminationPhase>()

11: v8::internal::HGraph::Optimize(v8::internal::BailoutReason*)

12: ??

13: ??

14: v8::internal::FastCloneShallowObjectStub::GenerateCode()

15: v8::internal::CodeStub::GetCode()

16: v8::internal::MacroAssembler::CallStub(v8::internal::CodeStub*, v8::internal::TypeFeedbackId)

17: v8::internal::FullCodeGenerator::VisitObjectLiteral(v8::internal::ObjectLiteral*)

18: v8::internal::ObjectLiteral::Accept(v8::internal::AstVisitor*)

19: v8::internal::FullCodeGenerator::Visit(v8::internal::AstNode*)

20: v8::internal::FullCodeGenerator::VisitForAccumulatorValue(v8::internal::Expression*)

21: v8::internal::FullCodeGenerator::VisitAssignment(v8::internal::Assignment*)

22: v8::internal::Assignment::Accept(v8::internal::AstVisitor*)

23: v8::internal::FullCodeGenerator::Visit(v8::internal::AstNode*)

24: v8::internal::FullCodeGenerator::VisitForEffect(v8::internal::Expression*)

25: v8::internal::FullCodeGenerator::VisitExpressionStatement(v8::internal::ExpressionStatement*)

26: v8::internal::ExpressionStatement::Accept(v8::internal::AstVisitor*)

27: v8::internal::FullCodeGenerator::Visit(v8::internal::AstNode*)

28: v8::internal::AstVisitor::VisitStatements(v8::internal::ZoneList<v8::internal::Statement*>*)

29: v8::internal::FullCodeGenerator::VisitBlock(v8::internal::Block*)

30: v8::internal::Block::Accept(v8::internal::AstVisitor*)

31: v8::internal::FullCodeGenerator::Visit(v8::internal::AstNode*)

32: v8::internal::FullCodeGenerator::VisitIfStatement(v8::internal::IfStatement*)

33: v8::internal::IfStatement::Accept(v8::internal::AstVisitor*)

34: v8::internal::FullCodeGenerator::Visit(v8::internal::AstNode*)

35: v8::internal::AstVisitor::VisitStatements(v8::internal::ZoneList<v8::internal::Statement*>*)

36: v8::internal::FullCodeGenerator::VisitSwitchStatement(v8::internal::SwitchStatement*)

37: v8::internal::SwitchStatement::Accept(v8::internal::AstVisitor*)

38: v8::internal::FullCodeGenerator::Visit(v8::internal::AstNode*)

39: v8::internal::AstVisitor::VisitStatements(v8::internal::ZoneList<v8::internal::Statement*>*)

40: v8::internal::FullCodeGenerator::Generate()

41: v8::internal::FullCodeGenerator::MakeCode(v8::internal::CompilationInfo*)

42: ??

43: ??

44: v8::internal::Compiler::GetLazyCode(v8::internal::Handle<v8::internal::JSFunction>)

45: ??

46: v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*)

47: ??

Illegal instruction (core dumped)

If anyone is able to help explain what's happening here I think that might help me with the problem I'm having the pass that I'm trying to implement.

Thanks,

-Gabriel