[PATCH] D118635: [GlobalISel] Combine: (G_*MULO x, 0) -> 0 + no carry out

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4574-4587
+    if (!isLegalOrBeforeLegalizer(
+            {TargetOpcode::G_BUILD_VECTOR, {DstTy, DstEltTy}}) ||
+        !isLegalOrBeforeLegalizer({TargetOpcode::G_CONSTANT, {DstEltTy}}))
+      return false;
+    LLT CarryEltTy = CarryTy.getElementType();
+    if (!isLegalOrBeforeLegalizer(
+            {TargetOpcode::G_BUILD_VECTOR, {CarryTy, CarryEltTy}}) ||
----------------
arsenm wrote:
> This condition is way too complicated and common to put here like this. We probably should have a predicate like isConstantUnsupported in the artifact combiner somewhere
I tried to clean it up a bit in a follow up (D118655) but maybe the artifact combiner is a better place to handle it.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4590
+    B.buildConstant(Dst, 0);
+    B.buildConstant(Carry, 0);
+  };
----------------
foad wrote:
> How does this work? Don't you need to build a single instruction that has two results (just like G_*MULO)?
I don't think it matters; although G_*MULO instructions produce two results, those results are just regular vregs. As long as they are still defined, the uses should work out.

So, we should be able to replace this with two instructions if we know what the results are going to be. In the case of

```
%mul, %o = G_*MULO %x, <zero>
```

We know that the value of `%mul` and `%o` must both be 0. So replacing them both with appropriately-sized 0s ought to be safe.


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

https://reviews.llvm.org/D118635