[llvm] r323548 - [Hexagon] Fix an incorrect assertion in HexagonConstExtenders

[Krzysztof Parzyszek via llvm-commits]

Author: kparzysz
Date: Fri Jan 26 11:20:50 2018
New Revision: 323548

URL: http://llvm.org/viewvc/llvm-project?rev=323548&view=rev
Log:
[Hexagon] Fix an incorrect assertion in HexagonConstExtenders

Added:
    llvm/trunk/test/CodeGen/Hexagon/cext-opt-range-assert.mir
Modified:
    llvm/trunk/lib/Target/Hexagon/HexagonConstExtenders.cpp

Modified: llvm/trunk/lib/Target/Hexagon/HexagonConstExtenders.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonConstExtenders.cpp?rev=323548&r1=323547&r2=323548&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Hexagon/HexagonConstExtenders.cpp (original)
+++ llvm/trunk/lib/Target/Hexagon/HexagonConstExtenders.cpp Fri Jan 26 11:20:50 2018
@@ -999,15 +999,19 @@ unsigned HCE::getDirectRegReplacement(un
   return 0;
 }
 
-// Return the allowable deviation from the current value of Rb which the
+// Return the allowable deviation from the current value of Rb (i.e. the
+// range of values that can be added to the current value) which the
 // instruction MI can accommodate.
 // The instruction MI is a user of register Rb, which is defined via an
 // extender. It may be possible for MI to be tweaked to work for a register
 // defined with a slightly different value. For example
-//   ... = L2_loadrub_io Rb, 0
+//   ... = L2_loadrub_io Rb, 1
 // can be modifed to be
-//   ... = L2_loadrub_io Rb', 1
-// if Rb' = Rb-1.
+//   ... = L2_loadrub_io Rb', 0
+// if Rb' = Rb+1.
+// The range for Rb would be [Min+1, Max+1], where [Min, Max] is a range
+// for L2_loadrub with offset 0. That means that Rb could be replaced with
+// Rc, where Rc-Rb belongs to [Min+1, Max+1].
 OffsetRange HCE::getOffsetRange(Register Rb, const MachineInstr &MI) const {
   unsigned Opc = MI.getOpcode();
   // Instructions that are constant-extended may be replaced with something
@@ -1618,7 +1622,7 @@ bool HCE::replaceInstrExpr(const ExtDesc
     assert(IdxOpc == Hexagon::A2_addi);
 
     // Clamp Diff to the 16 bit range.
-    int32_t D = isInt<16>(Diff) ? Diff : (Diff > 32767 ? 32767 : -32767);
+    int32_t D = isInt<16>(Diff) ? Diff : (Diff > 0 ? 32767 : -32768);
     BuildMI(MBB, At, dl, HII->get(IdxOpc))
       .add(MI.getOperand(0))
       .add(MachineOperand(ExtR))
@@ -1626,11 +1630,13 @@ bool HCE::replaceInstrExpr(const ExtDesc
     Diff -= D;
 #ifndef NDEBUG
     // Make sure the output is within allowable range for uses.
+    // "Diff" is a difference in the "opposite direction", i.e. Ext - DefV,
+    // not DefV - Ext, as the getOffsetRange would calculate.
     OffsetRange Uses = getOffsetRange(MI.getOperand(0));
-    if (!Uses.contains(Diff))
-      dbgs() << "Diff: " << Diff << " out of range " << Uses
+    if (!Uses.contains(-Diff))
+      dbgs() << "Diff: " << -Diff << " out of range " << Uses
              << " for " << MI;
-    assert(Uses.contains(Diff));
+    assert(Uses.contains(-Diff));
 #endif
     MBB.erase(MI);
     return true;

Added: llvm/trunk/test/CodeGen/Hexagon/cext-opt-range-assert.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Hexagon/cext-opt-range-assert.mir?rev=323548&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Hexagon/cext-opt-range-assert.mir (added)
+++ llvm/trunk/test/CodeGen/Hexagon/cext-opt-range-assert.mir Fri Jan 26 11:20:50 2018
@@ -0,0 +1,54 @@
+# RUN: llc -march=hexagon -run-pass hexagon-cext-opt %s -o - | FileCheck %s
+# REQUIRES: asserts
+#
+# This testcase used to trigger an incorrect assertion. Make sure it no
+# longer does.
+# CHECK: A2_tfrsi @G + 65536
+
+--- |
+  define void @fred() {
+    ret void
+  }
+  @G = external global [128 x i16], align 8
+...
+
+---
+name: fred
+tracksRegLiveness: true
+
+body: |
+  bb.0:
+    successors: %bb.1
+    %6:intregs = A2_tfrsi @G
+    %7:intregs = A2_addi killed %6, 2
+    %8:intregs = A2_tfrsi 127
+    ADJCALLSTACKDOWN 0, 0, implicit-def %r29, implicit-def dead %r30, implicit %r31, implicit %r30, implicit %r29
+    %r0 = COPY %7
+    %r1 = COPY %8
+    %9:intregs = IMPLICIT_DEF
+    J2_callr killed %9, implicit-def dead %pc, implicit-def dead %r31, implicit %r29, implicit %r0, implicit %r1, implicit-def %r29
+    ADJCALLSTACKUP 0, 0, implicit-def dead %r29, implicit-def dead %r30, implicit-def dead %r31, implicit %r29
+    %5:intregs = A2_tfrsi 8
+    %10:intregs = A2_tfrsi @G + 8
+    %4:intregs = A2_addi killed %10, 2
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %0:intregs = PHI %4, %bb.0, %3, %bb.1
+    %1:intregs = PHI %5, %bb.0, %2, %bb.1
+    %11:predregs = C2_cmpgtui %1, 127
+    %2:intregs = A2_addi %1, 8
+    %3:intregs = A2_addi %0, 16
+    J2_jumpf %11, %bb.1, implicit-def %pc
+
+  bb.2:
+    %13:intregs = A2_tfrsi @G
+    %14:intregs = A2_addi killed %13, 2
+    %15:intregs = A2_tfrsi 127
+    ADJCALLSTACKDOWN 0, 0, implicit-def %r29, implicit-def dead %r30, implicit %r31, implicit %r30, implicit %r29
+    %r0 = COPY %14
+    %r1 = COPY %15
+    %16:intregs = IMPLICIT_DEF
+    PS_callr_nr killed %16, implicit %r0, implicit %r1, implicit-def %r29
+    ADJCALLSTACKUP 0, 0, implicit-def dead %r29, implicit-def dead %r30, implicit-def dead %r31, implicit %r29
+...