[PATCH] D53478: [DAGCombine] SimplifyNodeWithTwoResults - ensure same legalization for LO/HI operands (PR21207)

[Nirav Dave via Phabricator via llvm-commits]

niravd added a comment.

which supports my supposition, but "hasOperation" seems to match your interpretation (maybe that's the right check here).

The initial PR21207 seems to only discuss reverting this particular case and states there are other cases in DAGCombiner (and I confirmed there are a few scattered about) .

If we return a custom node post-legalization it should be legalized into a non-custom node, so modulo causing an infinite chain of DAG changes (which we don't have any formal means of preventing anyways) shouldn't returning custom generally be okay? Do you have access to the OOT backend that's triggered PR21207?

(Side note: The 3rd and 4th optimization cases in this function looks redundant with the first two)

In https://reviews.llvm.org/D53478#1270788, @RKSimon wrote:

> In https://reviews.llvm.org/D53478#1270752, @niravd wrote:
>
> > I don't think that need to avoid custom operations post-legalization. The documentation doesn't seem to address this, but as I understand it, Custom is Legal but with non-standard lowering and we just happen to lower custom node in legalize as an easy way to sure we've legalized the DAG.
>
>
> I don't think we have any other cases in DAGCombiner where we use !LegalOperations || TLI.isOperationLegalOrCustom





Repository:
  rL LLVM

https://reviews.llvm.org/D53478