DAGCombiner: Continue combining if FoldConstantArithmetic() fails.

[MatzeB]

Summary

DAG.FoldConstantArithmetic() can fail even though both operands are
Constants if OpaqueConstants are involved. Continue trying other combine
possibilities in this case.

[MatzeB]

Is there a way to create a proper llvm-lit test for this? I assume relying on -debug-only=isel in the RUN line is not a good idea.
The test in http://reviews.llvm.org/D6940 works but it is more of an accident that aarch64 fails on non properly combined code...

[ab]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

1632

How about:

if (SDValue Folded = ...)
  return Folded;

Or going further, you could remove the N0C && N1C check, since FoldConstantArithmetic also does it (a sliver more wasteful though).

[MatzeB]

Simplify by using condition variables.

[ab]

LGTM.
I'm okay with no test if it's tricky to expose, but maybe we can: what happens now on that AArch64 testcase? Specifically, is there a difference between just this patch, and this patch + the assert fix in D6940?

-Ahmed

[MatzeB]

New version: Turns out if you continue combining in the presence of OpaqueConstants you have to be very carefull to not accidentally fold them into non-Opaque ones during a combine. This patch carefully finds these cases now (I basically looked at all places that create new target constants and checked whether they would be harmfully replace OpaqueConstants with new ones). I did extensive x86 test-suite runs and couldn't find any performance differences.

[MatzeB]

ping

[ab]

Peppering "isOpaque" checks in the remaining combines seems awfully brittle.

Basically, folding to constants isn't OK, but folding to undef is, correct? How about flipping the check then: do all the combines that are valid on Opaque constants first (-> undef), and then bail out if the constant is Opaque but we couldn't do anything.

[MatzeB]

I think the rule here is: "Don't fold opaque constants into another expensive (according to TargetTransformInfo::getIntImmConst) Constant.
It's not a correctness issue but it diminishes the effects of the ConstantHoisting pass.

It is indeed very brittle, but I am not convinced that reordering the optimization rules is the way to go.
There are many other reasons on how the code/checks in a combiner should be order: What makes sense for a human reader or ordering the conditions in a way that the ones that are more likely to fail come first.
Letting a minor detail like Opaque Constants dictate the ordering feels very wrong to me. If we want to push hard to avoid accidentaly transforming OpaqueConstants back to normal ones then the way to go would be not using ISD::Constant for them anymore but to introduce a new opcode, so people need to explicitely check for them before transforming them. In the current design of Opaque being an attribute to ISD::Constant sprinkling the code with isOpaque() is a logical consequence IMO... It's not like drastic things happen we forget some isOpaque() checks anyway, it's not a correctness issue just a minor performance thing.

• Matthias

[MatzeB]

2nd ping

[ab]

Fair enough, reordering the optimizations isn't a solution. Looking through old commits, I see there are a few that do exactly this: add isOpaque checks around DAG combines.
Having a separate node type for opaque constants does sound less brittle; any idea of what would be involved?

-Ahmed

[MatzeB]

That would obviously involve changing all targets to recognize that new node type, in all sorts of different addressing modes. I also don't know whether all of this was already discussed when OpaqueConstants were introduced. Anyway I just wanted to fix a few bugs here and not rewrite all this stuff, so this patch won't get a LGTM unless it's a rewrite of the whole OpaqueConstants concepts?

• Matthias

[ab]

I think there are two parts here:

