Handle arithmetic overflow in select_addr

[gretay-js]

Selection performs constant folding on memory addresses for amd64. Large constants can be created with flambda2 that cause overflow. This PR uses nativeint instead of int.
It's possible that better code can be generated if we recognize earlier that constants overflow and fold some of them rather than bailing out on the entire expression. I'll add a cmm-based test.

[lthls]

Just to make sure I understand this PR: this does not prevent overflows, it merely hopes that overflows on the target will be replicated exactly by Nativeint. Wouldn't it be better to use Int64 instead, as we know it's going to match the size of the actual operations even in the case of cross-compilation (assuming we ever support it) ?
My preferred fix would be to consider any overflow in displacement a red flag, and use Misc.no_overflow_* to check for overflow, with either a warning or just the default case when we do detect an overflow.
But I guess your comment about folding part of the expression refers to when we end up with displacements that are outside the range of allowed immediates ? That looks like an interesting improvement, but not very much related to overflows.

[gretay-js]

My preferred fix would be to consider any overflow in displacement a red flag, and use Misc.no_overflow_* to check for overflow, with either a warning or just the default case when we do detect an overflow. But I guess your comment about folding part of the expression refers to when we end up with displacements that are outside the range of allowed immediates ? That looks like an interesting improvement, but not very much related to overflows.

Use of Misc.no_overflow_ was my first version... Nativeint is slightly simpler but if correctness is more obvious without it, I'm happy to change.

What I meant in the optimization is to use is_immediate directly before every fold, which obviates the need for overflow checking on amd64 (I'm ignoring crosscompilation for the moment)

[gretay-js]

Wouldn't it be better to use Int64 instead, as we know it's going to match the size of the actual operations even in the case of cross-compilation (assuming we ever support it) ?

For cross-compilation, it would be Targetint not Int64, right?

[lthls]

Wouldn't it be better to use Int64 instead, as we know it's going to match the size of the actual operations even in the case of cross-compilation (assuming we ever support it) ?

For cross-compilation, it would be Targetint not Int64, right?

Both would work, since we're in the amd64 backend. Targetint does feel more natural, indeed.

[gretay-js]

I modified the code to use Misc.no_overflow_*.

[lthls]

I'm not sure why you chose to use a Cmm test, I can reproduce the problem quite reliably with the following ml file:

let[@inline never][@local never] f n =
  let n = Int64.of_int n in
  let open Int64 in
  to_int (add n (of_int Int.min_int))

let _ = Printf.printf "%d\n%!" (f 1)

I think adding a simple (* TEST *) header + the reference output should be enough to integrate it in the testsuite.

[lthls]

(The use of a .ml test is going to help when upstreaming the fix too)

[gretay-js]

I'm not sure why you chose to use a Cmm test, I can reproduce the problem quite reliably with the following ml file:

I replaced the cmm test with your example, it's so much better, thank you!

Just for the recorded, the output before this PR was 0x1. Correct output is 0x4000000000000001.