[PATCH] D106139: [AMDGPU] Combine srl of add that intends to get the carry of the add as addcarry
Abinav Puthan Purayil via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 22 09:36:29 PDT 2021
abinavpp marked an inline comment as done.
abinavpp added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp:3285-3286
+
+ // Make sure that the users of `N0` is not expecting the zero-extended type
+ for (SDNode *U : N0->uses()) {
+ if (U == N)
----------------
arsenm wrote:
> abinavpp wrote:
> > arsenm wrote:
> > > I don't understand why you are looking at the uses, you probably should just skip this if !N0.hasOnEUse
> > The add I'm handling in https://github.com/RadeonOpenCompute/ROCm/issues/488 is used by a truncate, so we need to check for it to improve coverage. https://alive2.llvm.org/ce/z/Vv4DSr is what I'm trying to cover here.
> If the transform can only be done from a trunc root, the combine should start by looking at the trunc, not the add
We need to handle both N0.hasOneUse() case (i.e. only srl uses the add) and !N0.hasOneUse() iff the other uses are truncates to the type of the UADDO.add that we intend to create. If we do the combine rooted at trunc, we miss out the N0.hasOneUse() case. I think it makes more sense to do this rooted at srl since it's at srl's perspective we see that the add can be a uaddo. I'm not happy with the traversal of the uses of add since I like DAG-combine functions to only worry about the operands directions (i.e. towards the leaves) and not worry about the uses direction (i.e. towards the root), but to get the coverage I'm looking for, I can't see a better way to do this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106139/new/
https://reviews.llvm.org/D106139
More information about the llvm-commits
mailing list