• checking the return value of FoldConstantArithmetic: that LGTM, though tests would of course be appreciated, if possible.
• disabling some optimizations on Opaque constants: that, I'll let Juergen comment on. I'm opposed to it because of the brittleness, but pragmatically (and there's precedent) that's a reasonable solution.

For your assert failure, I'm fine with just fixing that for now (that would be r230355, correct?).

-Ahmed

[MatzeB]

I can't commit the first part without the second because it leads to the DAGCombiner folding many opaque constants back to normal constants breaking lit tests and generally defeating the intention of ContantHoisting pass.

• Matthias

[MatzeB]

ping

[MatzeB]

ping

[MatzeB]

ping 3

[RKSimon]

Would it be tidier to set the N0C / N1C values to null if they are opaque instead of adding isOpaque() tests everywhere?

[hfinkel]

Can you please upload this patch with full context (see http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface for instructions).

I think the rule here is: "Don't fold opaque constants into another expensive (according to TargetTransformInfo::getIntImmConst) Constant.
It's not a correctness issue but it diminishes the effects of the ConstantHoisting pass.

That does not seem right. Am I right that we only expect isOpaque to be true on TargetConstants, not regular Constants? My understanding is that opaque TargetConstants are a correctness issue (it is not really a cost issue, but rather that sometimes we can't change the constant because it has a special meaning, a larger constant might not be representable for that operation (like the offset on an indexed load or store), etc.).

FWIW, I feel like our current handling of isOpaque is laughably sparse, and we should either complete the job (by auditing, as you've done), or rip it out. Perhaps instead we should just consider all TargetConstants opaque.

That having been said, I'd like to add some additional safety here. Such as, in ConstantSDNode:

const ConstantInt *getConstantIntValue() const { return Value; }

should become:

const ConstantInt *getConstantIntValue(bool allowOpaque = false) const {
  assert((allowOpaque || !isOpaque()) && "Invalid access to opaque value");
  return Value;
}

and likewise for the other interfaces. This way, by default, you can't even get an opaque value's numeric representation.

[MatzeB]

In D6946#174531, @hfinkel wrote:

Can you please upload this patch with full context (see http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface for instructions).

I think the rule here is: "Don't fold opaque constants into another expensive (according to TargetTransformInfo::getIntImmConst) Constant.
It's not a correctness issue but it diminishes the effects of the ConstantHoisting pass.

That does not seem right. Am I right that we only expect isOpaque to be true on TargetConstants, not regular Constants? My understanding is that opaque TargetConstants are a correctness issue (it is not really a cost issue, but rather that sometimes we can't change the constant because it has a special meaning, a larger constant might not be representable for that operation (like the offset on an indexed load or store), etc.).

No, isOpaque is not about TargetConstants but normal ConstantSDNodes which are equivalent to IR Constants. Opaque constants are created in IR by Transforms/Scalar/ConstantHoisting.cpp with the convention that a Constant that is immediately bitcasted to the same type is an opaque constant. The trick here is not to undo the work from that pass while at the same time folding as much as possible...

[hfinkel]

In D6946#174583, @MatzeB wrote:

In D6946#174531, @hfinkel wrote:

Can you please upload this patch with full context (see http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface for instructions).

I think the rule here is: "Don't fold opaque constants into another expensive (according to TargetTransformInfo::getIntImmConst) Constant.
It's not a correctness issue but it diminishes the effects of the ConstantHoisting pass.

That does not seem right. Am I right that we only expect isOpaque to be true on TargetConstants, not regular Constants? My understanding is that opaque TargetConstants are a correctness issue (it is not really a cost issue, but rather that sometimes we can't change the constant because it has a special meaning, a larger constant might not be representable for that operation (like the offset on an indexed load or store), etc.).

No, isOpaque is not about TargetConstants but normal ConstantSDNodes which are equivalent to IR Constants. Opaque constants are created in IR by Transforms/Scalar/ConstantHoisting.cpp with the convention that a Constant that is immediately bitcasted to the same type is an opaque constant. The trick here is not to undo the work from that pass while at the same time folding as much as possible...

Ah, indeed. Shouldn't the rule be: Don't fold an opaque constant into another constant unless he result is cheap or free (in the getIntImmConst sense)?

[MatzeB]

Ah, indeed. Shouldn't the rule be: Don't fold an opaque constant into another constant unless he result is cheap or free (in the getIntImmConst sense)?

Yes. In fact I did not bother to guard a few combines that produce a zero result, though admittedly the code does not check whether zero is an expensive constant - but I can't imagine that to be the case on any target.

Anyway I'm currently adapting the patch to ToT and will try out RKSimons suggestion which will probably simplify this a lot.

[MatzeB]

New revision adapted to ToT and with many explicite isOpaque() checks avoided by introducing a cast operation that returns nullptr if a node is a non-opaque constant.

[hfinkel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

1600

Our general convention seems to be to name functions like this 'getAs<Name>', not 'as<Name>', so I think getAsNonOpaqueConstant would be better.

Otherwise, LGTM.

[MatzeB]

This landed in r237822. Closing manually as "Differential Revision: XXX" was not the last line of the commit message.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

static bool isAllOnesConstant(SDValue V) {
   return Const != nullptr && Const->isAllOnesValue();
 }
 
 static bool isOneConstant(SDValue V) {
   ConstantSDNode *Const = dyn_cast<ConstantSDNode>(V);
   return Const != nullptr && Const->isOne();
 }
 
+/// If \p N is a ContantSDNode with isOpaque() == false return it casted to a
+/// ContantSDNode pointer else nullptr.
+static ConstantSDNode *asNonOpaqueConstant(SDValue N) {
+  ConstantSDNode *Const = dyn_cast<ConstantSDNode>(N);
+  return Const != nullptr && !Const->isOpaque() ? Const : nullptr;
+}
+
 SDValue DAGCombiner::visitADD(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N0.getValueType();
 
   // fold vector ops
   if (VT.isVector()) {
     if (SDValue FoldedVOp = SimplifyVBinOp(N))
       return FoldedVOp;
 
     // fold (add x, 0) -> x, vector edition
     if (ISD::isBuildVectorAllZeros(N1.getNode()))
       return N0;
     if (ISD::isBuildVectorAllZeros(N0.getNode()))
       return N1;
   }
 
   // fold (add x, undef) -> undef
   if (N0.getOpcode() == ISD::UNDEF)
     return N0;
   if (N1.getOpcode() == ISD::UNDEF)
     return N1;
   // fold (add c1, c2) -> c1+c2
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
-  ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
+  ConstantSDNode *N1C = asNonOpaqueConstant(N1);
   if (N0C && N1C)
     return DAG.FoldConstantArithmetic(ISD::ADD, SDLoc(N), VT, N0C, N1C);
   // canonicalize constant to RHS
   if (isConstantIntBuildVectorOrConstantInt(N0) &&
      !isConstantIntBuildVectorOrConstantInt(N1))
     return DAG.getNode(ISD::ADD, SDLoc(N), VT, N1, N0);
   // fold (add x, 0) -> x
   if (isNullConstant(N1))
     return N0;
   // fold (add Sym, c) -> Sym+c
   if (GlobalAddressSDNode *GA = dyn_cast<GlobalAddressSDNode>(N0))
     if (!LegalOperations && TLI.isOffsetFoldingLegal(GA) && N1C &&
         GA->getOpcode() == ISD::GlobalAddress)
       return DAG.getGlobalAddress(GA->getGlobal(), SDLoc(N1C), VT,
                                   GA->getOffset() +
                                     (uint64_t)N1C->getSExtValue());
   // fold ((c1-A)+c2) -> (c1+c2)-A
   if (N1C && N0.getOpcode() == ISD::SUB)
-    if (ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0.getOperand(0))) {
+    if (ConstantSDNode *N0C = asNonOpaqueConstant(N0.getOperand(0))) {
       SDLoc DL(N);
       return DAG.getNode(ISD::SUB, DL, VT,
                          DAG.getConstant(N1C->getAPIntValue()+
                                          N0C->getAPIntValue(), DL, VT),
                          N0.getOperand(1));
     }
   // reassociate add
   if (SDValue RADD = ReassociateOps(ISD::ADD, SDLoc(N), N0, N1))
if (ISD::isBuildVectorAllZeros(N1.getNode()))
       return N0;
   }
 
   // fold (sub x, x) -> 0
   // FIXME: Refactor this and xor and other similar operations together.
   if (N0 == N1)
     return tryFoldToZero(SDLoc(N), TLI, VT, DAG, LegalOperations, LegalTypes);
   // fold (sub c1, c2) -> c1-c2
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0.getNode());
-  ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1.getNode());
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
+  ConstantSDNode *N1C = asNonOpaqueConstant(N1);
   if (N0C && N1C)
     return DAG.FoldConstantArithmetic(ISD::SUB, SDLoc(N), VT, N0C, N1C);
   // fold (sub x, c) -> (add x, -c)
   if (N1C) {
     SDLoc DL(N);
     return DAG.getNode(ISD::ADD, DL, VT, N0,
                        DAG.getConstant(-N1C->getAPIntValue(), DL, VT));
   }
SDValue DAGCombiner::visitMUL(SDNode *N) {
   EVT VT = N0.getValueType();
 
   // fold (mul x, undef) -> 0
   if (N0.getOpcode() == ISD::UNDEF || N1.getOpcode() == ISD::UNDEF)
     return DAG.getConstant(0, SDLoc(N), VT);
 
   bool N0IsConst = false;
   bool N1IsConst = false;
+  bool N1IsOpaqueConst = false;
+  bool N0IsOpaqueConst = false;
   APInt ConstValue0, ConstValue1;
   // fold vector ops
   if (VT.isVector()) {
     if (SDValue FoldedVOp = SimplifyVBinOp(N))
       return FoldedVOp;
 
     N0IsConst = isConstantSplatVector(N0.getNode(), ConstValue0);
     N1IsConst = isConstantSplatVector(N1.getNode(), ConstValue1);
   } else {
     N0IsConst = isa<ConstantSDNode>(N0);
-    if (N0IsConst)
+    if (N0IsConst) {
       ConstValue0 = cast<ConstantSDNode>(N0)->getAPIntValue();
+      N0IsOpaqueConst = cast<ConstantSDNode>(N0)->isOpaque();
+    }
     N1IsConst = isa<ConstantSDNode>(N1);
-    if (N1IsConst)
+    if (N1IsConst) {
       ConstValue1 = cast<ConstantSDNode>(N1)->getAPIntValue();
+      N1IsOpaqueConst = cast<ConstantSDNode>(N1)->isOpaque();
+    }
   }
 
   // fold (mul c1, c2) -> c1*c2
-  if (N0IsConst && N1IsConst)
+  if (N0IsConst && N1IsConst && !N0IsOpaqueConst && !N1IsOpaqueConst)
     return DAG.FoldConstantArithmetic(ISD::MUL, SDLoc(N), VT,
                                       N0.getNode(), N1.getNode());
 
   // canonicalize constant to RHS (vector doesn't have to splat)
   if (isConstantIntBuildVectorOrConstantInt(N0) &&
      !isConstantIntBuildVectorOrConstantInt(N1))
     return DAG.getNode(ISD::MUL, SDLoc(N), VT, N1, N0);
   // fold (mul x, 0) -> 0
   if (N1IsConst && ConstValue1 == 0)
     return N1;
   // We require a splat of the entire scalar bit width for non-contiguous
   // bit patterns.
   bool IsFullSplat =
     ConstValue1.getBitWidth() == VT.getScalarType().getSizeInBits();
   // fold (mul x, 1) -> x
   if (N1IsConst && ConstValue1 == 1 && IsFullSplat)
     return N0;
   // fold (mul x, -1) -> 0-x
   if (N1IsConst && ConstValue1.isAllOnesValue()) {
     SDLoc DL(N);
     return DAG.getNode(ISD::SUB, DL, VT,
                        DAG.getConstant(0, DL, VT), N0);
   }
   // fold (mul x, (1 << c)) -> x << c
-  if (N1IsConst && ConstValue1.isPowerOf2() && IsFullSplat) {
+  if (N1IsConst && !N1IsOpaqueConst && ConstValue1.isPowerOf2() &&
+      IsFullSplat) {
     SDLoc DL(N);
     return DAG.getNode(ISD::SHL, DL, VT, N0,
                        DAG.getConstant(ConstValue1.logBase2(), DL,
                                        getShiftAmountTy(N0.getValueType())));
   }
   // fold (mul x, -(1 << c)) -> -(x << c) or (-x) << c
-  if (N1IsConst && (-ConstValue1).isPowerOf2() && IsFullSplat) {
+  if (N1IsConst && !N1IsOpaqueConst && (-ConstValue1).isPowerOf2() &&
+      IsFullSplat) {
     unsigned Log2Val = (-ConstValue1).logBase2();
     SDLoc DL(N);
     // FIXME: If the input is something that is easily negated (e.g. a
     // single-use add), we should put the negate there.
     return DAG.getNode(ISD::SUB, DL, VT,
                        DAG.getConstant(0, DL, VT),
                        DAG.getNode(ISD::SHL, DL, VT, N0,
                             DAG.getConstant(Log2Val, DL,
SDValue DAGCombiner::visitSDIV(SDNode *N) {
   // fold vector ops
   if (VT.isVector())
     if (SDValue FoldedVOp = SimplifyVBinOp(N))
       return FoldedVOp;
 
   // fold (sdiv c1, c2) -> c1/c2
   ConstantSDNode *N0C = isConstOrConstSplat(N0);
   ConstantSDNode *N1C = isConstOrConstSplat(N1);
-  if (N0C && N1C && !N1C->isNullValue())
+  if (N0C && N1C && !N0C->isOpaque() && !N1C->isOpaque())
     return DAG.FoldConstantArithmetic(ISD::SDIV, SDLoc(N), VT, N0C, N1C);
   // fold (sdiv X, 1) -> X
   if (N1C && N1C->isOne())
     return N0;
   // fold (sdiv X, -1) -> 0-X
   if (N1C && N1C->isAllOnesValue()) {
     SDLoc DL(N);
     return DAG.getNode(ISD::SUB, DL, VT,
                        DAG.getConstant(0, DL, VT), N0);
   }
   // If we know the sign bits of both operands are zero, strength reduce to a
   // udiv instead.  Handles (X&15) /s 4 -> X&15 >> 2
   if (!VT.isVector()) {
     if (DAG.SignBitIsZero(N1) && DAG.SignBitIsZero(N0))
       return DAG.getNode(ISD::UDIV, SDLoc(N), N1.getValueType(),
                          N0, N1);
   }
 
   // fold (sdiv X, pow2) -> simple ops after legalize
-  if (N1C && !N1C->isNullValue() && (N1C->getAPIntValue().isPowerOf2() ||
-                                     (-N1C->getAPIntValue()).isPowerOf2())) {
+  if (N1C && !N1C->isNullValue() && !N1C->isOpaque() &&
+      (N1C->getAPIntValue().isPowerOf2() ||
     // If dividing by powers of two is cheap, then don't perform the following
     // fold.
     if (TLI.isPow2SDivCheap())
       return SDValue();
 
     // Target-specific implementation of sdiv x, pow2.
     SDValue Res = BuildSDIVPow2(N);
     if (Res.getNode())
SDValue DAGCombiner::visitUDIV(SDNode *N) {
   // fold vector ops
   if (VT.isVector())
     if (SDValue FoldedVOp = SimplifyVBinOp(N))
       return FoldedVOp;
 
   // fold (udiv c1, c2) -> c1/c2
   ConstantSDNode *N0C = isConstOrConstSplat(N0);
   ConstantSDNode *N1C = isConstOrConstSplat(N1);
-  if (N0C && N1C && !N1C->isNullValue())
-    return DAG.FoldConstantArithmetic(ISD::UDIV, SDLoc(N), VT, N0C, N1C);
+  if (N0C && N1C)
+    if (SDValue Folded = DAG.FoldConstantArithmetic(ISD::UDIV, SDLoc(N), VT,
+                                                    N0C, N1C))
+      return Folded;
   // fold (udiv x, (1 << c)) -> x >>u c
-  if (N1C && N1C->getAPIntValue().isPowerOf2()) {
+  if (N1C && !N1C->isOpaque() && N1C->getAPIntValue().isPowerOf2()) {
     SDLoc DL(N);
     return DAG.getNode(ISD::SRL, DL, VT, N0,
                        DAG.getConstant(N1C->getAPIntValue().logBase2(), DL,
                                        getShiftAmountTy(N0.getValueType())));
   }
   // fold (udiv x, (shl c, y)) -> x >>u (log2(c)+y) iff c is power of 2
   if (N1.getOpcode() == ISD::SHL) {
-    if (ConstantSDNode *SHC = dyn_cast<ConstantSDNode>(N1.getOperand(0))) {
+    if (ConstantSDNode *SHC = asNonOpaqueConstant(N1.getOperand(0))) {
       if (SHC->getAPIntValue().isPowerOf2()) {
         EVT ADDVT = N1.getOperand(1).getValueType();
         SDLoc DL(N);
         SDValue Add = DAG.getNode(ISD::ADD, DL, ADDVT,
                                   N1.getOperand(1),
                                   DAG.getConstant(SHC->getAPIntValue()
                                                                   .logBase2(),
                                                   DL, ADDVT));

 SDValue DAGCombiner::visitSREM(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N->getValueType(0);
 
   // fold (srem c1, c2) -> c1%c2
   ConstantSDNode *N0C = isConstOrConstSplat(N0);
   ConstantSDNode *N1C = isConstOrConstSplat(N1);
-  if (N0C && N1C && !N1C->isNullValue())
-    return DAG.FoldConstantArithmetic(ISD::SREM, SDLoc(N), VT, N0C, N1C);
+  if (N0C && N1C)
+    if (SDValue Folded = DAG.FoldConstantArithmetic(ISD::SREM, SDLoc(N), VT,
+                                                    N0C, N1C))
+      return Folded;
   // If we know the sign bits of both operands are zero, strength reduce to a
   // urem instead.  Handles (X & 0x0FFFFFFF) %s 16 -> X&15
   if (!VT.isVector()) {
     if (DAG.SignBitIsZero(N1) && DAG.SignBitIsZero(N0))
       return DAG.getNode(ISD::UREM, SDLoc(N), VT, N0, N1);
   }
 
   // If X/C can be simplified by the division-by-constant logic, lower

 SDValue DAGCombiner::visitUREM(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N->getValueType(0);
 
   // fold (urem c1, c2) -> c1%c2
   ConstantSDNode *N0C = isConstOrConstSplat(N0);
   ConstantSDNode *N1C = isConstOrConstSplat(N1);
-  if (N0C && N1C && !N1C->isNullValue())
-    return DAG.FoldConstantArithmetic(ISD::UREM, SDLoc(N), VT, N0C, N1C);
+  if (N0C && N1C)
+    if (SDValue Folded = DAG.FoldConstantArithmetic(ISD::UREM, SDLoc(N), VT,
+                                                    N0C, N1C))
+      return Folded;
   // fold (urem x, pow2) -> (and x, pow2-1)
-  if (N1C && !N1C->isNullValue() && N1C->getAPIntValue().isPowerOf2()) {
+  if (N1C && !N1C->isNullValue() && !N1C->isOpaque() &&
+      N1C->getAPIntValue().isPowerOf2()) {
     SDLoc DL(N);
     return DAG.getNode(ISD::AND, DL, VT, N0,
                        DAG.getConstant(N1C->getAPIntValue() - 1, DL, VT));
   }
   // fold (urem x, (shl pow2, y)) -> (and x, (add (shl pow2, y), -1))
   if (N1.getOpcode() == ISD::SHL) {
-    if (ConstantSDNode *SHC = dyn_cast<ConstantSDNode>(N1.getOperand(0))) {
+    if (ConstantSDNode *SHC = asNonOpaqueConstant(N1.getOperand(0))) {
       if (SHC->getAPIntValue().isPowerOf2()) {
         SDLoc DL(N);
         SDValue Add =
           DAG.getNode(ISD::ADD, DL, VT, N1,
                  DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), DL,
                                  VT));
         AddToWorklist(Add.getNode());
         return DAG.getNode(ISD::AND, DL, VT, N0, Add);
if (VT.isVector()) {
     // fold (and x, -1) -> x, vector edition
     if (ISD::isBuildVectorAllOnes(N0.getNode()))
       return N1;
     if (ISD::isBuildVectorAllOnes(N1.getNode()))
       return N0;
   }
 
   // fold (and c1, c2) -> c1&c2
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
   ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
-  if (N0C && N1C)
+  if (N0C && N1C && !N1C->isOpaque())
     return DAG.FoldConstantArithmetic(ISD::AND, SDLoc(N), VT, N0C, N1C);
   // canonicalize constant to RHS
   if (isConstantIntBuildVectorOrConstantInt(N0) &&
      !isConstantIntBuildVectorOrConstantInt(N1))
     return DAG.getNode(ISD::AND, SDLoc(N), VT, N1, N0);
   // fold (and x, -1) -> x
   if (isAllOnesConstant(N1))
     return N0;
if (LL == RL && LR == RR) {
             TLI.isOperationLegal(ISD::SETCC,
               getSetCCResultType(N0.getValueType())))))
         return DAG.getSetCC(SDLoc(LocReference), N0.getValueType(),
                             LL, LR, Result);
     }
   }
 
   // (or (and X, C1), (and Y, C2))  -> (and (or X, Y), C3) if possible.
-  if (N0.getOpcode() == ISD::AND &&
-      N1.getOpcode() == ISD::AND &&
-      N0.getOperand(1).getOpcode() == ISD::Constant &&
-      N1.getOperand(1).getOpcode() == ISD::Constant &&
+  if (N0.getOpcode() == ISD::AND && N1.getOpcode() == ISD::AND &&
       // Don't increase # computations.
       (N0.getNode()->hasOneUse() || N1.getNode()->hasOneUse())) {
     // We can only do this xform if we know that bits from X that are set in C2
     // but not in C1 are already zero.  Likewise for Y.
-    const APInt &LHSMask =
-      cast<ConstantSDNode>(N0.getOperand(1))->getAPIntValue();
-    const APInt &RHSMask =
-      cast<ConstantSDNode>(N1.getOperand(1))->getAPIntValue();
+    if (const ConstantSDNode *N0O1C = asNonOpaqueConstant(N0.getOperand(1))) {
+      if (const ConstantSDNode *N1O1C = asNonOpaqueConstant(N1.getOperand(1))) {
+        // We can only do this xform if we know that bits from X that are set in
+        // C2 but not in C1 are already zero.  Likewise for Y.
+        const APInt &LHSMask = N0O1C->getAPIntValue();
+        const APInt &RHSMask = N1O1C->getAPIntValue();
 
         if (DAG.MaskedValueIsZero(N0.getOperand(0), RHSMask&~LHSMask) &&
             DAG.MaskedValueIsZero(N1.getOperand(0), LHSMask&~RHSMask)) {
           SDValue X = DAG.getNode(ISD::OR, SDLoc(N0), VT,
                                   N0.getOperand(0), N1.getOperand(0));
           SDLoc DL(LocReference);
           return DAG.getNode(ISD::AND, DL, VT, X,
                              DAG.getConstant(LHSMask | RHSMask, DL, VT));
         }
       }
+    }
+  }
 
   // (or (and X, M), (and X, N)) -> (and X, (or M, N))
   if (N0.getOpcode() == ISD::AND &&
       N1.getOpcode() == ISD::AND &&
       N0.getOperand(0) == N1.getOperand(0) &&
       // Don't increase # computations.
       (N0.getNode()->hasOneUse() || N1.getNode()->hasOneUse())) {
     SDValue X = DAG.getNode(ISD::OR, SDLoc(N0), VT,
if (isa<ShuffleVectorSDNode>(N0) &&
         if (TLI.isShuffleMaskLegal(Mask2, VT))
           return DAG.getVectorShuffle(VT, SDLoc(N), N1->getOperand(0),
                                       N0->getOperand(0), &Mask2[0]);
       }
     }
   }
 
   // fold (or c1, c2) -> c1|c2
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
   ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
-  if (N0C && N1C)
+  if (N0C && N1C && !N1C->isOpaque())
     return DAG.FoldConstantArithmetic(ISD::OR, SDLoc(N), VT, N0C, N1C);
   // canonicalize constant to RHS
   if (isConstantIntBuildVectorOrConstantInt(N0) &&
      !isConstantIntBuildVectorOrConstantInt(N1))
     return DAG.getNode(ISD::OR, SDLoc(N), VT, N1, N0);
   // fold (or x, 0) -> x
   if (isNullConstant(N1))
     return N0;
SDValue DAGCombiner::visitXOR(SDNode *N) {
   if (N0.getOpcode() == ISD::UNDEF && N1.getOpcode() == ISD::UNDEF)
     return DAG.getConstant(0, SDLoc(N), VT);
   // fold (xor x, undef) -> undef
   if (N0.getOpcode() == ISD::UNDEF)
     return N0;
   if (N1.getOpcode() == ISD::UNDEF)
     return N1;
   // fold (xor c1, c2) -> c1^c2
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
-  ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
+  ConstantSDNode *N1C = asNonOpaqueConstant(N1);
   if (N0C && N1C)
     return DAG.FoldConstantArithmetic(ISD::XOR, SDLoc(N), VT, N0C, N1C);
   // canonicalize constant to RHS
   if (isConstantIntBuildVectorOrConstantInt(N0) &&
      !isConstantIntBuildVectorOrConstantInt(N1))
     return DAG.getNode(ISD::XOR, SDLoc(N), VT, N1, N0);
   // fold (xor x, 0) -> x
   if (isNullConstant(N1))
if (N0.getOpcode() == ISD::AND && N0.getNode()->hasOneUse() &&
       N0->getOperand(1) == N1) {
     SDValue X = N0->getOperand(0);
     SDValue NotX = DAG.getNOT(SDLoc(X), X, VT);
     AddToWorklist(NotX.getNode());
     return DAG.getNode(ISD::AND, SDLoc(N), VT, NotX, N1);
   }
   // fold (xor (xor x, c1), c2) -> (xor x, (xor c1, c2))
   if (N1C && N0.getOpcode() == ISD::XOR) {
-    ConstantSDNode *N00C = dyn_cast<ConstantSDNode>(N0.getOperand(0));
-    ConstantSDNode *N01C = dyn_cast<ConstantSDNode>(N0.getOperand(1));
-    if (N00C) {
+    if (const ConstantSDNode *N00C = asNonOpaqueConstant(N0.getOperand(0))) {
       SDLoc DL(N);
       return DAG.getNode(ISD::XOR, DL, VT, N0.getOperand(1),
                          DAG.getConstant(N1C->getAPIntValue() ^
                                          N00C->getAPIntValue(), DL, VT));
     }
-    if (N01C) {
+    if (const ConstantSDNode *N01C = asNonOpaqueConstant(N0.getOperand(1))) {
       SDLoc DL(N);
       return DAG.getNode(ISD::XOR, DL, VT, N0.getOperand(0),
                          DAG.getConstant(N1C->getAPIntValue() ^
                                          N01C->getAPIntValue(), DL, VT));
     }
   }
   // fold (xor x, x) -> 0
   if (N0 == N1)
if (!VT.isVector() &&
     return SDValue(N, 0);
 
   return SDValue();
 }
 
 /// Handle transforms common to the three shifts, when the shift amount is a
 /// constant.
 SDValue DAGCombiner::visitShiftByConstant(SDNode *N, ConstantSDNode *Amt) {
-  // We can't and shouldn't fold opaque constants.
-  if (Amt->isOpaque())
-    return SDValue();
-
   SDNode *LHS = N->getOperand(0).getNode();
   if (!LHS->hasOneUse()) return SDValue();
 
   // We want to pull some binops through shifts, so that we have (and (shift))
   // instead of (shift (and)), likewise for add, or, xor, etc.  This sort of
   // thing happens with address calculations, so it's important to canonicalize
   // it.
   bool HighBitSet = false;  // Can we transform this if the high bit is set?
SDValue DAGCombiner::visitShiftByConstant(SDNode *N, ConstantSDNode *Amt) {
   case ISD::ADD:
     if (N->getOpcode() != ISD::SHL)
       return SDValue(); // only shl(add) not sr[al](add).
     HighBitSet = false; // We can only transform sra if the high bit is clear.
     break;
   }
 
   // We require the RHS of the binop to be a constant and not opaque as well.
-  ConstantSDNode *BinOpCst = dyn_cast<ConstantSDNode>(LHS->getOperand(1));
-  if (!BinOpCst || BinOpCst->isOpaque()) return SDValue();
+  ConstantSDNode *BinOpCst = asNonOpaqueConstant(LHS->getOperand(1));
+  if (!BinOpCst) return SDValue();
 
   // FIXME: disable this unless the input to the binop is a shift by a constant.
   // If it is not a shift, it pessimizes some common cases like:
   //
   //    void foo(int *X, int i) { X[i & 1235] = 1; }
   //    int bar(int *X, int i) { return X[i & 255]; }
   SDNode *BinOpLHSVal = LHS->getOperand(0).getNode();
   if ((BinOpLHSVal->getOpcode() != ISD::SHL &&
SDValue DAGCombiner::distributeTruncateThroughAnd(SDNode *N) {
   assert(N->getOpcode() == ISD::TRUNCATE);
   assert(N->getOperand(0).getOpcode() == ISD::AND);
 
   // (truncate:TruncVT (and N00, N01C)) -> (and (truncate:TruncVT N00), TruncC)
   if (N->hasOneUse() && N->getOperand(0).hasOneUse()) {
     SDValue N01 = N->getOperand(0).getOperand(1);
 
     if (ConstantSDNode *N01C = isConstOrConstSplat(N01)) {
+      if (!N01C->isOpaque()) {
         EVT TruncVT = N->getValueType(0);
         SDValue N00 = N->getOperand(0).getOperand(0);
         APInt TruncC = N01C->getAPIntValue();
         TruncC = TruncC.trunc(TruncVT.getScalarSizeInBits());
         SDLoc DL(N);
 
         return DAG.getNode(ISD::AND, DL, TruncVT,
                            DAG.getNode(ISD::TRUNCATE, DL, TruncVT, N00),
                            DAG.getConstant(TruncC, DL, TruncVT));
       }
     }
+  }
 
   return SDValue();
 }
 
 SDValue DAGCombiner::visitRotate(SDNode *N) {
   // fold (rot* x, (trunc (and y, c))) -> (rot* x, (and (trunc y), (trunc c))).
   if (N->getOperand(1).getOpcode() == ISD::TRUNCATE &&
       N->getOperand(1).getOperand(0).getOpcode() == ISD::AND) {
if (N1CV && N1CV->isConstant()) {
         }
       } else {
         N1C = isConstOrConstSplat(N1);
       }
     }
   }
 
   // fold (shl c1, c2) -> c1<<c2
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
-  if (N0C && N1C)
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
+  if (N0C && N1C && !N1C->isOpaque())
     return DAG.FoldConstantArithmetic(ISD::SHL, SDLoc(N), VT, N0C, N1C);
   // fold (shl 0, x) -> 0
   if (isNullConstant(N0))
     return N0;
   // fold (shl x, c >= size(x)) -> undef
   if (N1C && N1C->getZExtValue() >= OpSizeInBits)
     return DAG.getUNDEF(VT);
   // fold (shl x, 0) -> x
SDValue DAGCombiner::visitSHL(SDNode *N) {
   if (N1C && N0.getOpcode() == ISD::ADD && N0.getNode()->hasOneUse() &&
       (isa<ConstantSDNode>(N0.getOperand(1)) ||
        isConstantSplatVector(N0.getOperand(1).getNode(), Val))) {
     SDValue Shl0 = DAG.getNode(ISD::SHL, SDLoc(N0), VT, N0.getOperand(0), N1);
     SDValue Shl1 = DAG.getNode(ISD::SHL, SDLoc(N1), VT, N0.getOperand(1), N1);
     return DAG.getNode(ISD::ADD, SDLoc(N), VT, Shl0, Shl1);
   }
 
-  if (N1C) {
+  if (N1C && !N1C->isOpaque()) {
     SDValue NewSHL = visitShiftByConstant(N, N1C);
     if (NewSHL.getNode())
       return NewSHL;
   }
 
   return SDValue();
 }
 
 SDValue DAGCombiner::visitSRA(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N0.getValueType();
   unsigned OpSizeInBits = VT.getScalarType().getSizeInBits();
 
   // fold vector ops
   ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
   if (VT.isVector()) {
     if (SDValue FoldedVOp = SimplifyVBinOp(N))
       return FoldedVOp;
 
     N1C = isConstOrConstSplat(N1);
   }
 
   // fold (sra c1, c2) -> (sra c1, c2)
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
-  if (N0C && N1C)
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
+  if (N0C && N1C && !N1C->isOpaque())
     return DAG.FoldConstantArithmetic(ISD::SRA, SDLoc(N), VT, N0C, N1C);
   // fold (sra 0, x) -> 0
   if (isNullConstant(N0))
     return N0;
   // fold (sra -1, x) -> -1
   if (isAllOnesConstant(N0))
     return N0;
   // fold (sra x, (setge c, size(x))) -> undef
SDValue DAGCombiner::visitSRA(SDNode *N) {
   if (N1C && SimplifyDemandedBits(SDValue(N, 0)))
     return SDValue(N, 0);
 
 
   // If the sign bit is known to be zero, switch this to a SRL.
   if (DAG.SignBitIsZero(N0))
     return DAG.getNode(ISD::SRL, SDLoc(N), VT, N0, N1);
 
-  if (N1C) {
+  if (N1C && !N1C->isOpaque()) {
     SDValue NewSRA = visitShiftByConstant(N, N1C);
     if (NewSRA.getNode())
       return NewSRA;
   }
 
   return SDValue();
 }
 
 SDValue DAGCombiner::visitSRL(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N0.getValueType();
   unsigned OpSizeInBits = VT.getScalarType().getSizeInBits();
 
   // fold vector ops
   ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
   if (VT.isVector()) {
     if (SDValue FoldedVOp = SimplifyVBinOp(N))
       return FoldedVOp;
 
     N1C = isConstOrConstSplat(N1);
   }
 
   // fold (srl c1, c2) -> c1 >>u c2
-  ConstantSDNode *N0C = dyn_cast<ConstantSDNode>(N0);
-  if (N0C && N1C)
+  ConstantSDNode *N0C = asNonOpaqueConstant(N0);
+  if (N0C && N1C && !N1C->isOpaque())
     return DAG.FoldConstantArithmetic(ISD::SRL, SDLoc(N), VT, N0C, N1C);
   // fold (srl 0, x) -> 0
   if (isNullConstant(N0))
     return N0;
   // fold (srl x, c >= size(x)) -> undef
   if (N1C && N1C->getZExtValue() >= OpSizeInBits)
     return DAG.getUNDEF(VT);
   // fold (srl x, 0) -> x
if (NewOp1.getNode())
       return DAG.getNode(ISD::SRL, SDLoc(N), VT, N0, NewOp1);
   }
 
   // fold operands of srl based on knowledge that the low bits are not
   // demanded.
   if (N1C && SimplifyDemandedBits(SDValue(N, 0)))
     return SDValue(N, 0);
 
-  if (N1C) {
+  if (N1C && !N1C->isOpaque()) {
     SDValue NewSRL = visitShiftByConstant(N, N1C);
     if (NewSRL.getNode())
       return NewSRL;
   }
 
   // Attempt to convert a srl of a load into a narrower zero-extending load.
   SDValue NarrowLoad = ReduceLoadWidth(N);
   if (NarrowLoad.getNode())
if (DAG.MaskedValueIsZero(V.getOperand(0), Mask))
       return V.getOperand(1);
     if (DAG.MaskedValueIsZero(V.getOperand(1), Mask))
       return V.getOperand(0);
     break;
   case ISD::SRL:
     // Only look at single-use SRLs.
     if (!V.getNode()->hasOneUse())
       break;
-    if (ConstantSDNode *RHSC = dyn_cast<ConstantSDNode>(V.getOperand(1))) {
+    if (ConstantSDNode *RHSC = asNonOpaqueConstant(V.getOperand(1))) {
       // See if we can recursively simplify the LHS.
       unsigned Amt = RHSC->getZExtValue();
 
       // Watch out for shift count overflow though.
       if (Amt >= Mask.getBitWidth()) break;
       APInt NewMask = Mask << Amt;
       SDValue SimplifyLHS = GetDemandedBits(V.getOperand(0), NewMask);
       if (SimplifyLHS.getNode())

lib/CodeGen/SelectionDAG/TargetLowering.cpp

bool TargetLowering::SimplifyDemandedBits(SDValue Op,
   default:
     // Just use computeKnownBits to compute output bits.
     TLO.DAG.computeKnownBits(Op, KnownZero, KnownOne, Depth);
     break;
   }
 
   // If we know the value of all of the demanded bits, return this as a
   // constant.
-  if ((NewMask & (KnownZero|KnownOne)) == NewMask)
+  if ((NewMask & (KnownZero|KnownOne)) == NewMask) {
+    // Avoid folding to a constant if any OpaqueConstant is involved.
+    const SDNode *N = Op.getNode();
+    for (SDNodeIterator I = SDNodeIterator::begin(N),
+         E = SDNodeIterator::end(N); I != E; ++I) {
+      SDNode *Op = *I;
+      if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Op))
+        if (C->isOpaque())
+          return false;
+    }
     return TLO.CombineTo(Op,
                          TLO.DAG.getConstant(KnownOne, dl, Op.getValueType()));
+  }
 
   return false;
 }
 
 /// computeKnownBitsForTargetNode - Determine which of the bits specified
 /// in Mask are known to be either zero or one and return them in the
 /// KnownZero/KnownOne bitsets.
 void TargetLowering::computeKnownBitsForTargetNode(const SDValue Op,