[llvm] r279515 - Fix SystemZ hang caused by r279105
Elliot Colp via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 23 07:03:02 PDT 2016
Author: colpell
Date: Tue Aug 23 09:03:02 2016
New Revision: 279515
URL: http://llvm.org/viewvc/llvm-project?rev=279515&view=rev
Log:
Fix SystemZ hang caused by r279105
The change in r279105 causes an infinite loop in some cases, as it sets the upper bits of an AND mask constant, which DAGCombiner::SimplifyDemandedBits then unsets.
This patch reverts that part of the behaviour, instead relying on .td peepholes to perform the transformation to NILL. I reapplied my original fix for the problem addressed by r279105 (unsetting the upper bits, which prevents a compiler abort for a different reason).
Differential Revision: https://reviews.llvm.org/D23781
Modified:
llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp
llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td
Modified: llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp?rev=279515&r1=279514&r2=279515&view=diff
==============================================================================
--- llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/SystemZ/SystemZISelLowering.cpp Tue Aug 23 09:03:02 2016
@@ -5095,21 +5095,21 @@ SDValue SystemZTargetLowering::combineSH
// value that has its last 6 bits set, we can safely remove the AND operation.
//
// If the AND operation doesn't have the last 6 bits set, we can't remove it
- // entirely, but we can still truncate it to a 16-bit value with all other
- // bits set. This will allow us to generate a NILL instead of a NILF for
- // smaller code size.
+ // entirely, but we can still truncate it to a 16-bit value. This prevents
+ // us from ending up with a NILL with a signed operand, which will cause the
+ // instruction printer to abort.
SDValue N1 = N->getOperand(1);
if (N1.getOpcode() == ISD::AND) {
- SDValue AndOp = N1->getOperand(0);
SDValue AndMaskOp = N1->getOperand(1);
auto *AndMask = dyn_cast<ConstantSDNode>(AndMaskOp);
// The AND mask is constant
if (AndMask) {
- uint64_t AmtVal = AndMask->getZExtValue();
-
+ auto AmtVal = AndMask->getZExtValue();
+
// Bottom 6 bits are set
if ((AmtVal & 0x3f) == 0x3f) {
+ SDValue AndOp = N1->getOperand(0);
// This is the only use, so remove the node
if (N1.hasOneUse()) {
@@ -5129,30 +5129,25 @@ SDValue SystemZTargetLowering::combineSH
return Replace;
}
- // We can't remove the AND, but we can use NILL here instead of NILF if we
- // truncate the mask to 16 bits and set the remaining bits
- } else {
- unsigned BitWidth = AndMask->getAPIntValue().getBitWidth();
-
- // All bits for the operand's size except the lower 16
- uint64_t UpperBits = ((1ull << (uint64_t)BitWidth) - 1ull) &
- 0xffffffffffff0000ull;
-
- if ((AmtVal & UpperBits) != UpperBits) {
- auto NewMaskValue = (AmtVal & 0xffff) | UpperBits;
-
- auto NewMask = DAG.getConstant(NewMaskValue,
- SDLoc(AndMaskOp),
- AndMaskOp.getValueType());
- auto NewAnd = DAG.getNode(N1.getOpcode(), SDLoc(N1), N1.getValueType(),
- AndOp, NewMask);
- auto Replace = DAG.getNode(N->getOpcode(), SDLoc(N),
- N->getValueType(0), N->getOperand(0),
- NewAnd);
- DCI.AddToWorklist(Replace.getNode());
+ // We can't remove the AND, but we can use NILL here (normally we would
+ // use NILF). Only keep the last 16 bits of the mask. The actual
+ // transformation will be handled by .td definitions.
+ } else if (AmtVal >> 16 != 0) {
+ SDValue AndOp = N1->getOperand(0);
+
+ auto NewMask = DAG.getConstant(AndMask->getZExtValue() & 0x0000ffff,
+ SDLoc(AndMaskOp),
+ AndMaskOp.getValueType());
+
+ auto NewAnd = DAG.getNode(N1.getOpcode(), SDLoc(N1), N1.getValueType(),
+ AndOp, NewMask);
+
+ SDValue Replace = DAG.getNode(N->getOpcode(), SDLoc(N),
+ N->getValueType(0), N->getOperand(0),
+ NewAnd);
+ DCI.AddToWorklist(Replace.getNode());
- return Replace;
- }
+ return Replace;
}
}
}
Modified: llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td?rev=279515&r1=279514&r2=279515&view=diff
==============================================================================
--- llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td (original)
+++ llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td Tue Aug 23 09:03:02 2016
@@ -1834,6 +1834,37 @@ def : Pat<(sra (shl (i64 (anyext (i32 (z
def : Pat<(and (xor GR64:$x, (i64 -1)), GR64:$y),
(XGR GR64:$y, (NGR GR64:$y, GR64:$x))>;
+// Shift/rotate instructions only use the last 6 bits of the second operand
+// register, so we can safely use NILL (16 fewer bits than NILF) to only AND the
+// last 16 bits.
+// Complexity is added so that we match this before we match NILF on the AND
+// operation alone.
+let AddedComplexity = 4 in {
+ def : Pat<(shl GR32:$val, (and GR32:$shift, uimm32:$imm)),
+ (SLL GR32:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+
+ def : Pat<(sra GR32:$val, (and GR32:$shift, uimm32:$imm)),
+ (SRA GR32:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+
+ def : Pat<(srl GR32:$val, (and GR32:$shift, uimm32:$imm)),
+ (SRL GR32:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+
+ def : Pat<(shl GR64:$val, (and GR32:$shift, uimm32:$imm)),
+ (SLLG GR64:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+
+ def : Pat<(sra GR64:$val, (and GR32:$shift, uimm32:$imm)),
+ (SRAG GR64:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+
+ def : Pat<(srl GR64:$val, (and GR32:$shift, uimm32:$imm)),
+ (SRLG GR64:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+
+ def : Pat<(rotl GR32:$val, (and GR32:$shift, uimm32:$imm)),
+ (RLL GR32:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+
+ def : Pat<(rotl GR64:$val, (and GR32:$shift, uimm32:$imm)),
+ (RLLG GR64:$val, (NILL GR32:$shift, uimm32:$imm), 0)>;
+}
+
// Peepholes for turning scalar operations into block operations.
defm : BlockLoadStore<anyextloadi8, i32, MVCSequence, NCSequence, OCSequence,
XCSequence, 1>;
More information about the llvm-commits
mailing list