[PATCH] D60854: [DAGLegalize][PowerPC] Add promote legalization of addc/adde and subc/sube

Zixuan Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 21 22:19:38 PDT 2019


wuzish added a comment.

In D60854#1472817 <https://reviews.llvm.org/D60854#1472817>, @efriedma wrote:

> > In former condition, such transformation is acceptable even operation ADDE is illegal and it only cares about type legality because illegal operation will be legalized later in DAG legalization phase
>
> Generally, yes, before operation legalization we can create illegal operations.
>
> ADDE is sort of an exception, though; we don't have operation legalization support for it, and we don't want to add it.  So the condition for the DAGCombine should be fixed.


Yes, I found there is a TODO to support operation legalization as you said for ADDE. https://github.com/llvm/llvm-project/blob/e7fe6dd5edb828e702d02c7cdc6565ace4c84f5b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp#L2049.

So the principle is that the DAG nodes converted from LLVM IR basically all have related common type and operation legalization (in base class), but the nodes only generated during combining and legalizing phases don't have fully support for common type and operation legalization. Instead, we just don't generate such nodes if targets do not support. Right?

Well, so the easiest way to fix this bug is to remove `!LegalOperations` before `||`.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60854/new/

https://reviews.llvm.org/D60854





More information about the llvm-commits mailing list