[PATCH] D18893: [mips] Sign-extend i32 values truncated from previously zero-extended i32 values.

[Daniel Sanders via llvm-commits]

dsanders added inline comments.

================
Comment at: lib/Target/Mips/Mips64InstrInfo.td:516
@@ -511,3 +515,3 @@
               (EXTRACT_SUBREG GPR64:$src, sub_32)>;
-def : MipsPat<(trunc (assertzext GPR64:$src)),
+def : MipsPat<(trunc (assertzext_lt_i32 GPR64:$src)),
               (EXTRACT_SUBREG GPR64:$src, sub_32)>;
----------------
Could you add a comment explaining why this isn't valid for the i32 case?

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:823
@@ +822,3 @@
+
+  // fold (AssertZext (trunc (AssertZext x) -> (trunc (AssertZext x))
+  // if the type of the extension of the innermost AssertZext node is
----------------
Some closing parenthesis are missing on the LHS.

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:823-827
@@ +822,7 @@
+
+  // fold (AssertZext (trunc (AssertZext x) -> (trunc (AssertZext x))
+  // if the type of the extension of the innermost AssertZext node is
+  // smaller from that of the outermost node, eg:
+  // (AssertZext (trunc:i32 (AssertZext:i64 x, i8)), i32) ->
+  //                                                (trunc (AssertZext x, i8))
+  SDValue OuterAssertZext = N0.getOperand(0);
----------------
dsanders wrote:
> Some closing parenthesis are missing on the LHS.
The code implements the right behaviour but I think the expressions in the comment may be inside-out and the usage of 'outer' and 'inner' are the opposite way around to what I'd expect ('outer' is used for `N->getOperand(0)->getOperand(0)`). Shouldn't the first expression be?:
  (AssertZext:i32 (trunc:i32 (AssertZext:i64 X, i32)), i8)
    -> (trunc:i32 (AssertZext X, i8))

Also, I don't think the test cases cover this dag combine rule at the moment. assertzext-trunc.ll doesn't trigger the assertzext/trunc/assertzext case we've looked at before. divrem.ll and octeon_popcnt.ll used to trigger it but don't anymore because of the change to make the arguments signext.


http://reviews.llvm.org/D18893