[PATCH] D17973: [DAG]: Combine multiple AssertZexts separated by trunc

[Hans Wennborg via llvm-commits]

hans created this revision.
hans added subscribers: rnk, llvm-commits.

(Uploading as a work-in-progress in case anyone wants to comment.)

The two AssertZexts situation arises for boolean arguments on X86_64:

  void f(bool x) {

LLVM will copy x from a 32-bit register, insert an AssertZext to capture that bits 8-31 are zero (as byte-sized arguments get extended on x86_64), and then another AssertZext to capture that bits 1-7 are also zero, because that's true for bools.

Combining the two allows us to optimize this code:

  void g(bool);
  void f(bool x) {
    g(x);
  }

Previously:

  movzbl  %dil, %edi
  jmp     _Z1gb

After my patch, no zero-extension is generated, as the value in %edi was already extended by the caller.

This raises a number of questions, though:

1) Are 8- and 16-bit arguments really extended on x86_64? LLVM assumes so since http://reviews.llvm.org/rL34623, but the psABI doesn't seem to actually spell that out?

2) Are bools really extended to 32 bits on x86_64? http://reviews.llvm.org/rL127766 says they are not and "[...] We still insert an i32 ZextAssert when reading a function's arguments, but it is followed by a truncate and another i8 ZextAssert so it is not optimized." The psABI seems to have gone back and forth here(!).


My patch would break Win64, because there it's clear that a bool argument is extended only to 8 bits, not 32 bits. But from what I understand, this isn't represented well: Clang's WinX86_64ABIInfo::classify will slap a "zext" attribute on the x parameter, and we end up with both AssertZexts.

http://reviews.llvm.org/D17973

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -228,6 +228,7 @@
     //   otherwise              - N should be replaced by the returned Operand.
     //
     SDValue visitTokenFactor(SDNode *N);
+    SDValue visitAssertZext(SDNode *N);
     SDValue visitMERGE_VALUES(SDNode *N);
     SDValue visitADD(SDNode *N);
     SDValue visitSUB(SDNode *N);
@@ -1350,6 +1351,7 @@
   switch (N->getOpcode()) {
   default: break;
   case ISD::TokenFactor:        return visitTokenFactor(N);
+  case ISD::AssertZext:         return visitAssertZext(N);
   case ISD::MERGE_VALUES:       return visitMERGE_VALUES(N);
   case ISD::ADD:                return visitADD(N);
   case ISD::SUB:                return visitSUB(N);
@@ -1604,6 +1606,31 @@
   return Result;
 }
 
+SDValue DAGCombiner::visitAssertZext(SDNode *N) {
+  SDValue N0 = N->getOperand(0);
+  EVT VT = N->getValueType(0);
+
+  // Try to fold (assert_zext (trunc (assert_zext x t1)) t2).
+  if (N0.getOpcode() == ISD::TRUNCATE &&
+      N0.getOperand(0)->getOpcode() == ISD::AssertZext) {
+    SDValue Trunc = N0;
+    SDValue AZext1 = N0.getOperand(0);
+    EVT T1 = cast<VTSDNode>(AZext1->getOperand(1))->getVT();
+    EVT T2 = cast<VTSDNode>(N->getOperand(1))->getVT();
+
+    if (Trunc->getValueType(0) == T1 && T2.getSizeInBits() < T1.getSizeInBits()) {
+      // Trunc removes the known zeros of AZext1.
+      SDValue CombinedAssertZext =
+          DAG.getNode(ISD::AssertZext, SDLoc(N), AZext1->getValueType(0),
+                      AZext1->getOperand(0), N->getOperand(1));
+      return DAG.getNode(ISD::TRUNCATE, SDLoc(N), VT, CombinedAssertZext);
+
+    }
+  }
+
+  return SDValue();
+}
+
 /// MERGE_VALUES can always be eliminated.
 SDValue DAGCombiner::visitMERGE_VALUES(SDNode *N) {
   WorklistRemover DeadNodes(*this);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D17973.50078.patch
Type: text/x-patch
Size: 1946 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160308/d7dcd74b/attachment.bin>