[PATCH] D93963: [GlobalISel][AMDGPU] Lower G_UMULO/G_SMULO

[Pushpinder Singh via Phabricator via llvm-commits]

pdhaliwal added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1828
+  unsigned SrcBitWidth = SrcTy.getScalarSizeInBits();
+  assert(WideTy.getScalarSizeInBits() >= 2 * SrcBitWidth);
+
----------------
foad wrote:
> If the wide type is at least twice as wide as the original type, then the widened multiply provably will not overflow, so you don't need the final "or".
> 
> If we want to support widening to a type that is less than twice as wide as the original type, then remove this assert and only do the final "or" if `WideTy.getScalarSizeInBits() >= 2 * SrcBitWidth`. But I'm not sure how you would write a test for that code path.
That "or" exists to match the existing DAG logic. But you are right that the assert makes "or" redundant. I have removed the assert and added tests for s24 to verify the logic.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93963