Reapply 91904.

llvm-svn: 91996

lib/Target/PIC16/PIC16ISelDAGToDAG.h

@@ -36,7 +36,10 @@ class VISIBILITY_HIDDEN PIC16DAGToDAGISel : public SelectionDAGISel {
 public:
   explicit PIC16DAGToDAGISel(PIC16TargetMachine &tm) : 
         SelectionDAGISel(tm),
-        TM(tm), PIC16Lowering(*TM.getTargetLowering()) {}
+        TM(tm), PIC16Lowering(*TM.getTargetLowering()) { 
+    // Keep PIC16 specific DAGISel to use during the lowering
+    PIC16Lowering.ISel = this;
+  }
   
   // Pass Name
   virtual const char *getPassName() const {

lib/Target/PIC16/PIC16ISelLowering.cpp

@@ -1482,7 +1482,8 @@ bool PIC16TargetLowering::isDirectLoad(const SDValue Op) {
 // operand no. of the operand to be converted in 'MemOp'. Remember, PIC16 has 
 // no instruction that can operation on two registers. Most insns take
 // one register and one memory operand (addwf) / Constant (addlw).
-bool PIC16TargetLowering::NeedToConvertToMemOp(SDValue Op, unsigned &MemOp) {
+bool PIC16TargetLowering::NeedToConvertToMemOp(SDValue Op, unsigned &MemOp, 
+                      SelectionDAG &DAG) {
   // If one of the operand is a constant, return false.
   if (Op.getOperand(0).getOpcode() == ISD::Constant ||
       Op.getOperand(1).getOpcode() == ISD::Constant)
@@ -1491,11 +1492,33 @@ bool PIC16TargetLowering::NeedToConvertToMemOp(SDValue Op, unsigned &MemOp) {
   // Return false if one of the operands is already a direct
   // load and that operand has only one use.
   if (isDirectLoad(Op.getOperand(0))) {
-    if (Op.getOperand(0).hasOneUse())
-      return false;
-    else 
-      MemOp = 0;
+    if (Op.getOperand(0).hasOneUse()) {  
+      // Legal and profitable folding check uses the NodeId of DAG nodes.
+      // This NodeId is assigned by topological order. Therefore first 
+      // assign topological order then perform legal and profitable check.
+      // Note:- Though this ordering is done before begining with legalization,
+      // newly added node during legalization process have NodeId=-1 (NewNode)
+      // therefore before performing any check proper ordering of the node is
+      // required.
+      DAG.AssignTopologicalOrder();
+
+      // Direct load operands are folded in binary operations. But before folding
+      // verify if this folding is legal. Fold only if it is legal otherwise
+      // convert this direct load to a separate memory operation.
+      if(ISel->IsLegalAndProfitableToFold(Op.getOperand(0).getNode(), 
+                                         Op.getNode(), Op.getNode()))
+        return false;
+      else 
+        MemOp = 0;
+    }
   }
+
+  // For operations that are non-cummutative there is no need to check 
+  // for right operand because folding right operand may result in 
+  // incorrect operation. 
+  if (! SelectionDAG::isCommutativeBinOp(Op.getOpcode()))
+    return true;
+
   if (isDirectLoad(Op.getOperand(1))) {
     if (Op.getOperand(1).hasOneUse())
       return false;
@@ -1514,7 +1537,7 @@ SDValue PIC16TargetLowering::LowerBinOp(SDValue Op, SelectionDAG &DAG) {
   assert (Op.getValueType() == MVT::i8 && "illegal Op to lower");
 
   unsigned MemOp = 1;
-  if (NeedToConvertToMemOp(Op, MemOp)) {
+  if (NeedToConvertToMemOp(Op, MemOp, DAG)) {
     // Put one value on stack.
     SDValue NewVal = ConvertToMemOperand (Op.getOperand(MemOp), DAG, dl);
 
@@ -1533,7 +1556,7 @@ SDValue PIC16TargetLowering::LowerADD(SDValue Op, SelectionDAG &DAG) {
   assert (Op.getValueType() == MVT::i8 && "illegal add to lower");
   DebugLoc dl = Op.getDebugLoc();
   unsigned MemOp = 1;
-  if (NeedToConvertToMemOp(Op, MemOp)) {
+  if (NeedToConvertToMemOp(Op, MemOp, DAG)) {
     // Put one value on stack.
     SDValue NewVal = ConvertToMemOperand (Op.getOperand(MemOp), DAG, dl);
     
@@ -1574,7 +1597,7 @@ SDValue PIC16TargetLowering::LowerSUB(SDValue Op, SelectionDAG &DAG) {
                          DAG.getConstant(0-(C->getZExtValue()), MVT::i8));
     }
 
-  if (NeedToConvertToMemOp(Op, MemOp) ||
+  if (NeedToConvertToMemOp(Op, MemOp, DAG) ||
       (isDirectLoad(Op.getOperand(1)) && 
        (!isDirectLoad(Op.getOperand(0))) &&
        (Op.getOperand(0).getOpcode() != ISD::Constant)))

lib/Target/PIC16/PIC16ISelLowering.h

@@ -18,6 +18,7 @@
 #include "PIC16.h"
 #include "PIC16Subtarget.h"
 #include "llvm/CodeGen/SelectionDAG.h"
+#include "llvm/CodeGen/SelectionDAGISel.h"
 #include "llvm/Target/TargetLowering.h"
 #include <map>
 
@@ -216,7 +217,9 @@ namespace llvm {
 
     // This function checks if we need to put an operand of an operation on
     // stack and generate a load or not.
-    bool NeedToConvertToMemOp(SDValue Op, unsigned &MemOp); 
+    // DAG parameter is required to access DAG information during
+    // analysis.
+    bool NeedToConvertToMemOp(SDValue Op, unsigned &MemOp, SelectionDAG &DAG); 
 
     /// Subtarget - Keep a pointer to the PIC16Subtarget around so that we can
     /// make the right decision when generating code for different targets.
@@ -239,6 +242,11 @@ namespace llvm {
     // Check if operation has a direct load operand.
     inline bool isDirectLoad(const SDValue Op);
 
+  public:
+    // Keep a pointer to SelectionDAGISel to access its public 
+    // interface (It is required during legalization)
+    SelectionDAGISel   *ISel;
+
   private:
     // The frameindexes generated for spill/reload are stack based.
     // This maps maintain zero based indexes for these FIs.

test/CodeGen/PIC16/C16-49.ll

@@ -0,0 +1,15 @@
+;RUN: llvm-as < %s | llc -march=pic16
+
+@aa = global i16 55, align 1                      ; <i16*> [#uses=1]
+@bb = global i16 44, align 1                      ; <i16*> [#uses=1]
+@PORTD = external global i8                       ; <i8*> [#uses=1]
+
+define void @foo() nounwind {
+entry:
+  %tmp = volatile load i16* @aa                   ; <i16> [#uses=1]
+  %tmp1 = volatile load i16* @bb                  ; <i16> [#uses=1]
+  %sub = sub i16 %tmp, %tmp1                      ; <i16> [#uses=1]
+  %conv = trunc i16 %sub to i8                    ; <i8> [#uses=1]
+  store i8 %conv, i8* @PORTD
+  ret void
+}