Patch adds new GICombineRules for G_ADD:
- G_ADD(x, G_SUB(y, x)) -> y
- G_ADD(G_SUB(y, x), x) -> y
Patch additionally adds new combine tests for AArch64 target for these new rules.
Paths
| Differential D87936
[GISel] Add new combines for G_ADD ClosedPublic Authored by mkitzan on Sep 18 2020, 1:14 PM.
Details Summary Patch adds new GICombineRules for G_ADD:
Patch additionally adds new combine tests for AArch64 target for these new rules.
Diff Detail
Unit TestsFailed Event TimelineHerald added subscribers: llvm-commits, kerbowa, hiraditya and 4 others. · View Herald TranscriptSep 18 2020, 1:14 PM
Comment Actions
Primarily canonicalization, however depending on the target there can be perf wins. By eliminating the signed constant, there's a better chance the constant can be folded into the opcode as an immediate operand rather than being loaded into a register before being used (if the target supports immediate operands, otherwise nothing lost, eh?). Assume hypothetical target only supports 2b immediate operands to arithmetic ops and registers are 8b. Here's an example where the combine is a perf win. mov r1, -1 add r0, r1 --- sub r0, 1
Comment Actions
Comment Actions I can remove the G_ADD(x, y)->G_OR(x, y) combine if that's a blocker. Let me know if there's anything else specifically blocking the approval of this patch. Comment Actions I do think the add -> or has some value, but it should be separate with the helper function
Comment Actions Good point about using mi_match, updated the diff. Can work on the add -> or combine + helper function in a future patch. Better to not block committing the simpler combines.
Comment Actions
I guess this breaks down as soon as you have a 2 component mode Comment Actions I'll remove that combine as well. If there's more discussion and it's found to be beneficial then I can open another revision for it. Comment Actions Updates:
Comment Actions I think this looks reasonable now? I basically only have one nit.
This revision is now accepted and ready to land.Jun 6 2022, 9:25 AM Comment Actions Thanks for the review @paquette. I'll update the diff addressing your nit when I push the patch This revision was landed with ongoing or failed builds.Jun 6 2022, 11:19 AM Closed by commit rGb7fcf6632fe3: [GISel] Add new combines for G_ADD (authored by mkitzan). · Explain Why This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 430513 llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
llvm/include/llvm/Target/GlobalISel/Combine.td
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
llvm/test/CodeGen/AArch64/GlobalISel/combine-add.mir
llvm/test/CodeGen/AMDGPU/GlobalISel/add.v2i16.ll
llvm/test/CodeGen/AMDGPU/GlobalISel/combine-urem-pow-2.mir
llvm/test/CodeGen/AMDGPU/GlobalISel/extractelement.ll
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.memmove.ll
llvm/test/CodeGen/AMDGPU/GlobalISel/saddsat.ll
llvm/test/CodeGen/AMDGPU/GlobalISel/ssubsat.ll
|
APInt to allow it to work for wide integers?