rust-legacy-fork icon indicating copy to clipboard operation
rust-legacy-fork copied to clipboard

Remove the DREGS_WITHOUT_Z hack

Open dylanmckay opened this issue 6 years ago • 2 comments

This was introduced in #37 in order to work around a regalloc bug.

Since then, a root cause fix has landed in upstream LLVM in D54218.

Nirav pointed this out in the bugzilla associated with this issue; https://bugs.llvm.org/show_bug.cgi?id=38224.

Now that the root cause has been fixed, we should be able to remove the DREGS_WITHOUT_Z_WORKAROUND register class introduced in 9de46729ce2fb4bf4b2a19b7b387898f256d827f, which will give us more optimal code generation.

dylanmckay avatar Jan 16 '19 07:01 dylanmckay

I was just messing around a bit. Removing the DREGS_WITHOUT_Z hack (in upstream llvm) still seems to trigger the tests CodeGen/AVR/rust-avr-bug-37.ll and CodeGen/AVR/rust-avr-bug-95.ll.


That is:

commit 3f33b241bd1d7cd77fb89d058f6355668ea758d1 (HEAD -> avr-rust-126, origin/avr-rust-126)
Author: Daan Sprenkels <[email protected]>
Date:   Sat Mar 30 22:38:12 2019 +0100

    [AVR] Remove the DREGS_WITHOUT_Z hack
    
    As proposed by https://github.com/avr-rust/rust/issues/126

diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td
index c458fe7de06..5669d88b25e 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.td
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -1230,7 +1230,7 @@ isReMaterializable = 1 in
   // ldd Rd,   P+q
   // ldd Rd+1, P+q+1
   let Constraints = "@earlyclobber $dst" in
-  def LDDWRdPtrQ : Pseudo<(outs DREGS_WITHOUT_Z_WORKAROUND:$dst),
+  def LDDWRdPtrQ : Pseudo<(outs DREGS:$dst),
                           (ins memri:$memri),
                           "lddw\t$dst, $memri",
                           [(set i16:$dst, (load addr:$memri))]>,
diff --git a/llvm/lib/Target/AVR/AVRRegisterInfo.td b/llvm/lib/Target/AVR/AVRRegisterInfo.td
index e20f69beabe..b83f0597dd9 100644
--- a/llvm/lib/Target/AVR/AVRRegisterInfo.td
+++ b/llvm/lib/Target/AVR/AVRRegisterInfo.td
@@ -156,26 +156,6 @@ def DREGS : RegisterClass<"AVR", [i16], 8,
     R9R8, R7R6, R5R4, R3R2, R1R0
   )>;
 
-// The 16-bit DREGS register class, excluding the Z pointer register.
-//
-// This is used by instructions which cause high pointer register
-// contention which leads to an assertion in the register allocator.
-//
-// There is no technical reason why instructions that use this class
-// cannot use Z; it's simply a workaround a regalloc bug.
-//
-// More information can be found in PR39553.
-def DREGS_WITHOUT_Z_WORKAROUND : RegisterClass<"AVR", [i16], 8,
-  (
-    // Return value and arguments.
-    add R25R24, R19R18, R21R20, R23R22,
-    // Scratch registers.
-    R27R26,
-    // Callee saved registers.
-    R29R28, R17R16, R15R14, R13R12, R11R10,
-    R9R8, R7R6, R5R4, R3R2, R1R0
-  )>;
-
 // 16-bit register class for immediate instructions.
 def DLDREGS : RegisterClass<"AVR", [i16], 8,
   (

gives:

Failing Tests (3):
    LLVM :: CodeGen/AVR/issue-cannot-select-bswap.ll
    LLVM :: CodeGen/AVR/rust-avr-bug-37.ll
    LLVM :: CodeGen/AVR/rust-avr-bug-95.ll

  Expected Passes    : 119
  Expected Failures  : 3
  Unexpected Failures: 3

(Note: CodeGen/AVR/issue-cannot-select-bswap.ll also failed before applying the patch.)

dsprenkels avatar Mar 30 '19 22:03 dsprenkels

It looks like this workaround is not necessary anymore, so I have made a patch to remove it: https://reviews.llvm.org/D117831 All CodeGen tests pass with this workaround removed.

aykevl avatar Jan 20 '22 21:01 aykevl