[PATCH] D55494: [x86] allow 8-bit adds to be promoted by convertToThreeAddress() to form LEA

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

This patch looks good to me.

However, I will let Craig give the final approval.



================
Comment at: test/CodeGen/X86/popcnt.ll:37-41
 ; X64-NEXT:    addb %al, %dil
 ; X64-NEXT:    movl %edi, %eax
 ; X64-NEXT:    shrb $4, %al
-; X64-NEXT:    addb %dil, %al
+; X64-NEXT:    leal (%rax,%rdi), %eax
 ; X64-NEXT:    andb $15, %al
----------------
As I wrote before, I think this patch looks good to me.

I just wanted to point out that the new codegen might lead to an increase in the number of merge opcodes on Sandybridge (see explanation below). That being said, I don't think it is something that we should be worrying about. Sorry in advance for the pedantic comment below...

---

On Sandybridge (according to Agner and Intel docs), a partial write to AL triggers a merge opcode on a later read of AX/EAX/RAX. Basically, it is `as-if` AL is renamed separate from RAX. We trade a small increase in ILP at the cost of introducing a merge opcode on a dirty read of the underlying 2/4/8 byte register.

AMD CPUs and modern Intel CPUs (from IvyBridge onwards) don't rename AL separate from RAX. That means, a write to AL always merges into RAX with a (false) dependency on RAX.

Quoting Agner: `IvyBridge inserts an extra μop only in the case where a high 8-bit register (AH, BH, CH, DH) has been modified`.

Intel cpus (not AMD) rename high8-bit registers. A write to AH allocates a distinct physical register. The advantage is that a write to AL can now go in parallel with a write to AH. The downside is that a read of AX/EAX/RAX triggers a merge uOp if AH is dirty. The latency of that merge uop tends to be very small (however, it varies from processor to processor).

With this patch, we sometimes trade an partial byte read (example `addb`) with a full register read (through LEA) which has the potential of triggering an extra merge uOp on Sandybridge.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55494/new/

https://reviews.llvm.org/D55494