[PATCH] D72833: [GlobalISel] Use more MachineIRBuilder helper methods
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 16 06:33:04 PST 2020
arsenm added inline comments.
================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1995
+ MIRBuilder.setInsertPt(MIRBuilder.getMBB(), ++MIRBuilder.getInsertPt());
+
Register HiPart = MRI.createGenericVirtualRegister(Ty);
----------------
arsenm wrote:
> foad wrote:
> > In case you were wondering about this hunk: the old code left the G_SMULO/UMULO in place, which defined Res, and also created a new G_MUL, which defined Res. This caused an assertion failure when buildAShr tried to constant fold its arguments, because it wasn't expecting to find more than one definition of Res. Is this a general problem in GlobalISel lowering and is my workaround reasonable?
> It looks to me like the old code didn't modify the instruction in place, the new code does. Most places create new defs of the original result before erasing the original instruction, so this might be a widespread issue with the CSEMIRBuilder
Can you split this part into a separate change?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72833/new/
https://reviews.llvm.org/D72833
More information about the llvm-commits
mailing list