[PATCH] D88514: [GlobalISel] Fix multiply with overflow intrinsics legalization generating invalid MIR.

Amara Emerson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 29 14:53:53 PDT 2020


aemerson created this revision.
aemerson added reviewers: foad, aditya_nandakumar, arsenm.
aemerson added a project: LLVM.
Herald added subscribers: atanasyan, jrtc27, hiraditya, rovka, sdardis.
aemerson requested review of this revision.
Herald added a subscriber: wdng.

During lowering of G_UMULO and friends, the previous code moved the builder's insertion point to be after the legalizing instruction. When that happened, if there happened to be a "G_CONSTANT i32 0" immediately after, the CSEMIRBuilder would try to find that constant during the buildConstant(zero) call, and since it dominates itself would return the iterator unchanged, even though the def of the constant was *after* the current insertion point. This resulted in the compare being generated *before* the constant which it was using.

There's no need to modify the insertion point before building the mul-hi or constant. Delaying moving the insert point ensures those are built/CSEd before the G_ICMP is built.

We might still have a general issue with CSEMIRBuilder though with this edge case.

Fixes PR47679


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88514

Files:
  llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-mul.mir
  llvm/test/CodeGen/Mips/GlobalISel/legalizer/mul.mir
  llvm/test/CodeGen/Mips/GlobalISel/llvm-ir/mul.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88514.295127.patch
Type: text/x-patch
Size: 6820 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200929/69f87ee3/attachment.bin>


More information about the llvm-commits mailing list