[PATCH] D106139: [DAGCombine] Combine srX of add that intends to get the carry as uaddo

[Abinav Puthan Purayil via Phabricator via llvm-commits]

abinavpp added a comment.

In D106139#2904046 <https://reviews.llvm.org/D106139#2904046>, @lebedev.ri wrote:

> Two comments:
>
> 1. the transform you want is https://alive2.llvm.org/ce/z/NFDE9C i.e. you need to start from `lshr i64 %z, 32`, match it's operands, and do *not* look at it's uses.

We're not looking at lshr's uses here, but that of the add in lshr(add()). I would like this to cover truncUse() in AMDGPU/combine-srx-add.ll (g() in X86/addcarry.ll). I'm not sure if we can do that without looking at the add's uses.

In D106139#2904046 <https://reviews.llvm.org/D106139#2904046>, @lebedev.ri wrote:

> 2. This is really something that should go into instcombine

@foad mentioned the same. While I understand that target-specific combines should go to lib/target/<target> with setTargetDAGCombine(), I'm not able to determine if the target independent ones should go to target independent version of inst-combine, DAG-combine, or both. I understand that keeping at both has the advantage of better coverage since inst-combine might catch the pattern that DAG-combine might not be able to catch or vice-versa. I'm not able to see any motivation in *moving* this to inst-combine, but I do see a motivation in *copying* this to inst-combine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106139