<div dir="ltr">Hi Ryan,<div><br></div><div> with the optimzation I just did to block activation, which is only to unforward the receiver in block activation if the method refers to inst vars, there's the potential that an implicit receiver send in a block could hit a forwarded receiver. So this is a reminder that perhaps the block scanning needs to trigger unforwarding if there's an implicit receiver send. But we should have tests before I hack a fix. Also inlineLookupInNSMethodCacheSel:... isn't in the package (1375).</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 22, 2015 at 10:08 PM, <span dir="ltr"><<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
Ryan Macnak uploaded a new version of VMMaker to project VM Maker:<br>
<a href="http://source.squeak.org/VMMaker/VMMaker.oscog-rmacnak.1375.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/VMMaker/VMMaker.oscog-rmacnak.1375.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: VMMaker.oscog-rmacnak.1375<br>
Author: rmacnak<br>
Time: 22 June 2015, 10:07:47.302 pm<br>
UUID: 2834a848-0c6d-4c29-bd1e-a43ad70450d9<br>
Ancestors: VMMaker.oscog-cb.1374<br>
<br>
Do proper lookups for implicit receiver and outer send misses from cogged code.<br>
<br>
=============== Diff against VMMaker.oscog-cb.1374 ===============<br>
<br>
Item was changed:<br>
----- Method: CoInterpreter>>ceImplicitReceiverSend:receiver: (in category 'trampolines') -----<br>
ceImplicitReceiverSend: cacheAddress receiver: methodReceiver<br>
"An implicit receiver send cache missed."<br>
+ | nsSendCache methodReceiverClassTag cogMethod errSelIdx |<br>
- | nsSendCache methodMixin numArgs selector implicitReceiver cogMethod irClassTag mrClassTag errSelIdx |<br>
<api><br>
<option: #NewspeakVM><br>
<inline: false><br>
<var: #nsSendCache type: #'NSSendCache *'><br>
<var: #cogMethod type: #'CogMethod *'><br>
<br>
cogit assertCStackWellAligned.<br>
self assert: (objectMemory addressCouldBeOop: methodReceiver).<br>
+ self deny: (objectMemory isOopForwarded: methodReceiver).<br>
<br>
nsSendCache := self cCoerceSimple: cacheAddress to: #'NSSendCache *'.<br>
+ messageSelector := nsSendCache selector.<br>
+ argumentCount := nsSendCache numArgs.<br>
+ method := (self mframeHomeMethod: framePointer) methodObject.<br>
- selector := nsSendCache selector.<br>
- numArgs := nsSendCache numArgs.<br>
- methodMixin := self mMethodClass.<br>
<br>
+ self assert: (self stackValue: argumentCount + 1 "ret addr") = methodReceiver.<br>
- implicitReceiver := self<br>
- implicitReceiverFor: methodReceiver<br>
- mixin: methodMixin<br>
- implementing: selector.<br>
<br>
+ methodReceiverClassTag := objectMemory fetchClassTagOf: methodReceiver.<br>
- self assert: (self stackValue: numArgs + 1 "ret val") = methodReceiver.<br>
- self stackValue: numArgs + 1 "ret val " put: implicitReceiver.<br>
- "Replace the methodReceiver on the stack with the implicitReceiver. When the cache has<br>
- a hit, we don't care that the value on the stack is wrong because the compiled callee will<br>
- use the value in ReceiverResultReg to build its frame. But the interpreter will use<br>
- stack(numArgs)."<br>
<br>
+ (self<br>
+ inlineLookupInNSMethodCacheSel: messageSelector<br>
+ classTag: methodReceiverClassTag<br>
+ method: method<br>
+ lookupRule: LookupRuleImplicit)<br>
+ ifTrue:<br>
+ [localAbsentReceiverOrZero = 0<br>
+ ifTrue: [localAbsentReceiver := methodReceiver]<br>
+ ifFalse: [localAbsentReceiver := localAbsentReceiverOrZero].<br>
+ "check for coggability because method is in the cache"<br>
+ self ifAppropriateCompileToNativeCode: newMethod selector: messageSelector]<br>
+ ifFalse:<br>
+ [self deny: (objectMemory isOopForwarded: messageSelector).<br>
+ self deny: (objectMemory isForwardedClassTag: methodReceiverClassTag).<br>
+ lkupClassTag := methodReceiverClassTag.<br>
+ errSelIdx := self lookupImplicitReceiverSendNoMNU: methodReceiver.<br>
+ errSelIdx ~= 0 ifTrue:<br>
+ [self assert: errSelIdx == SelectorDoesNotUnderstand.<br>
+ self handleMNU: errSelIdx<br>
+ InMachineCodeTo: methodReceiver<br>
+ classForMessage: (objectMemory classForClassTag: methodReceiverClassTag).<br>
+ self unreachable].<br>
+ self addNewMethodToNSCache: LookupRuleImplicit].<br>
- mrClassTag := objectMemory fetchClassTagOf: methodReceiver.<br>
- irClassTag := objectMemory fetchClassTagOf: implicitReceiver.<br>
- argumentCount := numArgs.<br>
<br>
+ "Fix stacked receiver."<br>
+ self stackValue: argumentCount + 1 "ret addr" put: localAbsentReceiver.<br>
- (self lookupInMethodCacheSel: selector classTag: irClassTag)<br>
- ifTrue: ["check for coggability because method is in the cache"<br>
- self ifAppropriateCompileToNativeCode: newMethod selector: selector]<br>
- ifFalse: [<br>
- (objectMemory isOopForwarded: selector) ifTrue:<br>
- [self error: 'Selector should have fixed by mapObjectReferencesInMachineCodeForBecome'].<br>
- (objectMemory isForwardedClassTag: irClassTag) ifTrue:<br>
- [self error: 'Implicit receiver lookup should have followed fowarded objects'].<br>
- messageSelector := selector.<br>
- (errSelIdx := self lookupMethodNoMNUEtcInClass: (objectMemory classForClassTag: irClassTag)) ~= 0<br>
- ifTrue: [[self handleMNU: errSelIdx InMachineCodeTo: implicitReceiver classForMessage: (objectMemory classForClassTag: irClassTag).<br>
- self error: 'UNREACHABLE3']]].<br>
<br>
(self maybeMethodHasCogMethod: newMethod)<br>
ifTrue: [<br>
cogMethod := self cogMethodOf: newMethod.<br>
cogMethod selector = objectMemory nilObject<br>
+ ifTrue: [cogit setSelectorOf: cogMethod to: messageSelector]<br>
- ifTrue: [cogit setSelectorOf: cogMethod to: selector]<br>
ifFalse: ["Deal with anonymous accessors, e.g. in Newspeak.<br>
The cogMethod may not have the<br>
correct selector. If not, try and compile a new method<br>
with the correct selector."<br>
+ cogMethod selector ~= messageSelector ifTrue: [<br>
+ (cogit cog: newMethod selector: messageSelector)<br>
- cogMethod selector ~= selector ifTrue: [<br>
- (cogit cog: newMethod selector: selector)<br>
ifNotNil: [:newCogMethod | cogMethod := newCogMethod]]].<br>
+ cogMethod selector = messageSelector<br>
- cogMethod selector = selector<br>
ifTrue:<br>
[cogit<br>
linkNSSendCache: nsSendCache<br>
+ classTag: methodReceiverClassTag<br>
+ enclosingObject: localAbsentReceiverOrZero<br>
- classTag: mrClassTag<br>
- enclosingObject: (implicitReceiver = methodReceiver<br>
- ifTrue: [0] ifFalse: [implicitReceiver])<br>
target: cogMethod<br>
caller: self mframeHomeMethodExport]<br>
ifFalse: ["Out of code memory. Fall through to interpret."].<br>
instructionPointer := self popStack.<br>
self executeNewMethod.<br>
+ self unreachable].<br>
- self error: 'UNREACHABLE 1'].<br>
instructionPointer := self popStack.<br>
self interpretMethodFromMachineCode.<br>
+ self unreachable.!<br>
- self error: 'UNREACHABLE 2'.<br>
- ^nil!<br>
<br>
Item was changed:<br>
----- Method: CoInterpreter>>ceOuterSend:receiver: (in category 'trampolines') -----<br>
ceOuterSend: cacheAddress receiver: methodReceiver<br>
"An outer send cache missed."<br>
+ | nsSendCache depth methodReceiverClassTag cogMethod errSelIdx |<br>
- | nsSendCache methodMixin numArgs selector depth enclosingObject cogMethod eoClassTag mrClassTag errSelIdx |<br>
<api><br>
<option: #NewspeakVM><br>
<inline: false><br>
<var: #nsSendCache type: #'NSSendCache *'><br>
<var: #cogMethod type: #'CogMethod *'><br>
<br>
cogit assertCStackWellAligned.<br>
self assert: (objectMemory addressCouldBeOop: methodReceiver).<br>
+ self deny: (objectMemory isOopForwarded: methodReceiver).<br>
<br>
nsSendCache := self cCoerceSimple: cacheAddress to: #'NSSendCache *'.<br>
+ messageSelector := nsSendCache selector.<br>
+ argumentCount := nsSendCache numArgs.<br>
- selector := nsSendCache selector.<br>
- numArgs := nsSendCache numArgs.<br>
depth := nsSendCache depth.<br>
+ method := (self mframeHomeMethod: framePointer) methodObject.<br>
- methodMixin := self mMethodClass.<br>
<br>
+ self assert: (self stackValue: argumentCount + 1 "ret addr") = methodReceiver.<br>
- enclosingObject := self<br>
- enclosingObjectAt: depth<br>
- withObject: methodReceiver<br>
- withMixin: methodMixin.<br>
<br>
+ methodReceiverClassTag := objectMemory fetchClassTagOf: methodReceiver.<br>
- self assert: (self stackValue: numArgs + 1 "ret val") = methodReceiver.<br>
- self stackValue: numArgs + 1 "ret val " put: enclosingObject.<br>
- "Replace the methodReceiver on the stack with the enclosingObject. When the cache has<br>
- a hit, we don't care that the value on the stack is wrong because the compiled callee will<br>
- use the value in ReceiverResultReg to build its frame. But the interpreter will use<br>
- stack(numArgs)."<br>
<br>
+ (self<br>
+ inlineLookupInNSMethodCacheSel: messageSelector<br>
+ classTag: methodReceiverClassTag<br>
+ method: method<br>
+ lookupRule: depth)<br>
+ ifTrue:<br>
+ [localAbsentReceiverOrZero = 0<br>
+ ifTrue: [localAbsentReceiver := methodReceiver]<br>
+ ifFalse: [localAbsentReceiver := localAbsentReceiverOrZero].<br>
+ "check for coggability because method is in the cache"<br>
+ self ifAppropriateCompileToNativeCode: newMethod selector: messageSelector]<br>
+ ifFalse:<br>
+ [self deny: (objectMemory isOopForwarded: messageSelector).<br>
+ self deny: (objectMemory isForwardedClassTag: methodReceiverClassTag).<br>
+ lkupClassTag := methodReceiverClassTag.<br>
+ errSelIdx := self lookupOuterSendNoMNU: methodReceiver depth: depth.<br>
+ errSelIdx ~= 0 ifTrue:<br>
+ [self assert: errSelIdx == SelectorDoesNotUnderstand.<br>
+ self handleMNU: errSelIdx<br>
+ InMachineCodeTo: methodReceiver<br>
+ classForMessage: (objectMemory classForClassTag: methodReceiverClassTag).<br>
+ self unreachable].<br>
+ self addNewMethodToNSCache: depth].<br>
- mrClassTag := objectMemory fetchClassTagOf: methodReceiver.<br>
- eoClassTag := objectMemory fetchClassTagOf: enclosingObject.<br>
- argumentCount := numArgs.<br>
<br>
+ "Fix stacked receiver."<br>
+ self stackValue: argumentCount + 1 "ret addr" put: localAbsentReceiver.<br>
- (self lookupInMethodCacheSel: selector classTag: eoClassTag)<br>
- ifTrue: ["check for coggability because method is in the cache"<br>
- self ifAppropriateCompileToNativeCode: newMethod selector: selector]<br>
- ifFalse: [<br>
- (objectMemory isOopForwarded: selector) ifTrue:<br>
- [self error: 'Selector should have fixed by mapObjectReferencesInMachineCodeForBecome'].<br>
- (objectMemory isForwardedClassTag: eoClassTag) ifTrue:<br>
- [self error: 'Implicit receiver lookup should have followed fowarded objects'].<br>
- messageSelector := selector.<br>
- (errSelIdx := self lookupMethodNoMNUEtcInClass: (objectMemory classForClassTag: eoClassTag)) ~= 0<br>
- ifTrue: [[self handleMNU: errSelIdx InMachineCodeTo: enclosingObject classForMessage: (objectMemory classForClassTag: eoClassTag).<br>
- self error: 'UNREACHABLE3']]].<br>
<br>
(self maybeMethodHasCogMethod: newMethod)<br>
ifTrue: [<br>
cogMethod := self cogMethodOf: newMethod.<br>
cogMethod selector = objectMemory nilObject<br>
+ ifTrue: [cogit setSelectorOf: cogMethod to: messageSelector]<br>
- ifTrue: [cogit setSelectorOf: cogMethod to: selector]<br>
ifFalse: ["Deal with anonymous accessors, e.g. in Newspeak.<br>
The cogMethod may not have the<br>
correct selector. If not, try and compile a new method<br>
with the correct selector."<br>
+ cogMethod selector ~= messageSelector ifTrue: [<br>
+ (cogit cog: newMethod selector: messageSelector)<br>
- cogMethod selector ~= selector ifTrue: [<br>
- (cogit cog: newMethod selector: selector)<br>
ifNotNil: [:newCogMethod | cogMethod := newCogMethod]]].<br>
+ cogMethod selector = messageSelector<br>
- cogMethod selector = selector<br>
ifTrue:<br>
[cogit<br>
linkNSSendCache: nsSendCache<br>
+ classTag: methodReceiverClassTag<br>
+ enclosingObject: localAbsentReceiverOrZero<br>
- classTag: mrClassTag<br>
- enclosingObject: enclosingObject<br>
target: cogMethod<br>
caller: self mframeHomeMethodExport]<br>
ifFalse: ["Out of code memory. Fall through to interpret."].<br>
instructionPointer := self popStack.<br>
self executeNewMethod.<br>
+ self unreachable].<br>
- self error: 'UNREACHABLE 1'].<br>
instructionPointer := self popStack.<br>
self interpretMethodFromMachineCode.<br>
+ self unreachable.!<br>
- self error: 'UNREACHABLE 2'.<br>
- ^nil!<br>
<br>
Item was removed:<br>
- ----- Method: CoInterpreter>>implicitReceiverFor:mixin:implementing: (in category 'newspeak bytecode support') -----<br>
- implicitReceiverFor: methodReceiver mixin: methodMixin implementing: selector<br>
- "This is used to implement implicit receiver sends in Newspeak. Find the nearest<br>
- lexically-enclosing implementation of selector by searching up the static chain of the<br>
- method's receiver, starting at mixin's application. This implementation is derived from<br>
-<br>
- <ContextPart> implicitReceiverFor: methodReceiver <Object><br>
- in: methodMixin <Mixin><br>
- implementing: selector <Symbol> ^<Object>"<br>
-<br>
- <api><br>
- <option: #NewspeakVM><br>
- cogit breakOnImplicitReceiver ifTrue:<br>
- [self sendBreakpoint: selector receiver: nil].<br>
- ^super implicitReceiverFor: methodReceiver mixin: methodMixin implementing: selector!<br>
<br>
Item was added:<br>
+ ----- Method: CoInterpreter>>lookupImplicitReceiverSendNoMNU: (in category 'message sending') -----<br>
+ lookupImplicitReceiverSendNoMNU: methodReceiver<br>
+ "Do the full lookup for an implicit receiver send.<br>
+ IN: messageSelector<br>
+ IN: argumentCount<br>
+ OUT: localAbsentReceiver<br>
+ OUT: localAbsentReceiverOrZero<br>
+ OUT: newMethod<br>
+ OUT: primitiveIndex<br>
+ RESULT: 0 or SelectorDoesNotUnderstand"<br>
+<br>
+ | candidateReceiver candidateMixin candidateMixinApplication dictionary found |<br>
+ messageSelector := objectMemory followMaybeForwarded: messageSelector.<br>
+ candidateReceiver := methodReceiver.<br>
+ self deny: (objectMemory isForwarded: method).<br>
+ candidateMixin := self methodClassOf: method.<br>
+ localAbsentReceiverOrZero := 0.<br>
+ [self deny: (objectMemory isForwarded: candidateMixin).<br>
+ self deny: (objectMemory isForwarded: candidateReceiver).<br>
+ candidateMixinApplication := self<br>
+ findApplicationOfTargetMixin: candidateMixin<br>
+ startingAtBehavior: (objectMemory fetchClassOf: candidateReceiver).<br>
+ self deny: (candidateMixinApplication = 0).<br>
+ self deny: (candidateMixinApplication = objectMemory nilObject).<br>
+ self deny: (objectMemory isForwarded: candidateMixinApplication).<br>
+ self assert: (self addressCouldBeClassObj: candidateMixinApplication).<br>
+ dictionary := objectMemory followObjField: MethodDictionaryIndex ofObject: candidateMixinApplication.<br>
+ found := self lookupMethodInDictionary: dictionary.<br>
+ found ifTrue:<br>
+ [localAbsentReceiver := candidateReceiver.<br>
+ ^self lookupLexicalNoMNU: messageSelector from: candidateMixin rule: LookupRuleImplicit].<br>
+ candidateMixin := objectMemory followObjField: EnclosingMixinIndex ofObject: candidateMixin.<br>
+ self deny: (objectMemory isForwarded: candidateMixin).<br>
+ candidateMixin = objectMemory nilObject]<br>
+ whileFalse:<br>
+ [localAbsentReceiverOrZero := candidateReceiver := objectMemory followObjField: EnclosingObjectIndex ofObject: candidateMixinApplication].<br>
+ "There is no lexically visible method, so the implicit receiver is the method receiver."<br>
+ localAbsentReceiverOrZero := 0.<br>
+ localAbsentReceiver := methodReceiver.<br>
+ lkupClass := objectMemory fetchClassOf: methodReceiver. "For use by MNU"<br>
+ ^self lookupProtectedNoMNU: messageSelector startingAt: lkupClass rule: LookupRuleImplicit.!<br>
<br>
Item was added:<br>
+ ----- Method: CoInterpreter>>lookupLexicalNoMNU:from:rule: (in category 'message sending') -----<br>
+ lookupLexicalNoMNU: selector from: mixin rule: rule<br>
+ "A shared part of the lookup for implicit receiver sends that found a lexically visible<br>
+ method, and self and outer sends."<br>
+ | receiverClass mixinApplication dictionary found |<br>
+ receiverClass := objectMemory fetchClassOf: localAbsentReceiver.<br>
+ mixinApplication := self findApplicationOfTargetMixin: mixin startingAtBehavior: receiverClass.<br>
+ dictionary := objectMemory followObjField: MethodDictionaryIndex ofObject: mixinApplication.<br>
+ found := self lookupMethodInDictionary: dictionary.<br>
+ (found and: [(self accessModifierOfMethod: newMethod) = AccessModifierPrivate])<br>
+ ifTrue: [^0].<br>
+ ^self lookupProtectedNoMNU: selector startingAt: receiverClass rule: rule<br>
+ !<br>
<br>
Item was added:<br>
+ ----- Method: CoInterpreter>>lookupOuterSendNoMNU:depth: (in category 'message sending') -----<br>
+ lookupOuterSendNoMNU: methodReceiver depth: depth<br>
+ "Do the full lookup for a self or outer send.<br>
+ IN: messageSelector<br>
+ IN: argumentCount<br>
+ OUT: localAbsentReceiver<br>
+ OUT: localAbsentReceiverOrZero<br>
+ OUT: newMethod<br>
+ OUT: primitiveIndex<br>
+ RESULT: 0 or SelectorDoesNotUnderstand"<br>
+<br>
+ | targetMixin count mixinApplication |<br>
+ localAbsentReceiver := methodReceiver.<br>
+ localAbsentReceiverOrZero := 0.<br>
+ targetMixin := self methodClassOf: method.<br>
+ count := 0.<br>
+ [count < depth] whileTrue:<br>
+ [count := count + 1.<br>
+ mixinApplication := self<br>
+ findApplicationOfTargetMixin: targetMixin<br>
+ startingAtBehavior: (objectMemory fetchClassOf: localAbsentReceiver).<br>
+ localAbsentReceiverOrZero := localAbsentReceiver := objectMemory followObjField: EnclosingObjectIndex ofObject: mixinApplication.<br>
+ targetMixin := objectMemory followObjField: EnclosingMixinIndex ofObject: targetMixin].<br>
+ ^self lookupLexicalNoMNU: messageSelector from: targetMixin rule: depth!<br>
<br>
Item was added:<br>
+ ----- Method: CoInterpreter>>lookupProtectedNoMNU:startingAt:rule: (in category 'message sending') -----<br>
+ lookupProtectedNoMNU: selector startingAt: mixinApplication rule: rule<br>
+ "A shared part of the lookup for self, outer or implicit receiver sends that did not find a<br>
+ private lexically visible method, and (Newspeak) super sends."<br>
+ | lookupClass dictionary found |<br>
+ lookupClass := mixinApplication.<br>
+ [lookupClass = objectMemory nilObject] whileFalse:<br>
+ [dictionary := objectMemory followObjField: MethodDictionaryIndex ofObject: lookupClass.<br>
+ found := self lookupMethodInDictionary: dictionary.<br>
+ (found and: [(self accessModifierOfMethod: newMethod) ~= AccessModifierPrivate])<br>
+ ifTrue: [^0].<br>
+ lookupClass := self superclassOf: lookupClass].<br>
+ ^SelectorDoesNotUnderstand!<br>
<br>
Item was added:<br>
+ ----- Method: CoInterpreter>>unreachable (in category 'cog jit support') -----<br>
+ unreachable<br>
+ self error: 'UNREACHABLE'.!<br>
<br>
Item was removed:<br>
- ----- Method: StackInterpreter>>implicitReceiverFor:mixin:implementing: (in category 'newspeak bytecode support') -----<br>
- implicitReceiverFor: methodReceiver mixin: methodMixin implementing: selector<br>
- "This is used to implement implicit receiver sends in Newspeak. Find the nearest<br>
- lexically-enclosing implementation of selector by searching up the static chain of the<br>
- method's receiver, starting at mixin's application. This implementation is derived from<br>
-<br>
- <ContextPart> implicitReceiverFor: methodReceiver <Object><br>
- in: methodMixin <Mixin><br>
- implementing: selector <Symbol> ^<Object>"<br>
- <api><br>
- <option: #NewspeakVM><br>
- | candidateReceiver candidateMixin candidateMixinApplication dictionary found |<br>
- self deny: (objectMemory isOopForwarded: methodReceiver).<br>
- self deny: (objectMemory isForwarded: methodMixin).<br>
- "messageSelector is an implicit parameter of lookupMethodInDictionary:"<br>
- messageSelector := objectMemory followMaybeForwarded: selector.<br>
- candidateReceiver := methodReceiver.<br>
- candidateMixin := methodMixin.<br>
- [candidateMixinApplication := self<br>
- findApplicationOfTargetMixin: candidateMixin<br>
- startingAtBehavior: (objectMemory fetchClassOf: candidateReceiver).<br>
- self deny: (candidateMixinApplication = objectMemory nilObject).<br>
- dictionary := objectMemory followObjField: MethodDictionaryIndex ofObject: candidateMixinApplication.<br>
- found := self lookupMethodInDictionary: dictionary.<br>
- found ifTrue: [^candidateReceiver].<br>
- candidateMixin := objectMemory followObjField: EnclosingMixinIndex ofObject: candidateMixin.<br>
- candidateMixin = objectMemory nilObject]<br>
- whileFalse:<br>
- [candidateReceiver := objectMemory followObjField: EnclosingObjectIndex ofObject: candidateMixinApplication].<br>
- ^methodReceiver!<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">best,<div>Eliot</div></div>
</div>