gh-129552: Make condition inside codegen_addop_load_const a macro

[WolframAlph]

• Issue: Make condition inside codegen_addop_load_const a macro #129552

[ZeroIntensity]

I'd prefer this become an assertion.

[picnixz]

I'd prefer removing the < 256 and keep the macro constant as this could be changed in the future. Currently this is 257 so it is indeed redundant but if we remove the < 256, we might want to rather decide whether to use LOAD_SMALL_INT opcode depending on the value of that macro rather than the hardcoded 256.

cc @markshannon@Eclips4

[picnixz]

Note: I remember I asked Mark about it at some point but forgot where I asked. I think we had assertions indeed but I don't remember whether we used the macro or a hardcoded constant.

[iritkatriel]

I'd prefer removing the < 256 and keep the macro constant as this could be changed in the future. Currently this is 257 so it is indeed redundant but if we remove the < 256, we might want to rather decide whether to use LOAD_SMALL_INT opcode depending on the value of that macro rather than the hardcoded 256.

cc @markshannon@Eclips4

But aren't we also restricted by magnitude of oparg here? We probably don't want to use extended args for this.

[picnixz]

Ah yes.. the oparg. Right. Thanks Irit. That was the reason why we had the hardcoded 256. Nevermind my suggestion then, but we can keep an assertion as Peter suggested.

[WolframAlph]

Yes, at first I tried to remove 256 check but then realized that extended arg is generated in that case which I suppose is not desired behavior. What assertion and where? I don't quite get.

[picnixz]

In the if-block, just assert that it's also smaller than the _PY_NSMALLPOSINTS macro value (I guess that's what @ZeroIntensity wanted)

[WolframAlph]

In the if-block, just assert that it's also smaller than the _PY_NSMALLPOSINTS macro value (I guess that's what @ZeroIntensity wanted)

It is already guarded by val < 256. Isn't it even more redundant?

[picnixz]

Assertions are cost-free but if the value of that macro (small ints) changes, we might have some surprises (as we would have an array OOB at some point I think, but only if we make it smaller than 256)

Ideally I would prefer to have 256 as a macro to stress that it's for a non-extended oparg, as it could be confused with the max small posint but I think that this limit (256) is generally hardcoded.

[WolframAlph]

So based on #129568 (comment), I am leaving this condition as it is and converting it to a macro to be reused in CFG. cc @iritkatriel

[WolframAlph]

Closing this one as it will be done as part of another issue.