[PATCH] D95432: AMDGPU/GlobalISel: Combine uniform icmp with one use

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp:76-77
+  Register Condition = getConditionOp(MI).getReg();
+  if (!isSgprRegBank(Condition) || !MRI.hasOneNonDBGUse(Condition))
+    return false;
+
----------------
Petar.Avramovic wrote:
> arsenm wrote:
> > I don't think this really needs a one use check. Consider the case where another use already exists without the intermediate casts:
> > 
> > %x:s32 = G_ICMP
> > %y:s1 = G_TRUNC %x
> > %z:s32 = G_ZEXT %y
> > %select0 = G_SELECT %x
> > %select1 = G_SELECT %z
> I wanted to keep this as simple as possible and cover most common case (single use).
> Can we get mir like that? Regbankselect independently legalizes sgpr icmp and select. ICMP gets followed by G_TRUNC and G_SELECT has to G_ZEXT input condition. I would expect something like this:
> ```
> %x:s32 = G_ICMP
> %y:s1 = G_TRUNC %x
> ...
> %z:s32 = G_ZEXT %y
> %select1 = G_SELECT %z
> ...
> %w:s32 = G_ZEXT %y
> %select0 = G_SELECT %w
> ```
> Maybe we could cover this case for select, are there more?
> Afaik only trunc can end up having multiple uses. So we have to check G_TRUNC uses and find the one that Helper.dominates other uses. If this happens to be our G_ZEXT then move icmp before select and trunc after select, like this:
> ```
> ...
> %z:s32 = G_ZEXT %y //dead
> %x:s32 = G_ICMP
> %select1 = G_SELECT %x
> %y:s1 = G_TRUNC %x
> ...
> %w:s32 = G_ZEXT %y
> %select0 = G_SELECT %w
> ```
> 
If the builder is CSEing, you could end up with the same trunc used multiple times


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

https://reviews.llvm.org/D95432