[PATCH] DAGCombiner: Continue combining if FoldConstantArithmetic() fails.

[Matthias Braun]

In http://reviews.llvm.org/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...


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D6946

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/