Does this come from aesthetic considerations, or did you observe noticeable reductions in code size? If so, please share your findings!

These changes are part of a larger patch we successfully
tested against the critical path of one of our applications.
However we did not test of these changes individually;
I will add a variant on [1] to ensure there is no regression.

[1] https://github.com/OCamlPro/ocamlbench-repo

It just occurred to me that the following rewrites should probably
be enabled iff fast_code is false:

• mov ~> mov+cdqe;
• mov ~> mov+sal.

(We could also ban inc/dec instructions iff fast_code is true.)

These changes are part of a larger patch we successfully tested against the critical path of one of our applications.

Good to know, but that wasn't @alainfrisch's question. Does all this reduce the code size significantly? By how much?

Sorry, I misread Alain's question.

Here are the sizes for the binaries from the distribution:

And do these tiny reductions lead to any observable performance improvement? Smaller code is usually better, except if you start using less-optimized instructions.

I'm trying to find reasons to add complexity to the compiler and risk performance regressions (on specific versions of the CPU).

I will measure the gains in tight loops; in the meantime, here is an illustration
of the gain (or loss in the case of inc/dec) for each rewrite:

  # use 32-bit legacy instructions when data size is 32 bits -- section 9.2.1
  48 31 c0             	xor    %rax,%rax
  31 c0                	xor    %eax,%eax

  # ibid
  48 25 ff ff ff 7f    	and    $0x7fffffff,%rax
  25 ff ff ff 7f       	and    $0x7fffffff,%eax

  # ibid
  48 c7 c0 01 00 00 00 	mov    $0x1,%rax
  b8 01 00 00 00       	mov    $0x1,%eax

  # ibid
  48 c7 c0 ff ff ff 7f 	mov    $0x7fffffff,%rax
  b8 ff ff ff 7f       	mov    $0x7fffffff,%eax

  # avoid instruction that are eight+ bytes in length -- section 3.4.2.7
  48 b8 ff ff ff ff 00 	movabs $0xffffffff,%rax
  00 00 00 
  b8 ff ff ff ff       	mov    $0xffffffff,%eax
  48 98                	cltq   

  # ibid
  48 b8 00 00 00 00 01 	movabs $0x100000000,%rax
  00 00 00 
  b8 01 00 00 00       	mov    $0x1,%eax
  48 c1 e0 20          	shl    $0x20,%rax

  # avoid inc -- section 3.5.1.1
  48 ff c0             	inc    %rax
  48 83 c0 01          	add    $0x1,%rax

  # ibid
  48 ff c8             	dec    %rax
  48 83 e8 01          	sub    $0x1,%rax

The sections refer to "Intel 64 and IA-32 Architectures Optimization Manual",
available here.

I had a quick look at the new instruction patterns.

Using movl $n, %R32 instead of movq $n, %R64 when n is in [0...0xFFFF_FFFF] is a clear win: two bytes saved, very common idiom, no risk of performance regression.

The other patterns for "load integer constant" are less convincing to me. Replacing one movabsq instruction by two dependent instructions is smaller but might run slower! Plus, I suspect those cases of "load integer constant" to occur rarely.

Using movl $n, %R32 instead of movq $n, %R64 when n is in [0...0xFFFF_FFFF] is a clear win: two bytes saved, very common idiom, no risk of performance regression.

(The submitted patch considered only the values in the range of Int32.t.)
Your example leads to an even larger win:

movq $0xffffffff, %rax ~> 48 b8 ff ff ff ff 00 00 00 00
movl $0xffffffff, %eax ~> b8 ff ff ff ff

Here is the comparison against trunk mentioned above:
http://bench.flambda.ocamlpro.com/compare?test=2017-11-24-1724%2F4.07.0%2Bamd64tweaks%2Bbench&reference=2017-11-24-1724%2F4.07.0%2Btrunk%2Bbench

An instrumented version of the compiler gives the following distribution
of Iconst_int instructions for a run of make world.opt:

As @xavierleroy pointed out, there is no risk of performance regression if we
restrict the patch to only the first two cases.

Would you consider such a patch for merge?

Sorry to insist, but would you accept a patch reduced to the following changes?

1. the use of 32-bit instructions for xor / mov / and when the operand
   is in [0..0xFFFF_FFFF]
2. the use of add / sub instructions instead of incr / decr ones

the use of 32-bit instructions for xor / mov / and when the operand is in [0..0xFFFF_FFFF]

Definitely yes.

the use of add / sub instructions instead of incr / decr ones

Yes, but this is trading increased code size for reduced dependencies, so perhaps keep inc / dec if fast_code is false.

the use of 32-bit instructions for xor / mov / and when the operand is in [0..0xFFFF_FFFF]

Definitely yes.

I have removed the complex patterns around the mov instruction.

the use of add / sub instructions instead of incr / decr ones

Yes, but this is trading increased code size for reduced dependencies, so perhaps keep inc / dec if fast_code is false.

As suggested, I have made the code emitting inc / dec depend on
the value of fastcode_flag.

Travis is not happy. The check_all_arch fails because the last
commit uses a constant (0x01_0000_0000n) that is invalid on
a 32-bit architecture. See #1571.

the last commit uses a constant (0x01_0000_0000n) that is invalid on a 32-bit architecture.

Just write n <= 0xFFFF_FFFFn instead of n < 0x01_0000_0000. Just as clear + syntactically valid on a 32-bit system.

Just write n <= 0xFFFF_FFFFn instead of n < 0x01_0000_0000. Just as clear + syntactically valid on a 32-bit system.

Of course. My gut feeling was that it would hide a real issue.
I guess my understanding was slightly wrong: the code built
my check_all_arches is only expected to be compiled but not
to be executed (some operations over nativeint values
in cmmgen.ml would probably produce invalid results on
32-bit platforms anyway).

Travis is still not happy, but this time it is not even clear why.
The last two lines of the log are:

========= CHECKING asmcomp/arm64 ==============
make: *** [check_all_arches] Error 1

You have another occurrence of 0x01_0000_0000 left.

How embarrassing... I guess I was puzzled by the change/lack
of error message.

However, isn't this occurrence a bit more annoying? As it is an
int value on a 32-bit machine, I will probably not be allowed to
write 0xFFFF_FFFF. Of course, I can promote the value to be
compared to int64, but that does not feel right...

@xclerc: Looks like you merged trunk into your branch, and this resulted in the pull request being messed up. Can you remove the merge commit and rebase instead?

Some other ideas (for a future PR):

• For Iand: when n = 0xFF, use movzx

25 ff 00 00 00       	and    $0xff,%eax
0f b6 c0             	movzbl %al,%eax

• For other operations with 8-bit immediates: We may get a shorter encoding if we use 8-bit registers. This transformation is valid when the high 56-bits are not affected by the operation. IIRC this does not result in an extra merge micro-op.

48 83 cb 01          	or     $0x1,%rbx
80 cb 01             	or     $0x1,%bl

• We may want to consider two-instruction sequences that start with zero-cost operations (xor r32,r32) if they result in a shorter overall encoding.

48 c7 c0 01 00 00 00    mov $0x1,%rax
b8 01 00 00 00          mov $0x1,%eax
31 c0 b0 01             xor %eax,%eax; mov %0x1,%al

ping?

ping @xclerc

Ping again. What is the status here? If @xclerc is not interested in working on this anymore, maybe @smuenzel you would be interested in rebasing the restricted version of this PR and proposing it again, as a new PR?

(I am planning to close next time I encounter this PR if there has not been any progress.)

Closing. We can always reopen if there is interest.