[PATCH] D67802: [SelectionDAG][Mips][Sparc] Don't allow SimplifyDemandedBits to constant fold TargetConstant nodes to a Constant.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: arsenm, RKSimon, spatel.
Herald added subscribers: atanasyan, jrtc27, fedor.sergeev, hiraditya, arichardson, wdng, sdardis, jyknight.
Herald added a project: LLVM.

After the switch in SimplifyDemandedBits, it tries to create a
constant when possible. If the original node is a TargetConstant
the default in the switch will call computeKnownBits on the
TargetConstant which will succeed. This results in the
TargetConstant becoming a Constant. But TargetConstant exists to
avoid being changed.

I've fixed the two cases that relied on this in tree by explicitly
making the nodes constant instead of target constant. The Sparc
case is an old bug. The Mips case was recently introduced now that
ImmArg on intrinsics gets turned into a TargetConstant when the
SelectionDAG is created.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67802

Files:
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/Mips/MipsSEISelLowering.cpp
  llvm/lib/Target/Sparc/SparcISelLowering.cpp


Index: llvm/lib/Target/Sparc/SparcISelLowering.cpp
===================================================================
--- llvm/lib/Target/Sparc/SparcISelLowering.cpp
+++ llvm/lib/Target/Sparc/SparcISelLowering.cpp
@@ -2244,7 +2244,7 @@
     return DAG.getNode(SPISD::CMPICC, DL, MVT::Glue, Result, RHS);
   }
   case SPCC::FCC_UL : {
-    SDValue Mask   = DAG.getTargetConstant(1, DL, Result.getValueType());
+    SDValue Mask   = DAG.getConstant(1, DL, Result.getValueType());
     Result = DAG.getNode(ISD::AND, DL, Result.getValueType(), Result, Mask);
     SDValue RHS    = DAG.getTargetConstant(0, DL, Result.getValueType());
     SPCC = SPCC::ICC_NE;
@@ -2277,14 +2277,14 @@
     return DAG.getNode(SPISD::CMPICC, DL, MVT::Glue, Result, RHS);
   }
   case SPCC::FCC_LG :  {
-    SDValue Mask   = DAG.getTargetConstant(3, DL, Result.getValueType());
+    SDValue Mask   = DAG.getConstant(3, DL, Result.getValueType());
     Result = DAG.getNode(ISD::AND, DL, Result.getValueType(), Result, Mask);
     SDValue RHS    = DAG.getTargetConstant(0, DL, Result.getValueType());
     SPCC = SPCC::ICC_NE;
     return DAG.getNode(SPISD::CMPICC, DL, MVT::Glue, Result, RHS);
   }
   case SPCC::FCC_UE : {
-    SDValue Mask   = DAG.getTargetConstant(3, DL, Result.getValueType());
+    SDValue Mask   = DAG.getConstant(3, DL, Result.getValueType());
     Result = DAG.getNode(ISD::AND, DL, Result.getValueType(), Result, Mask);
     SDValue RHS    = DAG.getTargetConstant(0, DL, Result.getValueType());
     SPCC = SPCC::ICC_E;
Index: llvm/lib/Target/Mips/MipsSEISelLowering.cpp
===================================================================
--- llvm/lib/Target/Mips/MipsSEISelLowering.cpp
+++ llvm/lib/Target/Mips/MipsSEISelLowering.cpp
@@ -2295,10 +2295,13 @@
   SDLoc DL(Op);
   SDValue ChainIn = Op->getOperand(0);
   SDValue Address = Op->getOperand(2);
-  SDValue Offset  = Op->getOperand(3);
   EVT ResTy = Op->getValueType(0);
   EVT PtrTy = Address->getValueType(0);
 
+  // Convert from TargetConstant to Constant.
+  SDValue Offset = DAG.getConstant(Op.getConstantOperandAPInt(3), DL,
+                                   Op.getOperand(3).getValueType());
+
   // For N64 addresses have the underlying type MVT::i64. This intrinsic
   // however takes an i32 signed constant offset. The actual type of the
   // intrinsic is a scaled signed i10.
@@ -2370,9 +2373,12 @@
   SDValue ChainIn = Op->getOperand(0);
   SDValue Value   = Op->getOperand(2);
   SDValue Address = Op->getOperand(3);
-  SDValue Offset  = Op->getOperand(4);
   EVT PtrTy = Address->getValueType(0);
 
+  // Convert from TargetConstant to Constant.
+  SDValue Offset = DAG.getConstant(Op.getConstantOperandAPInt(4), DL,
+                                   Op.getOperand(4).getValueType());
+
   // For N64 addresses have the underlying type MVT::i64. This intrinsic
   // however takes an i32 signed constant offset. The actual type of the
   // intrinsic is a scaled signed i10.
Index: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -805,6 +805,8 @@
 
   KnownBits Known2, KnownOut;
   switch (Op.getOpcode()) {
+  case ISD::TargetConstant:
+    llvm_unreachable("Can't simplify this node");
   case ISD::SCALAR_TO_VECTOR: {
     if (!DemandedElts[0])
       return TLO.CombineTo(Op, TLO.DAG.getUNDEF(VT));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67802.220958.patch
Type: text/x-patch
Size: 3468 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190920/b2e8402a/attachment.bin>