[PATCH] D62300: [AVR] Fix incorrect source regclass of LDWRdPtr

Mon May 27 00:28:11 PDT 2019

Jim marked 2 inline comments as done.
Jim added inline comments.

================
Comment at: lib/Target/AVR/AVRRegisterInfo.td:168
 // More information can be found in PR39553.
-def DREGS_WITHOUT_Z_WORKAROUND : RegisterClass<"AVR", [i16], 8,
+def DREGS_WITHOUT_YZ_WORKAROUND : RegisterClass<"AVR", [i16], 8,
   (
----------------
Since we change the register class of pointer for LDWRdPtr to the small one . We have the same issue like Z register.

================
Comment at: lib/Target/AVR/AVRRegisterInfo.td:175
     // Callee saved registers.
-    R29R28, R17R16, R15R14, R13R12, R11R10,
+    R17R16, R15R14, R13R12, R11R10,
     R9R8, R7R6, R5R4, R3R2, R1R0
----------------
dylanmckay wrote:
> I can't tell why the removal of `Y` is needed.
> 
> `LDWRdPtr` expands to `LDRdPtr` and `LDDRdPtrQ`.
> 
> `LDRdPtr` accepts any pointer register (`X`, `Y`, or `Z`), and as you point out, `LDDRdPtrQ` only accepts `DREGS_WITHOUT_Z_WORKAROUND:` (AKA some GPRs and `X`, `Y`). The register constraints over the instruction pair is the intersection of the register classes for these sub instructions, which is the set of pointer registers `{ X, Y }`
> 
> What are your thoughts?
`LDRdPtr` accepts `X`, `Y` or `Z` as pointer register, But `LDDRdPtrQ` only accepts `Y` or `Z` as pointer register. 
 I changed the pointer register of `LDWRdPtr` from {`X`, `Y`, `Z`} to {`Y`, `Z`}.  Since their intersection of registers are `Y` or `Z` (Register class `PTRDISPREGS`). `Y` has more register pressure due to this change. So I removed Y from DREGS_WITHOUT_Z_WORKAROUND. In my experiment, if it is without the removal of `Y`, it has the same issue, ran out of registers, as `Z`.

================
Comment at: test/CodeGen/AVR/pseudo/LDDWRdPtrQ-same-src-dst.mir:1
-# RUN: llc -O0  %s -o - -march=avr | FileCheck %s
-
----------------
The source register and pointer register of LDDWRdPtrQ will not be the same. So I deleted this test.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62300