This doesn't seem to be working - it fails with set.ml instead. I wondered if the equivalent place for the Arch.win64 in amd64/reload.ml was the problem, but then it fails at complex.ml

At which point I'm getting out of my depth - but happy to help debugging further. AppVeyor seems very unhappy with the generated code, though I haven't looked in detail as to why...

By the way - did we only just enable the "Reviewers" feature today, or was GitHub listening to the dev meeting on Tuesday and psychically added a new feature?

OK, out of my depth, but still trying! This seems to fix things. I haven't fully confirmed the testsuite, because 4.04 changes aren't yet on trunk (something I may do something about later...) but how's this:

diff --git a/asmcomp/amd64/emit.mlp b/asmcomp/amd64/emit.mlp
index 85b4cee31..5b69920f3 100644
--- a/asmcomp/amd64/emit.mlp
+++ b/asmcomp/amd64/emit.mlp
@@ -992,7 +992,7 @@ let begin_assembly() =
   end;


-  if !Clflags.dlcode then begin
+  if !Clflags.dlcode || true then begin
     (* from amd64.S; could emit these constants on demand *)
     begin match system with
     | S_macosx -> D.section ["__TEXT";"__literal16"] None ["16byte_literals"]
diff --git a/asmcomp/amd64/reload.ml b/asmcomp/amd64/reload.ml
index 2e29de4c1..690e01651 100644
--- a/asmcomp/amd64/reload.ml
+++ b/asmcomp/amd64/reload.ml
@@ -94,7 +94,7 @@ method! reload_operation op arg res =
       then (arg, res)
       else super#reload_operation op arg res
   | Iconst_symbol _ ->
-      if !Clflags.pic_code || !Clflags.dlcode
+      if !Clflags.pic_code || !Clflags.dlcode || Arch.win64
       then super#reload_operation op arg res
       else (arg, res)
   | _ -> (* Other operations: all args and results in registers *)

I imagine that my || true should possibly be || win64 but I'm wondering if Cflags.pic_code should be in there too? Basically, I don't really understand why it works 😄

@mshinwell I still get the error with your patch.

Same observations as @dra27 :

• Iconst_symbol triggers the error, fixed in reload.ml
• caml_negf_mask also, fixed Emit.begin_assembly

Then it seems to work !

@dra27 @oandrieu I've incorporated those changes; perhaps you could try again. Can you try building something large to see if any further errors of this form occur?

Will do - but not until tomorrow.

All things considered, I'd rather remove the Iconst_symbol specific operation altogether, because 1- it is rarely usable (only in completely static code), and 2- it assumes that static symbols are in the low 4Gb of the addressing space, an assumption that currently holds in Linux and perhaps MacOS X as well, but could be invalidated at any time.

Sometimes removing code is the best solution..

@mshinwell I recompiled my not-so small application that exhibited the error in the first place — works fine now.

@xavierleroy you mean Istore_symbol (rather than Iconst_symbol) ?

@xavierleroy I presume you meant Istore_symbol, which I've now removed.

I don't yet understand why the changes to Iconst_symbol in Reload and the emitter are required. @dra27 @oandrieu Can you confirm those are still needed (I think they will be)?

I meant Istore_symbol, as you correctly guessed. Sorry about that.

The change in Reload for Iconst_symbol forces the result to be a register (and not a stack slot) in more cases. This is safe, and necessary in cases where Iconst_symbol is turned into a movabsq or into a PC-relative load. I'm not sure what went wrong before, but in any case the change looks sound to me.

The latest version (sans Istore_symbol) looks good to me.

I'll wait for David / Olivier to try it once more before merging.

Still works for me, thx.