llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

Missed optimisation opportunity for `xor` lowering

Open ljmf00-wekaio opened this issue 3 years ago • 5 comments

I noticed that compiling the following D code on LLVM 12 and LLVM 13 yields different results:

bool xor(bool lhs, bool rhs)
{
    return (lhs && !rhs) || (!lhs && rhs);
}

https://godbolt.org/z/WTW1qeEMY

Eventhough I'm not entirely sure this is a regression with LLVM itself or perhaps a different approach LDC takes to glue the LLVM backend, but it seems that it is not a correctness issue. Running alive2 against the different generated IRs, it can validate the transformation successfully. https://alive2.llvm.org/ce/z/cWdpvA

Perhaps this can be seen as an improvement to add this logic simplifiation in a transformation pass.

ljmf00-wekaio avatar Oct 12 '22 11:10 ljmf00-wekaio

Could you please try 15 or main branch? 13 is too old.

EugeneZelenko avatar Oct 12 '22 13:10 EugeneZelenko

AFAIK, LDC is not prepared to compile with LLVM 15, as it uses the old pass manager. The official builds uses LLVM 13 and can compile to 14. Anyway, it seems that it produces the same unoptimised IR https://d.godbolt.org/z/W99WKf9qz, so its either an AST-level optimization on the D frontend or a different approach it takes on IR generation. Applying @optStrategy UDA to force no IR optimizations, makes me assume that no AST transformation is made. Either way, according to godbolt, optimizing both IRs, can't lead to a xor lowering, meaning it is a missed optimization opportunity. Since I can't prove regression I'll change the title of this issue.

Optimisations that could take place:

(lhs && !rhs) || (!lhs && rhs)

https://alive2.llvm.org/ce/z/QWJcKv

(lhs || rhs) && !(lhs && rhs)

https://alive2.llvm.org/ce/z/8fQRvn

Both are equivelent to xor instruction.

ljmf00-wekaio avatar Oct 13 '22 13:10 ljmf00-wekaio

We may want to enhance a conditional branch fold in instcombine that swaps the destination blocks to also match a select-of-bools: https://alive2.llvm.org/ce/z/__bcRG

Note that currently we would canonicalize the new select in that sequence to include another 'not' instruction; I'm not sure yet what overall changes would occur if we alter that decision.

rotateright avatar Oct 13 '22 14:10 rotateright

https://alive2.llvm.org/ce/z/8fQRvn Both are equivelent to xor instruction.

That one looks like an obvious missing reduction, so: d85505a9323a (not sure if that changes anything for the motivating source example)

rotateright avatar Oct 13 '22 20:10 rotateright

https://alive2.llvm.org/ce/z/8fQRvn Both are equivelent to xor instruction.

That one looks like an obvious missing reduction, so: d85505a (not sure if that changes anything for the motivating source example)

Yes, the expressions I showed above is the equivelent translation to the IR showed in alive2, so the second one should generate optimized code now.

ljmf00-wekaio avatar Oct 14 '22 09:10 ljmf00-wekaio

I think the examples here are fixed now, so closing. We still have missing folds for logical-and and logical-or: #58589 ...so if you find other examples, please do file another bug report. Thanks!

rotateright avatar Nov 01 '22 12:11 rotateright