[llvm-commits] [llvm] r151365 - in /llvm/trunk: lib/Transforms/Scalar/GVN.cpp test/Transforms/GVN/commute.ll

Duncan Sands baldrick at free.fr
Fri Feb 24 11:11:45 PST 2012


Hi Dan,

>> +  if (I->isCommutative()) {
>> +    // Ensure that commutative instructions that only differ by a permutation
>> +    // of their operands get the same value number by sorting the operand value
>> +    // numbers.  Since all commutative instructions have two operands it is more
>> +    // efficient to sort by hand rather than using, say, std::sort.
>> +    assert(I->getNumOperands() == 2&&  "Unsupported commutative instruction!");
>> +    if (e.varargs[0]>  e.varargs[1])
>> +      std::swap(e.varargs[0], e.varargs[1]);
>> +  }
>
> The reassociate pass already canonicalizes the order of operands in
> add instructions. Is it really desirable for GVN to redo it?

just to be clear, this doesn't change the instruction, it only ensures that the
same value number is given to x+y and y+x.  This should be useful for floating
point arithmetic (which is commutative but not associative), though it probably
doesn't fire much for integer arithmetic since reassociate should get almost all
cases as you say [*].

>
>>
>>    if (CmpInst *C = dyn_cast<CmpInst>(I)) {
>> -    e.opcode = (C->getOpcode()<<  8) | C->getPredicate();
>> +    // Sort the operand value numbers so x<y and y>x get the same value number.
>> +    CmpInst::Predicate Predicate = C->getPredicate();
>> +    if (e.varargs[0]>  e.varargs[1]) {
>> +      std::swap(e.varargs[0], e.varargs[1]);
>> +      Predicate = CmpInst::getSwappedPredicate(Predicate);
>> +    }
>> +    e.opcode = (C->getOpcode()<<  8) | Predicate;
>
> Reassociate doesn't canonicalize comparisons like it does adds,
> but it easily could.

In the meantime we have this.  It is extremely cheap: it's just comparing and
flipping two integers in a SmallVector.  It fires from time to time, mostly
resulting in equality comparisons A=B being merged with B=A.

Ciao, Duncan.

[*] I didn't see it have any effect on some fairly big integer testcases.  It
might in theory fire in cases where reassociate was unable to reassociate due to
uses.



More information about the llvm-commits mailing list