[llvm-commits] Patch: Pattern matching for intrinsics

[Chandler Carruth]

On Wed, Dec 12, 2012 at 11:16 AM, Michael Ilseman <milseman at apple.com>wrote:

> Here's some new patches. There's a little bit of dirtiness due to not
> having variadic templates, but for the most part it's much better. I also
> go rid of the artificial duplication and classification from before. Thanks
> for all the suggestions, this is looking cleaner now!
>

Awesome!


> Patch 1: m_CombineOr and m_CombineAnd pattern combinators
>

Patch looks good... Some day, I would love to have m_Any and m_All instead
(and they be variadic) but yea, too gross w/o variadics...


> Patch 2: Pattern matching code for intrinsic
>

Looking at this one now...


>
> Provides m_Operand that allows matching against a Value's specified
> operand. Provides m_Intrinsic pattern that can be templatized over the
> intrinsic id and bind/match operands similarly to other pattern matchers.
> Implementations provided for 0 to 4 arguments, though it's very simple to
> extend for more. Also provides example template specialization for bswap
> (m_BSwap) and example of code cleanup for its use.
>
>
>
>
>
>
> On Dec 10, 2012, at 8:55 PM, Chandler Carruth <chandlerc at google.com>
> wrote:
>
> On Mon, Dec 10, 2012 at 7:33 PM, Michael Ilseman <milseman at apple.com>wrote:
>
>>
>> On Dec 10, 2012, at 4:53 PM, Chandler Carruth <chandlerc at google.com>
>> wrote:
>>
>> Hey Michael,
>>
>> On Dec 7, 2012, at 1:40 PM, Michael Ilseman <milseman at apple.com> wrote:
>>>
>>> > Below is a patch to add some pattern matching support for intrinsics
>>> to PatternMatch.h:
>>> >
>>> >
>>> > Pattern matching code for intrinsics.
>>> >
>>> > Provides m_Intrinsic pattern that can be templatized over the
>>> intrinsic id and bind/match operands similarly to other pattern matchers.
>>> Also provides example template specialization for bswap (m_BSwap) and
>>> example of code cleanup for its use. One motiviation for this is to ease
>>> upcoming peephole fast-math optimizations which may want to inspect into
>>> built-in intrinsics.
>>>
>>
>> I really like the idea of intrinsic matchers. I think the current
>> matching patterns need some tweaking though...
>>
>> First, for the baseline matching: I would make this pattern able to match
>> a reasonable number of arguments. I think my ideal syntax would be:
>>
>>   m_Intrinsic(Intrinsic::foo, m_Args(m_Foo(), m_Bar(), m_Baz(), ...))
>>
>> I'm not sure if having the extra 'm_Args' layer is too much, we could
>> fold them directly into the top level matcher.
>>
>>
>> I like the suggestion. The multi-argument top level matcher is what's
>> currently present in my patches, though it's miserably messy. Without
>> variadic templates, there might have to be a little mess, but I can
>> probably have a single Intrinsic_match struct that's parameterized over the
>> operand index.
>>
>> I think you can do this for as many arguments as we would ever need in a
>> pattern matcher by just providing default template arguments for them, and
>> have, say, 8 of them.
>>
>>
>> Yes, I had implemented m_Intrinsic from 0 to 2 arguments, but there's no
>> reason not to have more, especially with some cleanup and refactoring of
>> the structs.
>>
>>
>> I think the only other abstractions which should exist at the matcher
>> level should be based on the synthetic classes in IntrinsicInst.h. In
>> particular, I think Unary and Binary aren't good abstractions for intrinsic
>> calls. And we shouldn't need them if we provide generic argument matching
>> as above?
>>
>>
>> Unary and Binary were not to be used by the user, but were for
>> implementation mechanics only. They'll also disappear when I implement your
>> suggestions above. Given that, are there uses for the synthetic classes?
>> Would there be a user that would want to e.g. match any non-memread
>> intrinsic, but not care which one in particular they're matching? Or do you
>> mean the subclasses of IntrinsicInst?
>>
>
> I mean the particular subclasses.
>
>
>>  If so, then I'd feel more comfortable with someone who actually uses
>> those adding in special cases, as I'm not familiar with the ways or reasons
>> that you would want to distinguish them.
>>
>
> I agree. More importantly, we should add them when we have some concrete
> code to clean up using them. =]
>
> An interesting question is whether it's worth adding m_Bswap(...) and
> other aliases for m_Intrinsic(Intrinsic::bswap, ...)? I don't have any
> really good insight into the frequency with which you'll need to match
> particular intrinsics, or when the best cutoff point is for a custom
> matcher... As a strawman heuristic for choosing when to add a custom
> matcher, how about adding one for any intrinsic in the langref we match in
> more than one place? If you anticipate adding lots of matching rules for
> target-specific intrinsics, I'm not opposed to adding matchers even for
> those. I would just commit new aliases as they are convenient, they have no
> real design cost.
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121212/f613d329/attachment.html>