[PATCH] D93963: [GlobalISel][AMDGPU] Lower G_UMULO/G_SMULO
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 19 06:28:30 PDT 2021
arsenm added inline comments.
================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1854-1855
+ } else {
+ // Unsigned overflow occurred if the high part is non-zero
+ auto ZExtResult = MIRBuilder.buildZExtInReg(WideTy, Mul, SrcBitWidth);
+ Overflow =
----------------
foad wrote:
> pdhaliwal wrote:
> > arsenm wrote:
> > > I don't think the unsigned case is right. The DAG version inserts a shift here, not a mask
> > I have changed it use shift instead of masking. Just curious, why was previous logic wrong? I thought zero'ing the upper bits of multiplication result and then comparing it with latter should provide the correct result.
> I'm pretty sure the previous logic was fine too, it's just a different way of checking the upper part is zero.
Oh yes, I just can't read. Arguably avoiding the shift is better since shifts can be more expensive
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93963/new/
https://reviews.llvm.org/D93963
More information about the llvm-commits
mailing list