[llvm-commits] [PATCH][FAST-MATH] recognize fp negation

Chandler Carruth chandlerc at google.com
Fri Jan 4 13:21:09 PST 2013


On Fri, Jan 4, 2013 at 11:57 AM, Michael Ilseman <milseman at apple.com> wrote:

>
> On Jan 4, 2013, at 11:05 AM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>
> > I personally think it is more natural to define isFNeg() as a member
> function of class BinaryOperator.
> > It is in many ways similar to BinaryOperator::isCompoundAssignmentOp(),
> .., BinaryOperator::isShiftAssignOp() etc.
> > How come it sounds odd to you.
>
> I think Duncan was saying that all those methods feel a bit antiquated. In
> a separate patch, it might be good to remove these, but I'll let someone
> who has better historical context make that call. I don't know what's
> considered modern LLVM style for those checks, but I think it might be to
> use PatternMatch.h.


Without trying to address this specific patch (I've not looked), I'll try
to answer the general question of how these APIs should be structured:

I think it is a good thing to have obvious predicates on instruction
classes, but I think we should carefully limit the proliferation of
subclass predicates in the parent class APIs. To be clear, this last
category consists of any predicate like so:

class Derived;
class Base {
public:
  bool isDerivedProperty() { if (Derived *D = dyn_cast<Derived>(this))
return D->isProperty(); else return false; }
};

The only time this pattern makes sense to me is when there is a
tremendously large number of places which boil down to the dyn_cast +
method test, and there is no other use of the derived typed pointer, and a
reasonable number of them occur within large compound predicates making the
2-step test significantly more code than the base class predicate would be.

The best examples of this are in the Type class, where there really are
systematic patterns that we can make shorter this way.


I think we should move completely away from methods which are entirely
equivalent to { return isa<Derived>(this); } and instead just write the
'isa' pattern.


Finally, there is a reason to have *both* a predicate on a particular
instruction class and a pattern match interface: the pattern matching
interfaces *compose*! That is their key power, and the primary reason we
should be expanding the pattern matching support for the IR patterns that
optimizations need to match (yes, it is well named for once). Personally, I
suspect there should be nearly a 1:1 correspondence between predicate
methods on instruction classes and pattern matching utilities. For example,
BranchInst can be both conditional and unconditional, and we need pattern
matching to support both forms.


Now, BinaryOperator is quite special because the specific binary operators
don't get their own instruction classes. I think this is generally a good
thing to limit the interface creep of the IR, but at some point it might be
reasonable to introduce subclasses for particular instructions where there
are a large number of useful predicates which are only relevant to a
*single* operation. I don't think we're there yet though. As long as the
predicate methods on BinaryOperator make sense for more than one operation,
I think they're totally reasonable.

At most, I'm seeing some early indications that it might me nice to have
broad categories such as FPBinaryOperator that could help organize those
predicates. We can add that subclass lazily though by keeping the
predicates which would sink into such a class clearly organized, and making
them assert fail (rather than just return false) if called on an
inappropriate operation.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130104/2f5dac25/attachment.html>


More information about the llvm-commits mailing list