[llvm] r251785 - [PatternMatch] Switch to use ValueTracking::matchSelectPattern

Richard Smith via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 3 17:31:08 PST 2015


On Tue, Nov 3, 2015 at 9:27 AM, Vassil Vassilev <vvasilev at cern.ch> wrote:

> Hi James,
> On 03/11/15 01:20, James Molloy wrote:
>
>> Hi Vassil,
>>
>> So it looks like there's a cyclic dependency, but as far as I'm aware IR
>> is allowed to depend on Analysis- that's not a layering violation as far as
>> I know.
>>
>> Does that mean the module map is incorrect?
>>
> I am no expect but the current module map seems to expect layering
> separation between Analysis and IR (they are built as two standalone
> modules). Maybe this could be changed (there is already something like this
> in LLVM_Backend module description)... Richard Smith (adding him in CC)
> should know better ;)


As currently formulated, IR headers are not allowed to include Analysis
headers (the IR layer is lower than the Analysis layer). This is the only
violation of that rule in all of LLVM's headers, and introduces a
dependency cycle between the IR module and the Analysis module (where the
latter obviously depends on the former).

It looks like the right thing to do may be to move matchSelectPattern down
from Analysis to IR (and probably to move it to PatternMatch.h).


>
> Vassil
>
>
>> James
>>
>>
>>
>> On 2 Nov 2015, at 23:34, Vassil Vassilev <vvasilev at cern.ch> wrote:
>>>
>>> Hi,
>>>   It seems this commit broke the modules self host buildbot:
>>> http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/7969
>>> Vassil
>>>
>>>> On 02/11/15 03:54, James Molloy via llvm-commits wrote:
>>>> Author: jamesm
>>>> Date: Mon Nov  2 03:54:00 2015
>>>> New Revision: 251785
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=251785&view=rev
>>>> Log:
>>>> [PatternMatch] Switch to use ValueTracking::matchSelectPattern
>>>>
>>>> Instead of rolling our own min/max matching code (which is notoriously
>>>> hard to get completely right), use ValueTracking's instead.
>>>>
>>>> Modified:
>>>>      llvm/trunk/include/llvm/IR/PatternMatch.h
>>>>
>>>> Modified: llvm/trunk/include/llvm/IR/PatternMatch.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/PatternMatch.h?rev=251785&r1=251784&r2=251785&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/IR/PatternMatch.h (original)
>>>> +++ llvm/trunk/include/llvm/IR/PatternMatch.h Mon Nov  2 03:54:00 2015
>>>> @@ -29,6 +29,7 @@
>>>>   #ifndef LLVM_IR_PATTERNMATCH_H
>>>>   #define LLVM_IR_PATTERNMATCH_H
>>>>   +#include "llvm/Analysis/ValueTracking.h"
>>>>   #include "llvm/IR/CallSite.h"
>>>>   #include "llvm/IR/Constants.h"
>>>>   #include "llvm/IR/Instructions.h"
>>>> @@ -955,85 +956,69 @@ struct MaxMin_match {
>>>>     MaxMin_match(const LHS_t &LHS, const RHS_t &RHS) : L(LHS), R(RHS) {}
>>>>       template <typename OpTy> bool match(OpTy *V) {
>>>> -    // Look for "(x pred y) ? x : y" or "(x pred y) ? y : x".
>>>> -    auto *SI = dyn_cast<SelectInst>(V);
>>>> -    if (!SI)
>>>> -      return false;
>>>> -    auto *Cmp = dyn_cast<CmpInst_t>(SI->getCondition());
>>>> -    if (!Cmp)
>>>> -      return false;
>>>> -    // At this point we have a select conditioned on a comparison.
>>>> Check that
>>>> -    // it is the values returned by the select that are being compared.
>>>> -    Value *TrueVal = SI->getTrueValue();
>>>> -    Value *FalseVal = SI->getFalseValue();
>>>> -    Value *LHS = Cmp->getOperand(0);
>>>> -    Value *RHS = Cmp->getOperand(1);
>>>> -    if ((TrueVal != LHS || FalseVal != RHS) &&
>>>> -        (TrueVal != RHS || FalseVal != LHS))
>>>> -      return false;
>>>> -    typename CmpInst_t::Predicate Pred =
>>>> -        LHS == TrueVal ? Cmp->getPredicate() :
>>>> Cmp->getSwappedPredicate();
>>>> -    // Does "(x pred y) ? x : y" represent the desired max/min
>>>> operation?
>>>> -    if (!Pred_t::match(Pred))
>>>> -      return false;
>>>> -    // It does!  Bind the operands.
>>>> -    return L.match(LHS) && R.match(RHS);
>>>> +    Value *LHS, *RHS;
>>>> +    auto SPR = matchSelectPattern(V, LHS, RHS);
>>>> +    return Pred_t::match(SPR) && L.match(LHS) && R.match(RHS);
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying signed max predicates.
>>>>   struct smax_pred_ty {
>>>> -  static bool match(ICmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::ICMP_SGT || Pred == CmpInst::ICMP_SGE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_SMAX;
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying signed min predicates.
>>>>   struct smin_pred_ty {
>>>> -  static bool match(ICmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::ICMP_SLT || Pred == CmpInst::ICMP_SLE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_SMIN;
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying unsigned max predicates.
>>>>   struct umax_pred_ty {
>>>> -  static bool match(ICmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::ICMP_UGT || Pred == CmpInst::ICMP_UGE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_UMAX;
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying unsigned min predicates.
>>>>   struct umin_pred_ty {
>>>> -  static bool match(ICmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::ICMP_ULT || Pred == CmpInst::ICMP_ULE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_UMIN;
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying ordered max predicates.
>>>>   struct ofmax_pred_ty {
>>>> -  static bool match(FCmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::FCMP_OGT || Pred == CmpInst::FCMP_OGE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_FMAXNUM &&
>>>> +      (SPR.Ordered || SPR.NaNBehavior == SPNB_RETURNS_ANY);
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying ordered min predicates.
>>>>   struct ofmin_pred_ty {
>>>> -  static bool match(FCmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::FCMP_OLT || Pred == CmpInst::FCMP_OLE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_FMINNUM &&
>>>> +      (SPR.Ordered || SPR.NaNBehavior == SPNB_RETURNS_ANY);
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying unordered max predicates.
>>>>   struct ufmax_pred_ty {
>>>> -  static bool match(FCmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::FCMP_UGT || Pred == CmpInst::FCMP_UGE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_FMAXNUM &&
>>>> +      (!SPR.Ordered || SPR.NaNBehavior == SPNB_RETURNS_ANY);
>>>>     }
>>>>   };
>>>>     /// \brief Helper class for identifying unordered min predicates.
>>>>   struct ufmin_pred_ty {
>>>> -  static bool match(FCmpInst::Predicate Pred) {
>>>> -    return Pred == CmpInst::FCMP_ULT || Pred == CmpInst::FCMP_ULE;
>>>> +  static bool match(SelectPatternResult SPR) {
>>>> +    return SPR.Flavor == SPF_FMINNUM &&
>>>> +      (!SPR.Ordered || SPR.NaNBehavior == SPNB_RETURNS_ANY);
>>>>     }
>>>>   };
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>> ________________________________
>>
>> -- IMPORTANT NOTICE: The contents of this email and any attachments are
>> confidential and may also be privileged. If you are not the intended
>> recipient, please notify the sender immediately and do not disclose the
>> contents to any other person, use it for any purpose, or store or copy the
>> information in any medium. Thank you.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151103/cc4b3fc1/attachment.html>


More information about the llvm-commits mailing list