[llvm] [LICM] Fold associative binary ops to promote code hoisting (PR #81608)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 23 03:14:31 PDT 2024
https://github.com/nikic commented:
I've reverted this due to the incorrect flag preservation, but generally the test coverage on this PR leaves a lot to be desired. Please see https://llvm.org/docs/InstCombineContributorGuide.html#tests for some testing guidelines.
In particular, things I would note are:
* This implements a fairly generic fold using isAssociative(), but then only tests adds. Please also add tests for some other opcodes -- in particular also for FP operations. Note that for FP operations two binary operators with the same opcode may not both be associative! Only one of them might have the reassoc flag, in which case the transform is not legal.
* It's okay to include a test that is close to your original motivation, but please keep the bulk of the test coverage simple. Don't test on vectors when scalars would do. This reduces test noise and makes proof checking faster.
* There is a complete lack of negative tests. You have a bunch of conditions that abort the transform, but none of them are tested.
https://github.com/llvm/llvm-project/pull/81608
More information about the llvm-commits
mailing list