<headius>
enebo: EqqInstr doesn't seem to do callInfo?
<headius>
nor AsString...
<headius>
ArrayDeref
<kares[m]>
<enebo[m]> "there is one other really..." <- oh really? thought those days were finally behind us ... will attempt to carve some time looking into what changed
<enebo[m]>
kares: on rails-7-stable you can see a few
<enebo[m]>
headius: callInfo only matters at this point if you actually can pass it keyword args but I suppose maybe you can do that with eqq. The others contain no arguments at all right? callInfo would be 0 in both cases
<enebo[m]>
eqq technically could be a bug if you called it with a . syntax and defined === so that perhaps should get looked at in IRBuilder
<enebo[m]>
oh well if called as a normal call it will set callInfo. EQQInstr is only for implicit uses so I think any value change would be dynamic once the call is happening. I don't think syntactically you can pass explicit kwargs
<enebo[m]>
kares: the other thing about OID I would like to fix would be to make typemap "nativeish" where we can ask for OID type without a dyncall. If it exists we pay no cost otherwise we dyncall to allow AR to do as it see fit to resolve it.
<enebo[m]>
metadata does a dyncall once per column per resultset and we could almost entirely remove 100% of them (once the type is registered we know it)
<headius>
enebo: well this is the crux of my question yesterday about clearing it to 0 when there's no kwargs
<headius>
if the clearing is not necessary for cases that can't pass kwargs why is it necessary in places that don't pass kwargs
<headius>
and these instrs cannot pass kwargs fwiw
<headius>
eqq is just for when with multiple elements I believe and the other two are implicit calls
<headius>
enebo: is there a way I can omit setting call info if the target does not care about keywords?
<headius>
like only set it if method.getSignature().hasKwargs()?
<enebo[m]>
the reason to reset is not knowing if the native method will call something which then will examine callInfo and think there was kwargs
<enebo[m]>
ruby calls sites will set it to something but it also masks I think SPLAT since SPLAT happens between when the call originate and when it resolves what it is calling to
<enebo[m]>
native to native calls I think is one place where this state management stinks
<enebo[m]>
.new -> #initialize
<enebo[m]>
since we cannot 0 it out for that
<enebo[m]>
in that case today new effectively has to be marked keywords so it will not erase it
<enebo[m]>
I guess it could be possible if all calls would reset fully at the beginning of the call
<enebo[m]>
for Ruby->Ruby this is possible
<headius>
so I can't just omit setting callInfo even if target signature does not have keywords or something
<enebo[m]>
you want to know if it is an empty kwarg as one possible scenario
<enebo[m]>
if you pass empty kwrest to something which does not accept kwargs they get ignored BUT if something does accept kwargs and there is none provided it still makes an empty one
<enebo[m]>
having said that native methods are generally just do I have some of not
<enebo[m]>
we don't care about empty
<headius>
hmm ok
<enebo[m]>
if callMethod (or something) had callInfo as a parameter in cases where it may accept kwargs then not resetting at entering native would be fine
<enebo[m]>
since we would just read field and by virtue of passing that we are saying the args it is passing has those flags
<enebo[m]>
then all callsites themselves would be some initial value whether from IR callInfo + IR instrs side-effects or just set manually in that variant method
<enebo[m]>
This would change protocol to all callsites must set callInfo properly and nothing between entry of callsite and method it executes can call anything (think coercion method)
<enebo[m]>
IR instrs may alter callInfo state if it realizes it is empty kwargs or something like that which is dynamic
<enebo[m]>
That last bit happens no matter how this is done since you don't know what you have until you have it
<enebo[m]>
KEYWORD_EMPTY could not matter if you always kept the empty kwarg around too but this allows us to know an empty kwarg was provided or not when you care about them
<headius>
ok
<headius>
I'm asking because I am wiring in the indy callInfo logic and I now have the flags from call site right next to the target method
<headius>
wondering if I can match this up more intelligently than just blindly setting always
<enebo[m]>
The other angle would be to not have any argument processing in IR itself but that would have ramifications on the idea of ever supporting inlining
<enebo[m]>
Always set on way in could work I think but it would need a way to set initially for native methods making dynamic calls
<enebo[m]>
I guess too at your level if you know both sides you could omit using that strategy if you know it cannot use kwargs
<enebo[m]>
1.+(foo: 2) should error in a different way than it would naturally so error handling perhaps is a consideration
<enebo[m]>
HAHAHA or not in that case
<enebo[m]>
mri31 -e '1.+(foo: 2)'
<enebo[m]>
-e:1:in `+': Hash can't be coerced into Integer (TypeError)
<enebo[m]>
I wondered about this
<enebo[m]>
We largely ignore kwargs status in native methods when we process them right now
<enebo[m]>
like IO.open(1, **options) our native impl is just like 2 args cool story bro is last one a hash
<enebo[m]>
they are clearly doing that for + because who would do that
<enebo[m]>
I think semantically that should have been 0 for 1 arity mismatch
<enebo[m]>
I predict Ruby 3.x will get a better error message for passing a kw to something which does not expect one
<headius>
> you could omit using that strategy if you know it cannot use kwargs
<headius>
how can I know that
<enebo[m]>
you have site in hand and something tells you it can't
<enebo[m]>
which is missing piece
<enebo[m]>
keywords = true is sort of that proxy atm
<enebo[m]>
but I have using that for forwards of kws to a dyncall
<enebo[m]>
that is the new -> initialize scenario
<headius>
so if it has keywords = true I definitely need it, but if it is false that does not mean I can omit
<headius>
there's not enough info for other method types in other words
<enebo[m]>
well if we accept we will annotate all kws methods with keywords anno we could then depend on that
<enebo[m]>
this is even more complicated in that if you know you are not passing kwargs then you should not need it either
<enebo[m]>
but it still has to be 0 in case it thinks you might be passing a kwarg
<enebo[m]>
so I guess if you know the override you are calling you can know?
<enebo[m]>
but I am balancing this discussion on the notion that we can change something (callInfo set only on way in but also potentially by callMethod sorts of native dyncalls which care)
<enebo[m]>
If we make that change then lets see if we can change something else to eliminate it in cases
<enebo[m]>
As an aside MRI does have keyword variant dyncall methods
<headius>
yeah always on first for now but I think we can finesse this
<enebo[m]>
callMethod({same stuff}, callInfo) which is just a wrapper that sets it
<enebo[m]>
I think that plus not resetting on way into native methods would allow prop calls to preserve their state
<headius>
right that's essentially what these indy sites are doing with handles
<headius>
so I have incoming flags on the one hand and actual method on the other... I can make decisions about what to do with callInfo at that point
<enebo[m]>
callMethodForward
<enebo[m]>
which just omits the reset
<enebo[m]>
but I think this would be short-sighted. We may want to pass in a callInfo different from what we received?
<enebo[m]>
but right now callInfo is not only set on the way in for native methods
<enebo[m]>
// FIXME: This may propagate empty more than the current call? empty might need to be stuff elsewhere to prevent this.
<enebo[m]>
It should just set to 0 at some initial point
<enebo[m]>
this OR I think is having more than one setCallInfo while processing stuff in IR
<enebo[m]>
heh OR == |
<enebo[m]>
setting 0 on the way in was what I originally wanted (and effectively is what IR does) but native methods would require that through all code paths which are dyncalls
<enebo[m]>
which felt difficult but since you are playing at that level
<headius>
enebo: ok what about this case... what if I can see target method does not have kwargs and flags coming in is 0
<headius>
what I have seems to be working... callInfo is done inside the indy call site for all normal and super invokes
<headius>
I merged in my case/when optimizations, which was ultimately just two major things: case "string" and when "string" will use a frozen string now and homogeneous symbol case/when has a lookup table fast path
<enebo[m]>
headius: yeah I think you can do that and if we find out you can't then we mark it with keywords or figure something else to mark it
<headius>
I can give it a shot... for an IR method what's the right way to know if it would want callInfo to be set?
<enebo[m]>
not 0
<enebo[m]>
CALL_KEYWORD is just to know if it recieved kws
<enebo[m]>
CALL_KEYWORD_REST is statically known
<headius>
these are flags in the IRScope yes?
<enebo[m]>
_EMPTY is dynamic usually during processing but it is rare to see it set statically
<enebo[m]>
_SPLATS is also after you enter new method since it needs to know it accepts *foo
<headius>
is getIRScope().receivesKeywordArgs() not sufficient?
<enebo[m]>
ThreadContext
<enebo[m]>
Also with some comments above it CALL_*
<headius>
for the case where call side flags are 0
<enebo[m]>
oh for receiver
<enebo[m]>
no
<enebo[m]>
not for ruby
<headius>
yeah I want to know receiver
<headius>
I have flags = 0 in one hand and an IR method in the other
<headius>
what can I do to eliminate callInfo update
<enebo[m]>
err the method you are calling may dynamically be marked as accepting klwargs
<enebo[m]>
IRScope cannot know definitively
<enebo[m]>
ruby2_keywords def foo; end
<headius>
in what cases might that be determined after we have already compiled the method>
<headius>
?
<enebo[m]>
it is on *Method as state
<enebo[m]>
hmm but where
<enebo[m]>
and it may only be for ruby2keywords
<enebo[m]>
setRuby2Kerywords is on DynamicMethod but somethign higher has the state
<enebo[m]>
since only non-native methods can use that
<enebo[m]>
but it will only to mark that
<enebo[m]>
aaaaarrrgh :)
<headius>
I am confused
<enebo[m]>
ah I see
<enebo[m]>
I thought I did that
<enebo[m]>
setRuby2Keywords will work on IRScope but only in case of marking a method that
<enebo[m]>
It should not exist there because it is not static info or at least we cannot depend on it at JIT time since it may change later
<enebo[m]>
I believe this should only be on the *Method and we should expand it to be a match on whether it statically receives kwargs AND whether the method is currently marked that way
<enebo[m]>
Ok I am not sure I think 65f5958 should have been done
<enebo[m]>
I don't think you can unmark a ruby2keywords method so perhaps it is ok
<headius>
by the time I get it in a call site this should already have been decided, no?
<headius>
I mean, you can still add ruby2keywords flag later but it isn't really done
<enebo[m]>
not in main .rb file right?
<headius>
I'm assuming at the point I see a call goes to a particular target method, this should all have been settled
<enebo[m]>
we need a method entry to exist before we call that method on it so I think it will JIT it as no kwargs
<headius>
and if so then are there any conditions under which I could omit a call-side setCallInfo(0)
<enebo[m]>
and you can use that method at any point during execution
<enebo[m]>
including having called in n times first without it set
<headius>
right, I'm just saying it would seem to be really unusual to make a few calls to a method and then later it gets flipped to ruby2keywords
<headius>
so unusual perhaps I can rely on that not happening
<enebo[m]>
yeah but our precompile a file will not call that and will compile before that is called the firs time
<enebo[m]>
JIT I think is ok
<headius>
this doesn't affect compile time
<headius>
this is at call site binding time
<enebo[m]>
ah just for binding the caller
<headius>
I am not attempting to make a static decision, I am dynamically trying to determine if a method I have in hand could work fine without setCallInfo(0)
<enebo[m]>
is there a bonus to having this in IRScope over *Method?
<headius>
I don't care where it is
<enebo[m]>
I have been trying to have non-static stuff not be in IRScope
<enebo[m]>
you moved it there is why I am asking
<headius>
well I think I moved it there because I assumed IR would have to change argument processing when you set ruby2keywords
<headius>
but that's the only reason I can think of
<enebo[m]>
I find this curious
<enebo[m]>
it does make a change between useKeywordArguments vs ruby2Keywords
<enebo[m]>
that will change behavior
<headius>
that was long before you started to actually implement ruby 3 kwargs so I was just throwing darts
<enebo[m]>
well I think the whole frobnicate stuff is only for ruby2keywords
<enebo[m]>
however you changed this I think did fix a semantic issue
<enebo[m]>
but you did not say that but as you said this was early days
<enebo[m]>
so I would pre this to come into interpret from *MEthod and I think JIT can just access it directly
<enebo[m]>
but having said that...flags on IRScope should not persist and reload stuff which may change
<enebo[m]>
and it doesn't but if you just look at the code it looks like an omission which I suppose is why I think we should not save live info on IRScope (unless it can be avoided)
<enebo[m]>
Anyways....this is not that important to you today
<enebo[m]>
headius: for your purposes today scope.isRuby2keywords is one and scope.recievedKeywords both need to be checked
<enebo[m]>
they both mean different things but I can imagine you want something simple to check so maybe makr IRRuntimeHelpers method to hide that for now and run with that?
<headius>
Yeah ok I'll give it a try
<headius>
enebo: I'm going to add a rubyspec run with indy before I proceed to try removing setCallInfo
<headius>
looking good with the indy-based logic as it stands right now but I just pushed a CI update so it will run rubyspec with indy on 17
<headius>
I'll let that and my other PR cleanup of case/when building settle for a bit