[llvm] r251785 - [PatternMatch] Switch to use ValueTracking::matchSelectPattern
James Molloy via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 4 00:32:37 PST 2015
+David, for his thoughts.
In the meantime I'll back this out until we reach consensus.
Cheers,
James
On Wed, 4 Nov 2015 at 01:31 Richard Smith via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> 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.
>>>
>>>
>> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151104/92e7fc24/attachment.html>
More information about the llvm-commits
mailing list