[llvm-commits] [llvm] r61537 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/xor-demorgans.ll

[Chris Lattner]

On Dec 31, 2008, at 5:18 PM, Bill Wendling wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=61537&view=rev
> Log:
> Add transformation:
>
> xor (or (icmp, icmp), true) -> and(icmp, icmp)
>
> This is possible because of De Morgan's law.

Nice.  Note that this should only be done if the two compares each  
only ->hasOneUse().  Otherwise, you're introducing new compares.

Also, there is already code that handles a top-level not, please merge  
this into that code.  The code is:

   // Is this a ~ operation?
   if (Value *NotOp = dyn_castNotVal(&I)) {
     // ~(~X & Y) --> (X | ~Y) - De Morgan's Law
     // ~(~X | Y) === (X & ~Y) - De Morgan's Law

I'm not sure why that code exists and then the "bool not" code is  
separate, but it would be nice to merge them together. I realize this  
existed before your change :)

> +      if (Op0I) {
> +        Value *A, *B;
> +        if (match(Op0I, m_Or(m_Value(A), m_Value(B)))) {
> +          ICmpInst *AOp = dyn_cast<ICmpInst>(A);
> +          ICmpInst *BOp = dyn_cast<ICmpInst>(B);

Can you use m_ICmp here?  Why not also handle fcmp?  fcmp is  
invertible as well.

In practice, I think it would be better to simplify/merge this whole  
class of transformations by adding a new pair of static function:

bool isCheaplyInvertible(Value*);
Value *getChealyInvertedValue(Value*);

This would allow you to handle the icmp/fcmp/and "~" cases all  
identically instead of having slightly different copies of the various  
"demorgan" code.

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090102/3a40d42d/attachment.html>