Skip to content

Commit 2f3708a

Browse files
committed
[InstCombine][Docs] Update InstCombine contributor guide
1 parent ea73fc5 commit 2f3708a

File tree

1 file changed

+38
-0
lines changed

1 file changed

+38
-0
lines changed

llvm/docs/InstCombineContributorGuide.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,11 +404,31 @@ The use of TargetTransformInfo is only allowed for hooks for target-specific
404404
intrinsics, such as `TargetTransformInfo::instCombineIntrinsic()`. These are
405405
already inherently target-dependent anyway.
406406

407+
If some canonicalization narrow/widen the integer width of expressions, please
408+
check `shouldChangeType()` first. Otherwise, we may evaluate the expression
409+
in illegal/inefficient types. For vector types, follow the instructions below.
410+
407411
For vector-specific transforms that require cost-modelling, the VectorCombine
408412
pass can be used instead. In very rare circumstances, if there are no other
409413
alternatives, target-dependent transforms may be accepted into
410414
AggressiveInstCombine.
411415

416+
Generally, we prefer unsigned operations over signed operations in the middle-end, even
417+
if signed operations are more efficient on some targets. The following is an incomplete
418+
list of canonicalizations that implemented in InstCombine:
419+
420+
| Original Pattern | Canonical Form | Condition |
421+
|------------------------------|--------------------|-------------------------------|
422+
| `icmp spred X, Y` | `icmp upred X, Y` | `sign(X) == sign(Y)` |
423+
| `smin/smax X, Y` | `umin/umax X, Y` | `sign(X) == sign(Y)` |
424+
| `sext X` | `zext nneg X` | `X >=s 0` |
425+
| `sitofp X` | `uitofp nneg X` | `X >=s 0` |
426+
| `ashr X, Y` | `lshr X, Y` | `X >=s 0` |
427+
| `add X, Y` | `or disjoint X, Y` | `(X & Y) != 0` |
428+
| `mul X, C` | `shl X, Log2(C)` | `isPowerOf2(C)` |
429+
| `select Cond1, Cond2, false` | `and Cond1, Cond2` | `impliesPoison(Cond2, Cond1)` |
430+
| `select Cond1, true, Cond2` | `or Cond1, Cond2` | `impliesPoison(Cond2, Cond1)` |
431+
412432
### PatternMatch
413433

414434
Many transforms make use of the matching infrastructure defined in
@@ -531,6 +551,19 @@ need to add a one-use check for the inner instruction.
531551
One-use checks can be performed using the `m_OneUse()` matcher, or the
532552
`V->hasOneUse()` method.
533553

554+
### Flag handling
555+
556+
When possible, propagate poison-generating flags like `nuw` and `nsw` since they may be
557+
hard to salvage later. If it is not free (e.g. requires additional complexity like `willNotOverflow`
558+
or KnownBits queries), don't do that.
559+
560+
Be careful with in-place operand/predicate changes, as poison-generating flags may not be valid for new
561+
operands. It is recommended to create a new instruction with carefully handling of flags. If not
562+
applicable, call `Instruction::dropPoisonGeneratingFlags()` to clear flags in a conservative manner.
563+
564+
Do not rely on fcmp's `nsz` flag to perform optimizations. It is meaningless for fcmp so it should not affect
565+
the optimization.
566+
534567
### Generalization
535568

536569
Transforms can both be too specific (only handling some odd subset of patterns,
@@ -558,6 +591,11 @@ guidelines.
558591
use of ValueTracking queries. Whether this makes sense depends on the case,
559592
but it's usually a good idea to only handle the constant pattern first, and
560593
then generalize later if it seems useful.
594+
* When possible, handle more canonical patterns as well. It is encouraged to avoid
595+
potential phase-ordering issues. For example, if the motivating transform holds for
596+
`add`, it also holds for `or disjoint`. See the canonicalization list above for details.
597+
In most cases, it can be easily implemented with matchers like
598+
`m_AddLike/m_SExtLike/m_LogicalAnd/m_LogicalOr`.
561599

562600
## Guidelines for reviewers
563601

0 commit comments

Comments
 (0)